Browse Source

Use convenience memory allocation functions

This fixes also issues found by ASAN:

- Leaked dictionaries.
- Read-after-free on dictionary deallocation.
- Missing zero-initialization.
Adrian Perez de Castro 6 years ago
parent
commit
9494feafd2
4 changed files with 44 additions and 41 deletions
  1. 16 13
      hipack-dict.c
  2. 5 3
      hipack-list.c
  3. 18 21
      hipack-parser.c
  4. 5 4
      hipack-string.c

+ 16 - 13
hipack-dict.c

@@ -36,8 +36,9 @@ static inline hipack_dict_node_t*
 make_node (hipack_string_t *key,
            const hipack_value_t  *value)
 {
-    hipack_dict_node_t *node = (hipack_dict_node_t*)
-        malloc (sizeof (hipack_dict_node_t));
+    assert (key->size);
+    hipack_dict_node_t *node =
+            hipack_alloc_bzero (sizeof (hipack_dict_node_t));
     memcpy (&node->value, value, sizeof (hipack_value_t));
     node->key = key;
     return node;
@@ -49,7 +50,7 @@ free_node (hipack_dict_node_t *node)
 {
     hipack_string_free (node->key);
     hipack_value_free (&node->value);
-    free (node);
+    hipack_alloc_free (node);
 }
 
 
@@ -72,9 +73,10 @@ rehash (hipack_dict_t *dict)
         node->next = NULL;
 
     dict->size *= HIPACK_DICT_RESIZE_FACTOR;
-    dict->nodes = (hipack_dict_node_t**)
-        realloc (dict->nodes, sizeof (hipack_dict_node_t*) * dict->size);
-    memset (dict->nodes, 0x00, sizeof (hipack_dict_node_t*) * dict->size);
+    dict->nodes = hipack_alloc_array (dict->nodes,
+                                      sizeof (hipack_dict_node_t*),
+                                      dict->size);
+    memset (dict->nodes, 0, sizeof (hipack_dict_node_t*) * dict->size);
 
     for (hipack_dict_node_t *node = dict->first; node; node = node->next_node) {
         uint32_t hash_val = hipack_string_hash (node->key) % (dict->size - 1);
@@ -96,12 +98,12 @@ rehash (hipack_dict_t *dict)
 hipack_dict_t*
 hipack_dict_new (void)
 {
-    hipack_dict_t *dict = (hipack_dict_t*)
-        malloc (sizeof (hipack_dict_t));
-    dict->count = 0;
+    hipack_dict_t *dict = hipack_alloc_bzero (sizeof (hipack_dict_t));
     dict->size  = HIPACK_DICT_DEFAULT_SIZE;
-    dict->nodes = (hipack_dict_node_t**)
-        malloc (sizeof (hipack_dict_node_t*) * dict->size);
+    dict->nodes = hipack_alloc_array (NULL,
+                                      sizeof (hipack_dict_node_t*),
+                                      dict->size);
+    memset (dict->nodes, 0, sizeof (hipack_dict_node_t*) * dict->size);
     return dict;
 }
 
@@ -111,7 +113,8 @@ hipack_dict_free (hipack_dict_t *dict)
 {
     if (dict) {
         free_all_nodes (dict);
-        free (dict);
+        hipack_alloc_free (dict->nodes);
+        hipack_alloc_free (dict);
     }
 }
 
@@ -141,8 +144,8 @@ void hipack_dict_set_adopt_key (hipack_dict_t        *dict,
     for (; node; node = node->next) {
         if (hipack_string_equal (node->key, *key)) {
             hipack_value_free (&node->value);
-            hipack_string_free (*key);
             memcpy (&node->value, value, sizeof (hipack_value_t));
+            hipack_string_free (*key);
             *key = NULL;
             return;
         }

+ 5 - 3
hipack-list.c

@@ -16,9 +16,11 @@ hipack_list_t*
 hipack_list_new (uint32_t size)
 {
     hipack_list_t *list;
+
     if (size) {
-        list = (hipack_list_t*) malloc (
-            sizeof (hipack_list_t) + size * sizeof (hipack_value_t));
+        list = hipack_alloc_array_extra (NULL, size,
+                                         sizeof (hipack_value_t),
+                                         sizeof (hipack_list_t));
         list->size = size;
     } else {
         list = &s_empty_list;
@@ -33,7 +35,7 @@ hipack_list_free (hipack_list_t *list)
     if (list && list != &s_empty_list) {
         for (uint32_t i = 0; i < list->size; i++)
             hipack_value_free (&list->data[i]);
-        free (list);
+        hipack_alloc_free (list);
     }
 }
 

+ 18 - 21
hipack-parser.c

@@ -44,7 +44,7 @@ struct parser {
 
 
 static hipack_value_t parse_value (P, S);
-static hipack_dict_t* parse_keyval_items (P, int eos, S);
+static void parse_keyval_items (P, hipack_dict_t *result, int eos, S);
 
 
 static inline bool
@@ -237,14 +237,15 @@ string_resize (hipack_string_t *hstr, uint32_t *alloc, uint32_t size)
         if (new_size != *alloc) {
             *alloc = new_size;
             new_size = sizeof (hipack_string_t) + new_size * sizeof (uint8_t);
-            hstr = (hipack_string_t*)
-                (hstr ? realloc (hstr, new_size) : malloc (new_size));
+            hstr = hipack_alloc_array_extra (hstr, new_size,
+                                             sizeof (uint8_t),
+                                             sizeof (hipack_string_t));
         }
         hstr->size = size;
     } else {
-        *alloc = 0;
-        free (hstr);
+        hipack_alloc_free (hstr);
         hstr = NULL;
+        *alloc = 0;
     }
     return hstr;
 }
@@ -262,15 +263,15 @@ list_resize (hipack_list_t *list, uint32_t *alloc, uint32_t size)
         }
         if (new_size != *alloc) {
             *alloc = new_size;
-            new_size = sizeof (hipack_list_t) + new_size * sizeof (hipack_value_t);
-            list = (hipack_list_t*)
-                (list ? realloc (list, new_size) : malloc (new_size));
+            list = hipack_alloc_array_extra (list, new_size,
+                                             sizeof (hipack_value_t),
+                                             sizeof (hipack_list_t));
         }
         list->size = size;
     } else {
-        *alloc = 0;
-        free (list);
+        hipack_alloc_free (list);
         list = NULL;
+        *alloc = 0;
     }
     return list;
 }
@@ -397,10 +398,10 @@ error:
 static hipack_value_t
 parse_dict (P, S)
 {
-    hipack_dict_t *dict = NULL;
+    hipack_dict_t *dict = hipack_dict_new ();
     matchchar (p, '{', NULL, CHECK_OK);
     skipwhite (p, CHECK_OK);
-    dict = parse_keyval_items (p, '}', CHECK_OK);
+    parse_keyval_items (p, dict, '}', CHECK_OK);
     matchchar (p, '}', "unterminated dict value", CHECK_OK);
     return (hipack_value_t) { .type = HIPACK_DICT, .v_dict = dict };
 
@@ -600,10 +601,9 @@ error:
 }
 
 
-static hipack_dict_t*
-parse_keyval_items (P, int eos, S)
+static void
+parse_keyval_items (P, hipack_dict_t *result, int eos, S)
 {
-    hipack_dict_t *result = hipack_dict_new ();
     hipack_value_t value = DUMMY_VALUE;
     hipack_string_t *key = NULL;
 
@@ -637,14 +637,11 @@ parse_keyval_items (P, int eos, S)
         }
         skipwhite (p, CHECK_OK);
     }
-
-    return result;
+    return;
 
 error:
     hipack_string_free (key);
     hipack_value_free (&value);
-    hipack_dict_free (result);
-    return NULL;
 }
 
 
@@ -664,10 +661,10 @@ parse_message (P, S)
         /* Input starts with a Dict marker. */
         nextchar (p, CHECK_OK);
         skipwhite (p, CHECK_OK);
-        result = parse_keyval_items (p, '}', CHECK_OK);
+        parse_keyval_items (p, result, '}', CHECK_OK);
         matchchar (p, '}', "unterminated message", CHECK_OK);
     } else {
-        result = parse_keyval_items (p, EOF, CHECK_OK);
+        parse_keyval_items (p, result, EOF, CHECK_OK);
     }
     return result;
 

+ 5 - 4
hipack-string.c

@@ -27,8 +27,8 @@ hipack_string_new_from_lstring (const char *str, uint32_t len)
 {
     assert (str);
     if (len > 0) {
-        hipack_string_t *hstr = (hipack_string_t*) malloc (
-            sizeof (hipack_list_t) + len * sizeof (uint8_t));
+        hipack_string_t *hstr = hipack_alloc_array_extra (NULL,
+                len, sizeof (uint8_t), sizeof (hipack_string_t));
         memcpy (hstr->data, str, len);
         hstr->size = len;
         return hstr;
@@ -54,8 +54,9 @@ hipack_string_copy (const hipack_string_t *hstr)
 void
 hipack_string_free (hipack_string_t *hstr)
 {
-    if (hstr && hstr != &s_empty_string)
-        free (hstr);
+    if (hstr && hstr != &s_empty_string) {
+        hipack_alloc_free (hstr);
+    }
 }