COLLECTION & INI Cleanup

I started to cleanup the unit tests from the type cust around NULL and
found several problems that I had to address:
1) The choice of the "." as a search separator
turned out to be a poor choice.
The problem was that the file name has "."
and INI was relaying on files to be used as
property names.
I corrected that part in the INI but after discussion
with Simo we decided to switch from "." to "!"
as special symbol anyways.
2) Found that the property rename was not
reinitializing the hash. Corrected.
Added ticket to add unit tests around it (#83).

3 files changed, 68 insertions, 53 deletions

diff --git a/common/collection/collection.c b/common/collection/collection.c
index 31b47a05..d6fd872f 100644
--- a/common/collection/collection.c
+++ b/common/collection/collection.c
@@ -140,7 +140,8 @@ static int col_validate_property(const char *property)
 
     check = property;
     while (*check != '\0') {
-        if (((!isalnum(*check)) && (!ispunct(*check))) || (*check == '.')) {
+        /* It turned out that limiting collection charcters is bad */
+        if ((*check < ' ') || (*check == '!')) {
             invalid = 1;
             break;
         }
@@ -1165,7 +1166,7 @@ static int col_create_path_data(struct path_data **name_path,
     if(length > 0) {
         memcpy(new_name_path->name, name, length);
         new_name_path->length = length;
-        new_name_path->name[new_name_path->length] = '.';
+        new_name_path->name[new_name_path->length] = '!';
         new_name_path->length++;
         new_name_path->name[new_name_path->length] = '\0';
         TRACE_INFO_STRING("Name so far:", new_name_path->name);
@@ -1233,7 +1234,7 @@ static int col_match_item(struct collection_item *current,
 
             if (data_str == start) {
                 if (find_str > traverse_data->name_to_find) {
-                    if (*(find_str-1) == '.') {
+                    if (*(find_str-1) == '!') {
                         /* We matched the property but the search string is
                          * longer so we need to continue matching */
                         TRACE_INFO_STRING("col_match_item",
@@ -1254,7 +1255,7 @@ static int col_match_item(struct collection_item *current,
                 }
             }
             else if ((find_str == traverse_data->name_to_find) &&
-                     (*(data_str-1) == '.')) return COL_MATCH;
+                     (*(data_str-1) == '!')) return COL_MATCH;
 
             data_str--;
             find_str--;
@@ -1459,8 +1460,8 @@ static int col_walk_items(struct collection_item *ci,
 
 /* Find an item by property name and perform an action on it. */
 /* No pattern matching supported in the first implementation. */
-/* To refer to child properties use dotted notatation like this: */
-/* parent.child.subchild.subsubchild etc.  */
+/* To refer to child properties use notatation like this: */
+/* parent!child!subchild!subsubchild etc.  */
 static int col_find_item_and_do(struct collection_item *ci,
                                 const char *property_to_find,
                                 int type,
@@ -1475,7 +1476,7 @@ static int col_find_item_and_do(struct collection_item *ci,
     unsigned depth = 0;
     int count = 0;
     const char *last_part;
-    char *dot;
+    char *sep;
 
     TRACE_FLOW_STRING("col_find_item_and_do", "Entry.");
 
@@ -1511,18 +1512,18 @@ static int col_find_item_and_do(struct collection_item *ci,
 
         traverse_data->name_len_to_find = strlen(property_to_find);
 
-        /* Check if the search string ends with dot - this is ellegal */
-        if (traverse_data->name_to_find[traverse_data->name_len_to_find - 1] == '.') {
+        /* Check if the search string ends with "!" - this is illegal */
+        if (traverse_data->name_to_find[traverse_data->name_len_to_find - 1] == '!') {
             TRACE_ERROR_NUMBER("Search string is invalid.", EINVAL);
             free(traverse_data);
             return EINVAL;
         }
 
-        /* Find last dot if any */
-        dot = strrchr(traverse_data->name_to_find, '.');
-        if (dot != NULL) {
-            dot++;
-            last_part = dot;
+        /* Find last ! if any */
+        sep = strrchr(traverse_data->name_to_find, '!');
+        if (sep != NULL) {
+            sep++;
+            last_part = sep;
         }
         else last_part = traverse_data->name_to_find;
 
@@ -2600,6 +2601,17 @@ int col_modify_item(struct collection_item *item,
             TRACE_ERROR_STRING("Failed to allocate memory", "");
             return ENOMEM;
         }
+
+        /* Update property length and hash if we rename the property */
+        item->phash = FNV1a_base;
+        item->property_len = 0;
+
+        while (property[item->property_len] != 0) {
+            item->phash = item->phash ^ toupper(property[item->property_len]);
+            item->phash *= FNV1a_prime;
+            item->property_len++;
+        }
+
     }
 
     /* We need to change data ? */
diff --git a/common/collection/collection.h b/common/collection/collection.h
index 73ac7b34..6868cb6e 100644
--- a/common/collection/collection.h
+++ b/common/collection/collection.h
@@ -165,20 +165,20 @@ struct collection_iterator;
  * get_item function will return you the item that is your "wallet".
  * You can then change something or just get information about the item you
  * retrieved. But in most cases you do not the wallet itself. You want to get
- * something from the vallet or put something into it. IMO money would be an
+ * something from the wallet or put something into it. IMO money would be an
  * obvious choice. To do this you use update_xxx_property functions.
  * There might be a bag somewhere deep and you might want to add something to
  * it. add_xxx_property_xxx functions allow you to specify sub collection you
  * want the item to be added to. If this sub collection argument is NULL top
  * level collection is assumed.
- * The search in the collections users a dotted notation to refer to an item (or
+ * The search in the collections users a "x!y!z" notation to refer to an item (or
  * property). You can search for "wallet" and it will find any first instance of
  * the "wallet" in your luggage. But you might have two wallets. One is yours and
- * another is your significant other's. So you might say find "my.wallet".
+ * another is your significant other's. So you might say find "my!wallet".
  * It will find wallet in your bad (collection) named "my". This collection can
  * be many levels deep inside other collections. You do not need to know the
  * full path to get to it. But if you have the full path you can use the fill
- * path like this "luggage.newbags.my.wallet".
+ * path like this "luggage!newbags!my!wallet".
  * It is useful to be able to put bags into bags as well as get them out of each
  * other. When the collection is created the header keeps a reference count on
  * how many copies of the collection are known to the world. So one can put a
@@ -188,6 +188,9 @@ struct collection_iterator;
  * By extracting reference from an internal collection the caller gains access
  * to the collection directly and thus has responsibility to destroy it after
  * use.
+ * Characters with codes less than space in ASCII table are illegal for property
+ * names.
+ * Character '!' also illegal in property name and reserved for "x!y!z" notation.
  */
 
 /* Function that creates a named collection */
@@ -324,7 +327,7 @@ int col_set_timestamp(struct collection_item *ci,
 /* Update functions */
 /* All update functions search the property using the search algorithm
  * described at the top of the header file.
- * Use same dotted notation to specify a property.
+ * Use same "x!y" notation to specify a property.
  */
 /* Update a string property in the collection.
  * Length should include the terminating 0  */
@@ -450,7 +453,7 @@ int col_get_item(struct collection_item *ci,       /* Collection to find things
 /* Group of functions that allows retrieving individual elements of the collection_item
  * hiding the internal implementation.
  */
-const char *col_get_item_property(struct collection_item *ci,int *property_len);
+const char *col_get_item_property(struct collection_item *ci, int *property_len);
 int col_get_item_type(struct collection_item *ci);
 int col_get_item_length(struct collection_item *ci);
 void *col_get_item_data(struct collection_item *ci);
diff --git a/common/collection/collection_ut.c b/common/collection/collection_ut.c
index a3da2e20..6d913f9a 100644
--- a/common/collection/collection_ut.c
+++ b/common/collection/collection_ut.c
@@ -124,7 +124,7 @@ int single_collection_test(void)
         return error;
     }
 
-    error = col_add_str_property(handle, NULL, "property 1", "some data", 0);
+    error = col_add_str_property(handle, NULL, "property 1!", "some data", 0);
     if (error) printf("Expected error adding bad property to collection %d\n", error);
     else {
         printf("Expected error but got success\n");
@@ -362,7 +362,7 @@ int mixed_collection_test(void)
     col_debug_collection(socket1, COL_TRAVERSE_DEFAULT);
 
     printf("SOCKET1 MUST NO BE USED AFTER THIS POINT!!!\n");
-    socket1 = (struct collection_item *)(NULL);
+    socket1 = NULL;
 
     printf("Add collection PEER as PEER1 to subcollection SOCKET1 of the EVENT.\n");
 
@@ -414,7 +414,7 @@ int mixed_collection_test(void)
 
     /* Check if the property in the collection */
     found = 0;
-    error = col_is_item_in_collection(event, "peer1.delay", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
+    error = col_is_item_in_collection(event, "peer1!delay", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
     if (error) {
         col_destroy_collection(peer);
         col_destroy_collection(host);
@@ -428,13 +428,13 @@ int mixed_collection_test(void)
     else printf("Error property is not found!\n");
 
 
-    col_print_item(event, "peer1.IPv6");
-    col_print_item(event, "event.socket1.peer1.IPv6");
-    col_print_item(event, "event.peer1.IPv6");
-    col_print_item(event, "speer1.IPv6");
-    col_print_item(event, "eer1.IPv6");
-    col_print_item(event, ".peer1.IPv6");
-    col_print_item(event, "t.peer1.IPv6");
+    col_print_item(event, "peer1!IPv6");
+    col_print_item(event, "event!socket1!peer1!IPv6");
+    col_print_item(event, "event!peer1!IPv6");
+    col_print_item(event, "speer1!IPv6");
+    col_print_item(event, "eer1!IPv6");
+    col_print_item(event, "!peer1!IPv6");
+    col_print_item(event, "t!peer1!IPv6");
 
     /* Traverse collection */
     error = col_print_collection2(event);
@@ -443,9 +443,9 @@ int mixed_collection_test(void)
         return error;
     }
 
-    printf("Delete property PEER1.DELAY from the EVENT collection.\n");
+    printf("Delete property PEER1!DELAY from the EVENT collection.\n");
 
-    error = col_delete_property(event, "peer1.delay", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT);
+    error = col_delete_property(event, "peer1!delay", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT);
     if (error) {
         col_destroy_collection(peer);
         col_destroy_collection(host);
@@ -558,9 +558,9 @@ int iterator_test(void)
     struct collection_item *socket1;
     struct collection_item *socket2;
     struct collection_item *socket3;
-    struct collection_iterator *iterator = (struct collection_iterator *)(NULL);
+    struct collection_iterator *iterator = NULL;
     int error = EOK;
-    struct collection_item *item = (struct collection_item *)(NULL);
+    struct collection_item *item = NULL;
     char binary_dump[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8 };
     int depth = 0;
     int idepth = 0;
@@ -677,7 +677,7 @@ int iterator_test(void)
         }
 
         /* Are we done ? */
-        if (item == (struct collection_item *)(NULL)) break;
+        if (item == NULL) break;
 
         depth = 0;
         col_get_item_depth(iterator, &depth);
@@ -757,7 +757,7 @@ int iterator_test(void)
         }
 
         /* Are we done ? */
-        if (item == (struct collection_item *)(NULL)) break;
+        if (item == NULL) break;
 
         depth = 0;
         col_get_item_depth(iterator, &depth);
@@ -792,7 +792,7 @@ int iterator_test(void)
         }
 
         /* Are we done ? */
-        if (item == (struct collection_item *)(NULL)) break;
+        if (item == NULL) break;
 
         depth = 0;
         col_get_item_depth(iterator, &depth);
@@ -827,7 +827,7 @@ int iterator_test(void)
         }
 
         /* Are we done ? */
-        if (item == (struct collection_item *)(NULL)) break;
+        if (item == NULL) break;
 
         depth = 0;
         col_get_item_depth(iterator, &depth);
@@ -863,7 +863,7 @@ int iterator_test(void)
         }
 
         /* Are we done ? */
-        if (item == (struct collection_item *)(NULL)) break;
+        if (item == NULL) break;
 
         depth = 0;
         col_get_item_depth(iterator, &depth);
@@ -899,7 +899,7 @@ int iterator_test(void)
         }
 
         /* Are we done ? */
-        if (item == (struct collection_item *)(NULL)) break;
+        if (item == NULL) break;
 
         depth = 0;
         col_get_item_depth(iterator, &depth);
@@ -934,7 +934,7 @@ int iterator_test(void)
         }
 
         /* Are we done ? */
-        if (item == (struct collection_item *)(NULL)) break;
+        if (item == NULL) break;
 
         depth = 0;
         col_get_item_depth(iterator, &depth);
@@ -971,7 +971,7 @@ int iterator_test(void)
         }
 
         /* Are we done ? */
-        if (item == (struct collection_item *)(NULL)) break;
+        if (item == NULL) break;
 
         depth = 0;
         col_get_item_depth(iterator, &depth);
@@ -1015,7 +1015,7 @@ int insert_extract_test(void)
     struct collection_item *col;
     struct collection_item *col2;
     int error = EOK;
-    struct collection_item *item = (struct collection_item *)(NULL);
+    struct collection_item *item = NULL;
 
     printf("\n\n==== INSERTION TEST ====\n\n");
 
@@ -1279,7 +1279,7 @@ int search_test(void)
 
     col_debug_collection(level1, COL_TRAVERSE_DEFAULT);
 
-    error = col_is_item_in_collection(level1, "level1.level2.level3.level4.", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
+    error = col_is_item_in_collection(level1, "level1!level2!level3!level4!", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
     if (!error) {
         col_destroy_collection(level1);
         col_destroy_collection(level2);
@@ -1291,13 +1291,13 @@ int search_test(void)
 
     found = 0;
     error = 0;
-    error = col_is_item_in_collection(level1, "level1.level2.level3.level4.id", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
+    error = col_is_item_in_collection(level1, "level1!level2!level3!level4!id", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
     if ((error) || (!found)) {
         col_destroy_collection(level1);
         col_destroy_collection(level2);
         col_destroy_collection(level3);
         col_destroy_collection(level4);
-        printf("Failed to find item [level1.level2.level3.level4.id]. Error %d\n", error);
+        printf("Failed to find item [level1!level2!level3!level4!id]. Error %d\n", error);
         return error ? error : ENOENT;
     }
     else printf("Expected item is found\n");
@@ -1305,20 +1305,20 @@ int search_test(void)
 
     found = 0;
     error = 0;
-    error = col_is_item_in_collection(level1, "level3.level4.id", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
+    error = col_is_item_in_collection(level1, "level3!level4!id", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
     if ((error) || (!found)) {
         col_destroy_collection(level1);
         col_destroy_collection(level2);
         col_destroy_collection(level3);
         col_destroy_collection(level4);
-        printf("Failed to find item [level3.level4.id]. Error %d\n", error);
+        printf("Failed to find item [level3!level4!id]. Error %d\n", error);
         return error ? error : ENOENT;
     }
     else printf("Expected item is found\n");
 
     found = 0;
     error = 0;
-    error = col_is_item_in_collection(level1, "level3.packets", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
+    error = col_is_item_in_collection(level1, "level3!packets", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
     if ((error) || (!found)) {
         col_destroy_collection(level1);
         col_destroy_collection(level2);
@@ -1331,26 +1331,26 @@ int search_test(void)
 
     found = 0;
     error = 0;
-    error = col_is_item_in_collection(level1, "level1.level2.stack", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
+    error = col_is_item_in_collection(level1, "level1!level2!stack", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
     if ((error) || (!found)) {
         col_destroy_collection(level1);
         col_destroy_collection(level2);
         col_destroy_collection(level3);
         col_destroy_collection(level4);
-        printf("Failed to find item [level1.level2.stack]. Error %d\n", error);
+        printf("Failed to find item [level1!level2!stack]. Error %d\n", error);
         return error ? error : ENOENT;
     }
     else printf("Expected item is found\n");
 
     found = 0;
     error = 0;
-    error = col_is_item_in_collection(level1, "level1.level2.level3", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
+    error = col_is_item_in_collection(level1, "level1!level2!level3", COL_TYPE_ANY, COL_TRAVERSE_DEFAULT, &found);
     if ((error) || (!found)) {
         col_destroy_collection(level1);
         col_destroy_collection(level2);
         col_destroy_collection(level3);
         col_destroy_collection(level4);
-        printf("Failed to find item [level1.level2.level3]. Error %d\n", error);
+        printf("Failed to find item [level1!level2!level3]. Error %d\n", error);
         return error ? error : ENOENT;
     }
     else printf("Expected item is found\n");