strict checking on binary tree operations

diff --git a/src/appconfig.c b/src/appconfig.c
index 4749890bd9bc12160b2ad4934e19d1aed3a64206..81ab01be2cda7016ad6ffcbe7a9465dec723b42a 100644 (file)
--- a/src/appconfig.c
+++ b/src/appconfig.c
@@ -72,8 +72,8 @@ static int config_value_compare(void* a, void* b) {
     else return strcmp(((struct config_value *)a)->name, ((struct config_value *)b)->name);
 }
 
-#define config_value_index_add(co, cv) avl_insert_lock(&((co)->values_index), (avl *)(cv))
-#define config_value_index_del(co, cv) avl_remove_lock(&((co)->values_index), (avl *)(cv))
+#define config_value_index_add(co, cv) (struct config_value *)avl_insert_lock(&((co)->values_index), (avl *)(cv))
+#define config_value_index_del(co, cv) (struct config_value *)avl_remove_lock(&((co)->values_index), (avl *)(cv))
 
 static struct config_value *config_value_index_find(struct config *co, const char *name, uint32_t hash) {
     struct config_value tmp;
@@ -98,8 +98,8 @@ avl_tree_lock config_root_index = {
         AVL_LOCK_INITIALIZER
 };
 
-#define config_index_add(cfg) avl_insert_lock(&config_root_index, (avl *)(cfg))
-#define config_index_del(cfg) avl_remove_lock(&config_root_index, (avl *)(cfg))
+#define config_index_add(cfg) (struct config *)avl_insert_lock(&config_root_index, (avl *)(cfg))
+#define config_index_del(cfg) (struct config *)avl_remove_lock(&config_root_index, (avl *)(cfg))
 
 static struct config *config_index_find(const char *name, uint32_t hash) {
     struct config tmp;
@@ -127,7 +127,8 @@ static inline struct config *config_section_create(const char *section)
 
     avl_init_lock(&co->values_index, config_value_compare);
 
-    config_index_add(co);
+    if(unlikely(config_index_add(co) != co))
+        error("INTERNAL ERROR: indexing of section '%s', already exists.", co->name);
 
     config_global_write_lock();
     struct config *co2 = config_root;
@@ -154,7 +155,8 @@ static inline struct config_value *config_value_create(struct config *co, const
     cv->hash = simple_hash(cv->name);
     cv->value = strdupz(value);
 
-    config_value_index_add(co, cv);
+    if(unlikely(config_value_index_add(co, cv) != cv))
+        error("INTERNAL ERROR: indexing of config '%s' in section '%s': already exists.", cv->name, co->name);
 
     config_section_write_lock(co);
     struct config_value *cv2 = co->values;
@@ -198,14 +200,15 @@ int config_rename(const char *section, const char *old, const char *new) {
     cv2 = config_value_index_find(co, new, 0);
     if(cv2) goto cleanup;
 
-    config_value_index_del(co, cv);
+    if(unlikely(config_value_index_del(co, cv) != cv))
+        error("INTERNAL ERROR: deletion of config '%s' from section '%s', deleted tge wrong config entry.", cv->name, co->name);
 
     freez(cv->name);
     cv->name = strdupz(new);
-
     cv->hash = simple_hash(cv->name);
+    if(unlikely(config_value_index_add(co, cv) != cv))
+        error("INTERNAL ERROR: indexing of config '%s' in section '%s', already exists.", cv->name, co->name);
 
-    config_value_index_add(co, cv);
     config_section_unlock(co);
 
     return 0;

diff --git a/src/apps_plugin.c b/src/apps_plugin.c
index 7d5c325e34edc052b5ca91d2b789232816634cc0..350df46a9c21d0057224005b24c03b2f517a4d6f 100644 (file)
--- a/src/apps_plugin.c
+++ b/src/apps_plugin.c
@@ -1004,7 +1004,9 @@ static inline void file_descriptor_not_used(int id)
                 if(unlikely(debug))
                     fprintf(stderr, "apps.plugin:   >> slot %d is empty.\n", id);
 
-                file_descriptor_remove(&all_files[id]);
+                if(unlikely(file_descriptor_remove(&all_files[id]) != (void *)&all_files[id]))
+                    error("INTERNAL ERROR: removal of unused fd from index, removed a different fd");
+
 #ifdef NETDATA_INTERNAL_CHECKS
                 all_files[id].magic = 0x00000000;
 #endif /* NETDATA_INTERNAL_CHECKS */
@@ -1056,7 +1058,8 @@ static inline int file_descriptor_find_or_add(const char *name)
             all_files_index.root = NULL;
             for(i = 0; i < all_files_size; i++) {
                 if(!all_files[i].count) continue;
-                file_descriptor_add(&all_files[i]);
+                if(unlikely(file_descriptor_add(&all_files[i]) != (void *)&all_files[i]))
+                    error("INTERNAL ERROR: duplicate indexing of fd during realloc.");
             }
 
             if(unlikely(debug))
@@ -1145,7 +1148,8 @@ static inline int file_descriptor_find_or_add(const char *name)
 #ifdef NETDATA_INTERNAL_CHECKS
     all_files[c].magic = 0x0BADCAFE;
 #endif /* NETDATA_INTERNAL_CHECKS */
-    file_descriptor_add(&all_files[c]);
+    if(unlikely(file_descriptor_add(&all_files[c]) != (void *)&all_files[c]))
+        error("INTERNAL ERROR: duplicate indexing of fd.");
 
     if(unlikely(debug))
         fprintf(stderr, "apps.plugin: using fd position %d (name: %s)\n", c, all_files[c].name);

diff --git a/src/avl.h b/src/avl.h
index de4c24bbac131e007c9c380bdf297b1dcb2876df..53ef7ecaff7d9a497828e15b92684f92d6788883 100644 (file)
--- a/src/avl.h
+++ b/src/avl.h
@@ -56,16 +56,16 @@ typedef struct avl_tree_lock {
  * a is linked directly to the tree, so it has to
  * be properly allocated by the caller.
  */
-avl *avl_insert_lock(avl_tree_lock *t, avl *a) __attribute__((returns_nonnull));
-avl *avl_insert(avl_tree *t, avl *a) __attribute__((returns_nonnull));
+avl *avl_insert_lock(avl_tree_lock *t, avl *a) __attribute__((returns_nonnull, warn_unused_result));
+avl *avl_insert(avl_tree *t, avl *a) __attribute__((returns_nonnull, warn_unused_result));
 
 /* Remove an element a from the AVL tree t
  * returns a pointer to the removed element
  * or NULL if an element equal to a is not found
  * (equal as returned by t->compar())
  */
-avl *avl_remove_lock(avl_tree_lock *t, avl *a);
-avl *avl_remove(avl_tree *t, avl *a);
+avl *avl_remove_lock(avl_tree_lock *t, avl *a) __attribute__((warn_unused_result));
+avl *avl_remove(avl_tree *t, avl *a) __attribute__((warn_unused_result));
 
 /* Find the element into the tree that equal to a
  * (equal as returned by t->compar())

diff --git a/src/dictionary.c b/src/dictionary.c
index 91d3b45f1f8be424be9e1e68133babac17423b0d..fb9efeedb564258b24c16e5b7f0774fdc278fc23 100644 (file)
--- a/src/dictionary.c
+++ b/src/dictionary.c
@@ -59,9 +59,6 @@ static int name_value_compare(void* a, void* b) {
     else return strcmp(((NAME_VALUE *)a)->name, ((NAME_VALUE *)b)->name);
 }
 
-#define dictionary_name_value_index_add_nolock(dict, nv) do { NETDATA_DICTIONARY_STATS_INSERTS_PLUS1(dict); avl_insert(&((dict)->values_index), (avl *)(nv)); } while(0)
-#define dictionary_name_value_index_del_nolock(dict, nv) do { NETDATA_DICTIONARY_STATS_DELETES_PLUS1(dict); avl_remove(&(dict->values_index), (avl *)(nv)); } while(0)
-
 static inline NAME_VALUE *dictionary_name_value_index_find_nolock(DICTIONARY *dict, const char *name, uint32_t hash) {
     NAME_VALUE tmp;
     tmp.hash = (hash)?hash:simple_hash(name);
@@ -95,7 +92,10 @@ static NAME_VALUE *dictionary_name_value_create_nolock(DICTIONARY *dict, const c
     }
 
     // index it
-    dictionary_name_value_index_add_nolock(dict, nv);
+    NETDATA_DICTIONARY_STATS_INSERTS_PLUS1(dict);
+    if(unlikely(avl_insert(&((dict)->values_index), (avl *)(nv)) != (avl *)nv))
+        error("dictionary: INTERNAL ERROR: duplicate insertion to dictionary.");
+
     NETDATA_DICTIONARY_STATS_ENTRIES_PLUS1(dict);
 
     return nv;
@@ -104,7 +104,9 @@ static NAME_VALUE *dictionary_name_value_create_nolock(DICTIONARY *dict, const c
 static void dictionary_name_value_destroy_nolock(DICTIONARY *dict, NAME_VALUE *nv) {
     debug(D_DICTIONARY, "Destroying name value entry for name '%s'.", nv->name);
 
-    dictionary_name_value_index_del_nolock(dict, nv);
+    NETDATA_DICTIONARY_STATS_DELETES_PLUS1(dict);
+    if(unlikely(avl_remove(&(dict->values_index), (avl *)(nv)) != (avl *)nv))
+        error("dictionary: INTERNAL ERROR: dictionary invalid removal of node.");
 
     NETDATA_DICTIONARY_STATS_ENTRIES_MINUS1(dict);
 

diff --git a/src/plugin_tc.c b/src/plugin_tc.c
index cecadf36036fa21a12033a59b4db691ca944f3ed..1eef22f2f9f9e37639a9cc385d19c91baf43a7bd 100644 (file)
--- a/src/plugin_tc.c
+++ b/src/plugin_tc.c
@@ -100,8 +100,8 @@ avl_tree tc_device_root_index = {
         tc_device_compare
 };
 
-#define tc_device_index_add(st) avl_insert(&tc_device_root_index, (avl *)(st))
-#define tc_device_index_del(st) avl_remove(&tc_device_root_index, (avl *)(st))
+#define tc_device_index_add(st) (struct tc_device *)avl_insert(&tc_device_root_index, (avl *)(st))
+#define tc_device_index_del(st) (struct tc_device *)avl_remove(&tc_device_root_index, (avl *)(st))
 
 static inline struct tc_device *tc_device_index_find(const char *id, uint32_t hash) {
     struct tc_device tmp;
@@ -121,8 +121,8 @@ static int tc_class_compare(void* a, void* b) {
     else return strcmp(((struct tc_class *)a)->id, ((struct tc_class *)b)->id);
 }
 
-#define tc_class_index_add(st, rd) avl_insert(&((st)->classes_index), (avl *)(rd))
-#define tc_class_index_del(st, rd) avl_remove(&((st)->classes_index), (avl *)(rd))
+#define tc_class_index_add(st, rd) (struct tc_class *)avl_insert(&((st)->classes_index), (avl *)(rd))
+#define tc_class_index_del(st, rd) (struct tc_class *)avl_remove(&((st)->classes_index), (avl *)(rd))
 
 static inline struct tc_class *tc_class_index_find(struct tc_device *st, const char *id, uint32_t hash) {
     struct tc_class tmp;
@@ -146,7 +146,8 @@ static inline void tc_class_free(struct tc_device *n, struct tc_class *c) {
 
     debug(D_TC_LOOP, "Removing from device '%s' class '%s', parentid '%s', leafid '%s', seen=%d", n->id, c->id, c->parentid?c->parentid:"", c->leafid?c->leafid:"", c->seen);
 
-    tc_class_index_del(n, c);
+    if(unlikely(tc_class_index_del(n, c) != c))
+        error("plugin_tc: INTERNAL ERROR: attempt remove class '%s' from device '%s': removed a different calls", c->id, n->id);
 
     freez(c->id);
     freez(c->name);
@@ -619,7 +620,8 @@ static inline struct tc_device *tc_device_create(char *id)
         d->enabled = (char)-1;
 
         avl_init(&d->classes_index, tc_class_compare);
-        tc_device_index_add(d);
+        if(unlikely(tc_device_index_add(d) != d))
+            error("plugin_tc: INTERNAL ERROR: removing device '%s' removed a different device.", d->id);
 
         if(!tc_device_root) {
             tc_device_root = d;
@@ -660,7 +662,8 @@ static inline struct tc_class *tc_class_add(struct tc_device *n, char *id, char
             c->leaf_hash = simple_hash(c->leafid);
         }
 
-        tc_class_index_add(n, c);
+        if(unlikely(tc_class_index_add(n, c) != c))
+            error("plugin_tc: INTERNAL ERROR: attempt index class '%s' on device '%s': already exists", c->id, n->id);
     }
 
     c->seen = 1;
@@ -677,7 +680,8 @@ static inline void tc_device_free(struct tc_device *n)
         else tc_device_root = n->prev;
     }
 
-    tc_device_index_del(n);
+    if(unlikely(tc_device_index_del(n) != n))
+        error("plugin_tc: INTERNAL ERROR: removing device '%s' removed a different device.", n->id);
 
     while(n->classes) tc_class_free(n, n->classes);
 

diff --git a/src/registry.h b/src/registry.h
index cb7fb9f5e699ce99897e4018aef40a2f7b3ae75e..4947486c4351bc5ae6088fcd76ac97287cafdf1b 100644 (file)
--- a/src/registry.h
+++ b/src/registry.h
@@ -55,11 +55,9 @@
 // should only happen when netdata starts
 extern int registry_init(void);
 
-/*
 // free all data held by the registry
 // should only happen when netdata exits
 extern void registry_free(void);
-*/
 
 // HTTP requests handled by the registry
 extern int registry_request_access_json(struct web_client *w, char *person_guid, char *machine_guid, char *url, char *name, time_t when);

diff --git a/src/registry_person.h b/src/registry_person.h
index 2bbefa318ee57f09ee18d3ae1f97a5d4f9eed4c6..73c9f11c417453b9c234869b2d00c151f947b1a7 100644 (file)
--- a/src/registry_person.h
+++ b/src/registry_person.h
@@ -41,7 +41,7 @@ typedef struct registry_person REGISTRY_PERSON;
 
 // PERSON_URL
 extern REGISTRY_PERSON_URL *registry_person_url_index_find(REGISTRY_PERSON *p, const char *url);
-extern REGISTRY_PERSON_URL *registry_person_url_index_add(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) __attribute__((warn_unused_result));
+extern REGISTRY_PERSON_URL *registry_person_url_index_add(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) __attribute__((returns_nonnull, warn_unused_result));
 extern REGISTRY_PERSON_URL *registry_person_url_index_del(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) __attribute__((warn_unused_result));
 
 extern REGISTRY_PERSON_URL *registry_person_url_allocate(REGISTRY_PERSON *p, REGISTRY_MACHINE *m, REGISTRY_URL *u, char *name, size_t namelen, time_t when);

diff --git a/src/registry_url.h b/src/registry_url.h
index 34c1d189669cd412a69fc5f19a1eb659a7be765a..50bd2497e220961131986975fead9e7dc82cac13 100644 (file)
--- a/src/registry_url.h
+++ b/src/registry_url.h
@@ -22,8 +22,8 @@ typedef struct registry_url REGISTRY_URL;
 
 // REGISTRY_URL INDEX
 extern int registry_url_compare(void *a, void *b);
-extern REGISTRY_URL *registry_url_index_del(REGISTRY_URL *u);
-extern REGISTRY_URL *registry_url_index_add(REGISTRY_URL *u) __attribute__((returns_nonnull));
+extern REGISTRY_URL *registry_url_index_del(REGISTRY_URL *u) __attribute__((warn_unused_result));
+extern REGISTRY_URL *registry_url_index_add(REGISTRY_URL *u) __attribute__((returns_nonnull, warn_unused_result));
 
 // REGISTRY_URL MANAGEMENT
 extern REGISTRY_URL *registry_url_get(const char *url, size_t urllen) __attribute__((returns_nonnull));

diff --git a/src/rrd.c b/src/rrd.c
index 0b0da71d04d163a1fcc74868895771adc29ef97c..9f42b7b1b7140c9d19507df7775a093923985b99 100644 (file)
--- a/src/rrd.c
+++ b/src/rrd.c
@@ -118,7 +118,7 @@ RRDFAMILY *rrdfamily_create(const char *id) {
 
         RRDFAMILY *ret = rrdfamily_index_add(&localhost, rc);
         if(ret != rc)
-            fatal("INTERNAL ERROR: Expected to INSERT RRDFAMILY '%s' into index, but inserted '%s'.", rc->family, (ret)?ret->family:"NONE");
+            fatal("RRDFAMILY: INTERNAL ERROR: Expected to INSERT RRDFAMILY '%s' into index, but inserted '%s'.", rc->family, (ret)?ret->family:"NONE");
     }
 
     rc->use_count++;
@@ -130,10 +130,10 @@ void rrdfamily_free(RRDFAMILY *rc) {
     if(!rc->use_count) {
         RRDFAMILY *ret = rrdfamily_index_del(&localhost, rc);
         if(ret != rc)
-            fatal("INTERNAL ERROR: Expected to DELETE RRDFAMILY '%s' from index, but deleted '%s'.", rc->family, (ret)?ret->family:"NONE");
+            fatal("RRDFAMILY: INTERNAL ERROR: Expected to DELETE RRDFAMILY '%s' from index, but deleted '%s'.", rc->family, (ret)?ret->family:"NONE");
 
         if(rc->variables_root_index.avl_tree.root != NULL)
-            fatal("INTERNAL ERROR: Variables index of RRDFAMILY '%s' that is freed, is not empty.", rc->family);
+            fatal("RRDFAMILY: INTERNAL ERROR: Variables index of RRDFAMILY '%s' that is freed, is not empty.", rc->family);
 
         freez((void *)rc->family);
         freez(rc);
@@ -222,8 +222,8 @@ static int rrddim_compare(void* a, void* b) {
     else return strcmp(((RRDDIM *)a)->id, ((RRDDIM *)b)->id);
 }
 
-#define rrddim_index_add(st, rd) avl_insert_lock(&((st)->dimensions_index), (avl *)(rd))
-#define rrddim_index_del(st,rd ) avl_remove_lock(&((st)->dimensions_index), (avl *)(rd))
+#define rrddim_index_add(st, rd) (RRDDIM *)avl_insert_lock(&((st)->dimensions_index), (avl *)(rd))
+#define rrddim_index_del(st,rd ) (RRDDIM *)avl_remove_lock(&((st)->dimensions_index), (avl *)(rd))
 
 static RRDDIM *rrddim_index_find(RRDSET *st, const char *id, uint32_t hash) {
     RRDDIM tmp;
@@ -381,7 +381,8 @@ void rrdset_set_name(RRDSET *st, const char *name)
         rrddimvar_rename_all(rd);
     pthread_rwlock_unlock(&st->rwlock);
 
-    rrdset_index_add_name(&localhost, st);
+    if(unlikely(rrdset_index_add_name(&localhost, st) != st))
+        error("RRDSET: INTERNAL ERROR: attempted to index duplicate chart name '%s'", st->name);
 }
 
 // ----------------------------------------------------------------------------
@@ -634,7 +635,8 @@ RRDSET *rrdset_create(const char *type, const char *id, const char *name, const
         rrdsetvar_create(st, "update_every", RRDVAR_TYPE_INT, &st->update_every, 0);
     }
 
-    rrdset_index_add(&localhost, st);
+    if(unlikely(rrdset_index_add(&localhost, st) != st))
+        error("RRDSET: INTERNAL ERROR: attempt to index duplicate chart '%s'", st->id);
 
     rrdsetcalc_link_matching(st);
     rrdcalctemplate_link_matching(st);
@@ -777,7 +779,8 @@ RRDDIM *rrddim_add(RRDSET *st, const char *id, const char *name, long multiplier
 
     pthread_rwlock_unlock(&st->rwlock);
 
-    rrddim_index_add(st, rd);
+    if(unlikely(rrddim_index_add(st, rd) != rd))
+        error("RRDDIM: INTERNAL ERROR: attempt to index duplicate dimension '%s' on chart '%s'", rd->id, st->id);
 
     return(rd);
 }
@@ -816,7 +819,8 @@ void rrddim_free(RRDSET *st, RRDDIM *rd)
     while(rd->variables)
         rrddimvar_free(rd->variables);
 
-    rrddim_index_del(st, rd);
+    if(unlikely(rrddim_index_del(st, rd) != rd))
+        error("RRDDIM: INTERNAL ERROR: attempt to remove from index dimension '%s' on chart '%s', removed a different dimension.", rd->id, st->id);
 
     // free(rd->annotations);
     if(rd->mapped == RRD_MEMORY_MODE_SAVE) {
@@ -857,7 +861,8 @@ void rrdset_free_all(void)
         while(st->dimensions)
             rrddim_free(st, st->dimensions);
 
-        rrdset_index_del(&localhost, st);
+        if(unlikely(rrdset_index_del(&localhost, st) != st))
+            error("RRDSET: INTERNAL ERROR: attempt to remove from index chart '%s', removed a different chart.", st->id);
 
         st->rrdfamily->use_count--;
         if(!st->rrdfamily->use_count)