allow traversing null-value dictionaries (#13162)

* allow traversing null-value dictionaries

* fix lgtm report

* void the value too

* removed NEVERNULL directive

2 files changed, 45 insertions, 11 deletions

diff --git a/libnetdata/dictionary/dictionary.c b/libnetdata/dictionary/dictionary.c
index 5927807302..d999aab054 100644
--- a/libnetdata/dictionary/dictionary.c
+++ b/libnetdata/dictionary/dictionary.c
@@ -77,11 +77,12 @@ typedef struct name_value {
     struct name_value *next;    // a double linked list to allow fast insertions and deletions
     struct name_value *prev;
 
-    char *name;                 // the name of the dictionary item
-    void *value;                // the value of the dictionary item
-
     size_t name_len;            // the size of the name, including the terminating zero
     size_t value_len;           // the size of the value (assumed binary)
+
+    void *value;                // the value of the dictionary item
+    char *name;                 // the name of the dictionary item
+
 } NAME_VALUE;
 
 /*
@@ -173,7 +174,6 @@ size_t dictionary_stats_deletes(DICTIONARY *dict) {
 size_t dictionary_stats_resets(DICTIONARY *dict) {
     return dict->resets;
 }
-
 size_t dictionary_stats_walkthroughs(DICTIONARY *dict) {
     return dict->walkthroughs;
 }
@@ -515,7 +515,7 @@ static NAME_VALUE *namevalue_create_unsafe(DICTIONARY *dict, const char *name, s
             }
         }
         else {
-            // the caller want an item without any value
+            // the caller wants an item without any value
             nv->value = NULL;
         }
 
@@ -533,6 +533,8 @@ static NAME_VALUE *namevalue_create_unsafe(DICTIONARY *dict, const char *name, s
 static void namevalue_reset_unsafe(DICTIONARY *dict, NAME_VALUE *nv, void *value, size_t value_len) {
     debug(D_DICTIONARY, "Dictionary entry with name '%s' found. Changing its value.", nv->name);
 
+    DICTIONARY_STATS_VALUE_RESETS_PLUS1(dict, nv->value_len, value_len);
+
     if(dict->del_callback)
         dict->del_callback(nv->name, nv->value, dict->del_callback_data);
 
@@ -543,7 +545,6 @@ static void namevalue_reset_unsafe(DICTIONARY *dict, NAME_VALUE *nv, void *value
     }
     else {
         debug(D_DICTIONARY, "Dictionary: cloning value to '%s'", nv->name);
-        DICTIONARY_STATS_VALUE_RESETS_PLUS1(dict, nv->value_len, value_len);
 
         void *oldvalue = nv->value;
         void *newvalue = NULL;
@@ -982,6 +983,22 @@ static size_t dictionary_unittest_set_clone(DICTIONARY *dict, char **names, char
     return errors;
 }
 
+static size_t dictionary_unittest_set_null(DICTIONARY *dict, char **names, char **values, size_t entries) {
+    (void)values;
+    size_t errors = 0;
+    size_t i = 0;
+    for(; i < entries ;i++) {
+        void *val = dictionary_set(dict, names[i], NULL, 0);
+        if(val != NULL) { fprintf(stderr, ">>> %s() returns a non NULL value\n", __FUNCTION__); errors++; }
+    }
+    if(dictionary_stats_entries(dict) != i) {
+        fprintf(stderr, ">>> %s() dictionary items do not match\n", __FUNCTION__);
+        errors++;
+    }
+    return errors;
+}
+
+
 static size_t dictionary_unittest_set_nonclone(DICTIONARY *dict, char **names, char **values, size_t entries) {
     size_t errors = 0;
     for(size_t i = 0; i < entries ;i++) {
@@ -1264,7 +1281,11 @@ static size_t dictionary_unittest_sorted_walkthrough(DICTIONARY *dict, char **na
 static void dictionary_unittest_sorting(DICTIONARY *dict, char **names, char **values, size_t entries, size_t *errors) {
     dictionary_unittest_run_and_measure_time(dict, "adding entries", names, values, entries, errors, dictionary_unittest_set_clone);
     dictionary_unittest_run_and_measure_time(dict, "sorted walkthrough", names, values, entries, errors, dictionary_unittest_sorted_walkthrough);
-    dictionary_unittest_run_and_measure_time(dict, "destroying dictionary", names, values, entries, errors, dictionary_unittest_destroy);
+}
+
+static void dictionary_unittest_null_dfe(DICTIONARY *dict, char **names, char **values, size_t entries, size_t *errors) {
+    dictionary_unittest_run_and_measure_time(dict, "adding null value entries", names, values, entries, errors, dictionary_unittest_set_null);
+    dictionary_unittest_run_and_measure_time(dict, "traverse foreach read loop", names, values, entries, errors, dictionary_unittest_foreach);
 }
 
 int dictionary_unittest(size_t entries) {
@@ -1319,6 +1340,17 @@ int dictionary_unittest(size_t entries) {
     fprintf(stderr, "\nCreating dictionary single threaded, clone, %zu items\n", entries);
     dict = dictionary_create(DICTIONARY_FLAG_SINGLE_THREADED);
     dictionary_unittest_sorting(dict, names, values, entries, &errors);
+    dictionary_unittest_run_and_measure_time(dict, "destroying full dictionary", names, values, entries, &errors, dictionary_unittest_destroy);
+
+    fprintf(stderr, "\nCreating dictionary single threaded, clone, %zu items\n", entries);
+    dict = dictionary_create(DICTIONARY_FLAG_SINGLE_THREADED);
+    dictionary_unittest_null_dfe(dict, names, values, entries, &errors);
+    dictionary_unittest_run_and_measure_time(dict, "destroying full dictionary", names, values, entries, &errors, dictionary_unittest_destroy);
+
+    fprintf(stderr, "\nCreating dictionary single threaded, noclone, %zu items\n", entries);
+    dict = dictionary_create(DICTIONARY_FLAG_SINGLE_THREADED|DICTIONARY_FLAG_VALUE_LINK_DONT_CLONE);
+    dictionary_unittest_null_dfe(dict, names, values, entries, &errors);
+    dictionary_unittest_run_and_measure_time(dict, "destroying full dictionary", names, values, entries, &errors, dictionary_unittest_destroy);
 
     dictionary_unittest_free_char_pp(names, entries);
     dictionary_unittest_free_char_pp(values, entries);
diff --git a/libnetdata/dictionary/dictionary.h b/libnetdata/dictionary/dictionary.h
index ba0f92076c..ebe77134d1 100644
--- a/libnetdata/dictionary/dictionary.h
+++ b/libnetdata/dictionary/dictionary.h
@@ -84,7 +84,7 @@ extern size_t dictionary_destroy(DICTIONARY *dict);
 //
 // Passing NULL as value, the dictionary will callocz() the newly allocated value, otherwise it will copy it.
 // Passing 0 as value_len, the dictionary will set the value to NULL (no allocations for value will be made).
-extern void *dictionary_set(DICTIONARY *dict, const char *name, void *value, size_t value_len) NEVERNULL;
+extern void *dictionary_set(DICTIONARY *dict, const char *name, void *value, size_t value_len);
 
 // Get an item from the dictionary
 // If it returns NULL, the item is not found
@@ -166,12 +166,14 @@ typedef DICTFE_CONST struct dictionary_foreach {
 #define dfe_start_rw(dict, value, mode) \
         do { \
             DICTFE value ## _dfe = {};  \
-            const char *value ## _name; (void)(value ## _name); \
+            const char *value ## _name; (void)(value ## _name); (void)value; \
             for((value) = dictionary_foreach_start_rw(&value ## _dfe, (dict), (mode)), ( value ## _name ) = value ## _dfe.name; \
-                (value) ;\
-                (value) = dictionary_foreach_next(&value ## _dfe), ( value ## _name ) = value ## _dfe.name)
+                (value ## _dfe.name) ;\
+                (value) = dictionary_foreach_next(&value ## _dfe), ( value ## _name ) = value ## _dfe.name) \
+            {
 
 #define dfe_done(value) \
+            }           \
             dictionary_foreach_done(&value ## _dfe); \
         } while(0)