@cryptotaxi247 / netdata-1 / commits / d4c83d0cb

strict checking on binary tree operations

Costa Tsaousis (ktsaou) committed Jan 1, 2017 at 19:34 UTC d4c83d0cba04990c34d808641dc926b22181f82e

9 files changed +60 -44

src/appconfig.c

+12 -9

@@ -72,8 +72,8 @@ static int config_value_compare(void* a, void* b) {

72 else return strcmp(((struct config_value *)a)->name, ((struct config_value *)b)->name);

73 }

75 -#define config_value_index_add(co, cv) avl_insert_lock(&((co)->values_index), (avl *)(cv))

76 -#define config_value_index_del(co, cv) avl_remove_lock(&((co)->values_index), (avl *)(cv))

75 +#define config_value_index_add(co, cv) (struct config_value *)avl_insert_lock(&((co)->values_index), (avl *)(cv))

76 +#define config_value_index_del(co, cv) (struct config_value *)avl_remove_lock(&((co)->values_index), (avl *)(cv))

78 static struct config_value *config_value_index_find(struct config *co, const char *name, uint32_t hash) {

79 struct config_value tmp;

@@ -98,8 +98,8 @@ avl_tree_lock config_root_index = {

98 AVL_LOCK_INITIALIZER

99 };

100

101 -#define config_index_add(cfg) avl_insert_lock(&config_root_index, (avl *)(cfg))

102 -#define config_index_del(cfg) avl_remove_lock(&config_root_index, (avl *)(cfg))

101 +#define config_index_add(cfg) (struct config *)avl_insert_lock(&config_root_index, (avl *)(cfg))

102 +#define config_index_del(cfg) (struct config *)avl_remove_lock(&config_root_index, (avl *)(cfg))

103

104 static struct config *config_index_find(const char *name, uint32_t hash) {

105 struct config tmp;

@@ -127,7 +127,8 @@ static inline struct config *config_section_create(const char *section)

127

128 avl_init_lock(&co->values_index, config_value_compare);

129

130 - config_index_add(co);

130 + if(unlikely(config_index_add(co) != co))

131 + error("INTERNAL ERROR: indexing of section '%s', already exists.", co->name);

132

133 config_global_write_lock();

134 struct config *co2 = config_root;

@@ -154,7 +155,8 @@ static inline struct config_value *config_value_create(struct config *co, const

155 cv->hash = simple_hash(cv->name);

156 cv->value = strdupz(value);

157

157 - config_value_index_add(co, cv);

158 + if(unlikely(config_value_index_add(co, cv) != cv))

159 + error("INTERNAL ERROR: indexing of config '%s' in section '%s': already exists.", cv->name, co->name);

160

161 config_section_write_lock(co);

162 struct config_value *cv2 = co->values;

@@ -198,14 +200,15 @@ int config_rename(const char *section, const char *old, const char *new) {

200 cv2 = config_value_index_find(co, new, 0);

201 if(cv2) goto cleanup;

202

201 - config_value_index_del(co, cv);

203 + if(unlikely(config_value_index_del(co, cv) != cv))

204 + error("INTERNAL ERROR: deletion of config '%s' from section '%s', deleted tge wrong config entry.", cv->name, co->name);

205

206 freez(cv->name);

207 cv->name = strdupz(new);

205 -

208 cv->hash = simple_hash(cv->name);

209 + if(unlikely(config_value_index_add(co, cv) != cv))

210 + error("INTERNAL ERROR: indexing of config '%s' in section '%s', already exists.", cv->name, co->name);

211

208 - config_value_index_add(co, cv);

212 config_section_unlock(co);

213

214 return 0;

src/apps_plugin.c

+7 -3

@@ -1004,7 +1004,9 @@ static inline void file_descriptor_not_used(int id)

1004 if(unlikely(debug))

1005 fprintf(stderr, "apps.plugin: >> slot %d is empty.\n", id);

1006

1007 - file_descriptor_remove(&all_files[id]);

1007 + if(unlikely(file_descriptor_remove(&all_files[id]) != (void *)&all_files[id]))

1008 + error("INTERNAL ERROR: removal of unused fd from index, removed a different fd");

1009 +

1010 #ifdef NETDATA_INTERNAL_CHECKS

1011 all_files[id].magic = 0x00000000;

1012 #endif /* NETDATA_INTERNAL_CHECKS */

@@ -1056,7 +1058,8 @@ static inline int file_descriptor_find_or_add(const char *name)

1058 all_files_index.root = NULL;

1059 for(i = 0; i < all_files_size; i++) {

1060 if(!all_files[i].count) continue;

1059 - file_descriptor_add(&all_files[i]);

1061 + if(unlikely(file_descriptor_add(&all_files[i]) != (void *)&all_files[i]))

1062 + error("INTERNAL ERROR: duplicate indexing of fd during realloc.");

1063 }

1064

1065 if(unlikely(debug))

@@ -1145,7 +1148,8 @@ static inline int file_descriptor_find_or_add(const char *name)

1148 #ifdef NETDATA_INTERNAL_CHECKS

1149 all_files[c].magic = 0x0BADCAFE;

1150 #endif /* NETDATA_INTERNAL_CHECKS */

1148 - file_descriptor_add(&all_files[c]);

1151 + if(unlikely(file_descriptor_add(&all_files[c]) != (void *)&all_files[c]))

1152 + error("INTERNAL ERROR: duplicate indexing of fd.");

1153

1154 if(unlikely(debug))

1155 fprintf(stderr, "apps.plugin: using fd position %d (name: %s)\n", c, all_files[c].name);

src/avl.h

+4 -4

@@ -56,16 +56,16 @@ typedef struct avl_tree_lock {

56 * a is linked directly to the tree, so it has to

57 * be properly allocated by the caller.

58 */

59 -avl *avl_insert_lock(avl_tree_lock *t, avl *a) __attribute__((returns_nonnull));

60 -avl *avl_insert(avl_tree *t, avl *a) __attribute__((returns_nonnull));

59 +avl *avl_insert_lock(avl_tree_lock *t, avl *a) __attribute__((returns_nonnull, warn_unused_result));

60 +avl *avl_insert(avl_tree *t, avl *a) __attribute__((returns_nonnull, warn_unused_result));

62 /* Remove an element a from the AVL tree t

63 * returns a pointer to the removed element

64 * or NULL if an element equal to a is not found

65 * (equal as returned by t->compar())

66 */

67 -avl *avl_remove_lock(avl_tree_lock *t, avl *a);

68 -avl *avl_remove(avl_tree *t, avl *a);

67 +avl *avl_remove_lock(avl_tree_lock *t, avl *a) __attribute__((warn_unused_result));

68 +avl *avl_remove(avl_tree *t, avl *a) __attribute__((warn_unused_result));

70 /* Find the element into the tree that equal to a

71 * (equal as returned by t->compar())

src/dictionary.c

+7 -5

@@ -59,9 +59,6 @@ static int name_value_compare(void* a, void* b) {

59 else return strcmp(((NAME_VALUE *)a)->name, ((NAME_VALUE *)b)->name);

60 }

62 -#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)

63 -#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)

64 -

62 static inline NAME_VALUE *dictionary_name_value_index_find_nolock(DICTIONARY *dict, const char *name, uint32_t hash) {

63 NAME_VALUE tmp;

64 tmp.hash = (hash)?hash:simple_hash(name);

@@ -95,7 +92,10 @@ static NAME_VALUE *dictionary_name_value_create_nolock(DICTIONARY *dict, const c

92 }

94 // index it

98 - dictionary_name_value_index_add_nolock(dict, nv);

95 + NETDATA_DICTIONARY_STATS_INSERTS_PLUS1(dict);

96 + if(unlikely(avl_insert(&((dict)->values_index), (avl *)(nv)) != (avl *)nv))

97 + error("dictionary: INTERNAL ERROR: duplicate insertion to dictionary.");

98 +

99 NETDATA_DICTIONARY_STATS_ENTRIES_PLUS1(dict);

100

101 return nv;

@@ -104,7 +104,9 @@ static NAME_VALUE *dictionary_name_value_create_nolock(DICTIONARY *dict, const c

104 static void dictionary_name_value_destroy_nolock(DICTIONARY *dict, NAME_VALUE *nv) {

105 debug(D_DICTIONARY, "Destroying name value entry for name '%s'.", nv->name);

106

107 - dictionary_name_value_index_del_nolock(dict, nv);

107 + NETDATA_DICTIONARY_STATS_DELETES_PLUS1(dict);

108 + if(unlikely(avl_remove(&(dict->values_index), (avl *)(nv)) != (avl *)nv))

109 + error("dictionary: INTERNAL ERROR: dictionary invalid removal of node.");

110

111 NETDATA_DICTIONARY_STATS_ENTRIES_MINUS1(dict);

112

src/plugin_tc.c

+12 -8

@@ -100,8 +100,8 @@ avl_tree tc_device_root_index = {

100 tc_device_compare

101 };

102

103 -#define tc_device_index_add(st) avl_insert(&tc_device_root_index, (avl *)(st))

104 -#define tc_device_index_del(st) avl_remove(&tc_device_root_index, (avl *)(st))

103 +#define tc_device_index_add(st) (struct tc_device *)avl_insert(&tc_device_root_index, (avl *)(st))

104 +#define tc_device_index_del(st) (struct tc_device *)avl_remove(&tc_device_root_index, (avl *)(st))

105

106 static inline struct tc_device *tc_device_index_find(const char *id, uint32_t hash) {

107 struct tc_device tmp;

@@ -121,8 +121,8 @@ static int tc_class_compare(void* a, void* b) {

121 else return strcmp(((struct tc_class *)a)->id, ((struct tc_class *)b)->id);

122 }

123

124 -#define tc_class_index_add(st, rd) avl_insert(&((st)->classes_index), (avl *)(rd))

125 -#define tc_class_index_del(st, rd) avl_remove(&((st)->classes_index), (avl *)(rd))

124 +#define tc_class_index_add(st, rd) (struct tc_class *)avl_insert(&((st)->classes_index), (avl *)(rd))

125 +#define tc_class_index_del(st, rd) (struct tc_class *)avl_remove(&((st)->classes_index), (avl *)(rd))

126

127 static inline struct tc_class *tc_class_index_find(struct tc_device *st, const char *id, uint32_t hash) {

128 struct tc_class tmp;

@@ -146,7 +146,8 @@ static inline void tc_class_free(struct tc_device *n, struct tc_class *c) {

146

147 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);

148

149 - tc_class_index_del(n, c);

149 + if(unlikely(tc_class_index_del(n, c) != c))

150 + error("plugin_tc: INTERNAL ERROR: attempt remove class '%s' from device '%s': removed a different calls", c->id, n->id);

151

152 freez(c->id);

153 freez(c->name);

@@ -619,7 +620,8 @@ static inline struct tc_device *tc_device_create(char *id)

620 d->enabled = (char)-1;

621

622 avl_init(&d->classes_index, tc_class_compare);

622 - tc_device_index_add(d);

623 + if(unlikely(tc_device_index_add(d) != d))

624 + error("plugin_tc: INTERNAL ERROR: removing device '%s' removed a different device.", d->id);

625

626 if(!tc_device_root) {

627 tc_device_root = d;

@@ -660,7 +662,8 @@ static inline struct tc_class *tc_class_add(struct tc_device *n, char *id, char

662 c->leaf_hash = simple_hash(c->leafid);

663 }

664

663 - tc_class_index_add(n, c);

665 + if(unlikely(tc_class_index_add(n, c) != c))

666 + error("plugin_tc: INTERNAL ERROR: attempt index class '%s' on device '%s': already exists", c->id, n->id);

667 }

668

669 c->seen = 1;

@@ -677,7 +680,8 @@ static inline void tc_device_free(struct tc_device *n)

680 else tc_device_root = n->prev;

681 }

682

680 - tc_device_index_del(n);

683 + if(unlikely(tc_device_index_del(n) != n))

684 + error("plugin_tc: INTERNAL ERROR: removing device '%s' removed a different device.", n->id);

685

686 while(n->classes) tc_class_free(n, n->classes);

687

src/registry.h

-2

@@ -55,11 +55,9 @@

55 // should only happen when netdata starts

56 extern int registry_init(void);

58 -/*

58 // free all data held by the registry

59 // should only happen when netdata exits

60 extern void registry_free(void);

62 -*/

62 // HTTP requests handled by the registry

63 extern int registry_request_access_json(struct web_client *w, char *person_guid, char *machine_guid, char *url, char *name, time_t when);

src/registry_person.h

+1 -1

@@ -41,7 +41,7 @@ typedef struct registry_person REGISTRY_PERSON;

42 // PERSON_URL

43 extern REGISTRY_PERSON_URL *registry_person_url_index_find(REGISTRY_PERSON *p, const char *url);

44 -extern REGISTRY_PERSON_URL *registry_person_url_index_add(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) __attribute__((warn_unused_result));

44 +extern REGISTRY_PERSON_URL *registry_person_url_index_add(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) __attribute__((returns_nonnull, warn_unused_result));

45 extern REGISTRY_PERSON_URL *registry_person_url_index_del(REGISTRY_PERSON *p, REGISTRY_PERSON_URL *pu) __attribute__((warn_unused_result));

47 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);

src/registry_url.h

+2 -2

@@ -22,8 +22,8 @@ typedef struct registry_url REGISTRY_URL;

23 // REGISTRY_URL INDEX

24 extern int registry_url_compare(void *a, void *b);

25 -extern REGISTRY_URL *registry_url_index_del(REGISTRY_URL *u);

26 -extern REGISTRY_URL *registry_url_index_add(REGISTRY_URL *u) __attribute__((returns_nonnull));

25 +extern REGISTRY_URL *registry_url_index_del(REGISTRY_URL *u) __attribute__((warn_unused_result));

26 +extern REGISTRY_URL *registry_url_index_add(REGISTRY_URL *u) __attribute__((returns_nonnull, warn_unused_result));

28 // REGISTRY_URL MANAGEMENT

29 extern REGISTRY_URL *registry_url_get(const char *url, size_t urllen) __attribute__((returns_nonnull));

src/rrd.c

+15 -10

@@ -118,7 +118,7 @@ RRDFAMILY *rrdfamily_create(const char *id) {

118

119 RRDFAMILY *ret = rrdfamily_index_add(&localhost, rc);

120 if(ret != rc)

121 - fatal("INTERNAL ERROR: Expected to INSERT RRDFAMILY '%s' into index, but inserted '%s'.", rc->family, (ret)?ret->family:"NONE");

121 + fatal("RRDFAMILY: INTERNAL ERROR: Expected to INSERT RRDFAMILY '%s' into index, but inserted '%s'.", rc->family, (ret)?ret->family:"NONE");

122 }

123

124 rc->use_count++;

@@ -130,10 +130,10 @@ void rrdfamily_free(RRDFAMILY *rc) {

130 if(!rc->use_count) {

131 RRDFAMILY *ret = rrdfamily_index_del(&localhost, rc);

132 if(ret != rc)

133 - fatal("INTERNAL ERROR: Expected to DELETE RRDFAMILY '%s' from index, but deleted '%s'.", rc->family, (ret)?ret->family:"NONE");

133 + fatal("RRDFAMILY: INTERNAL ERROR: Expected to DELETE RRDFAMILY '%s' from index, but deleted '%s'.", rc->family, (ret)?ret->family:"NONE");

134

135 if(rc->variables_root_index.avl_tree.root != NULL)

136 - fatal("INTERNAL ERROR: Variables index of RRDFAMILY '%s' that is freed, is not empty.", rc->family);

136 + fatal("RRDFAMILY: INTERNAL ERROR: Variables index of RRDFAMILY '%s' that is freed, is not empty.", rc->family);

137

138 freez((void *)rc->family);

139 freez(rc);

@@ -222,8 +222,8 @@ static int rrddim_compare(void* a, void* b) {

222 else return strcmp(((RRDDIM *)a)->id, ((RRDDIM *)b)->id);

223 }

224

225 -#define rrddim_index_add(st, rd) avl_insert_lock(&((st)->dimensions_index), (avl *)(rd))

226 -#define rrddim_index_del(st,rd ) avl_remove_lock(&((st)->dimensions_index), (avl *)(rd))

225 +#define rrddim_index_add(st, rd) (RRDDIM *)avl_insert_lock(&((st)->dimensions_index), (avl *)(rd))

226 +#define rrddim_index_del(st,rd ) (RRDDIM *)avl_remove_lock(&((st)->dimensions_index), (avl *)(rd))

227

228 static RRDDIM *rrddim_index_find(RRDSET *st, const char *id, uint32_t hash) {

229 RRDDIM tmp;

@@ -381,7 +381,8 @@ void rrdset_set_name(RRDSET *st, const char *name)

381 rrddimvar_rename_all(rd);

382 pthread_rwlock_unlock(&st->rwlock);

383

384 - rrdset_index_add_name(&localhost, st);

384 + if(unlikely(rrdset_index_add_name(&localhost, st) != st))

385 + error("RRDSET: INTERNAL ERROR: attempted to index duplicate chart name '%s'", st->name);

386 }

387

388 // ----------------------------------------------------------------------------

@@ -634,7 +635,8 @@ RRDSET *rrdset_create(const char *type, const char *id, const char *name, const

635 rrdsetvar_create(st, "update_every", RRDVAR_TYPE_INT, &st->update_every, 0);

636 }

637

637 - rrdset_index_add(&localhost, st);

638 + if(unlikely(rrdset_index_add(&localhost, st) != st))

639 + error("RRDSET: INTERNAL ERROR: attempt to index duplicate chart '%s'", st->id);

640

641 rrdsetcalc_link_matching(st);

642 rrdcalctemplate_link_matching(st);

@@ -777,7 +779,8 @@ RRDDIM *rrddim_add(RRDSET *st, const char *id, const char *name, long multiplier

779

780 pthread_rwlock_unlock(&st->rwlock);

781

780 - rrddim_index_add(st, rd);

782 + if(unlikely(rrddim_index_add(st, rd) != rd))

783 + error("RRDDIM: INTERNAL ERROR: attempt to index duplicate dimension '%s' on chart '%s'", rd->id, st->id);

784

785 return(rd);

786 }

@@ -816,7 +819,8 @@ void rrddim_free(RRDSET *st, RRDDIM *rd)

819 while(rd->variables)

820 rrddimvar_free(rd->variables);

821

819 - rrddim_index_del(st, rd);

822 + if(unlikely(rrddim_index_del(st, rd) != rd))

823 + error("RRDDIM: INTERNAL ERROR: attempt to remove from index dimension '%s' on chart '%s', removed a different dimension.", rd->id, st->id);

824

825 // free(rd->annotations);

826 if(rd->mapped == RRD_MEMORY_MODE_SAVE) {

@@ -857,7 +861,8 @@ void rrdset_free_all(void)

861 while(st->dimensions)

862 rrddim_free(st, st->dimensions);

863

860 - rrdset_index_del(&localhost, st);

864 + if(unlikely(rrdset_index_del(&localhost, st) != st))

865 + error("RRDSET: INTERNAL ERROR: attempt to remove from index chart '%s', removed a different chart.", st->id);

866

867 st->rrdfamily->use_count--;

868 if(!st->rrdfamily->use_count)