Safe allocations

README.md

@@ -15,7 +15,7 @@ Hiredis-cluster is a fork of Hiredis-vip, with the following improvements:
 * Using CMake as build system
 * Code style guide (using clang-format)
 * Improved testing
-* Memory leak corrections
+* Memory leak corrections and allocation failure handling
 
 ## Features
 

adlist.c

@@ -27,10 +27,11 @@
  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
  * POSSIBILITY OF SUCH DAMAGE.
  */
+#include <hiredis/alloc.h>
+#include <stdlib.h>
 
 #include "adlist.h"
 #include "hiutil.h"
-#include <stdlib.h>
 
 /* Create a new list. The created list can be freed with
  * AlFreeList(), but private value of every node need to be freed
@@ -40,7 +41,7 @@
 hilist *listCreate(void) {
     struct hilist *list;
 
-    if ((list = hi_alloc(sizeof(*list))) == NULL)
+    if ((list = hi_malloc(sizeof(*list))) == NULL)
         return NULL;
     list->head = list->tail = NULL;
     list->len = 0;
@@ -78,7 +79,7 @@ void listRelease(hilist *list) {
 hilist *listAddNodeHead(hilist *list, void *value) {
     listNode *node;
 
-    if ((node = hi_alloc(sizeof(*node))) == NULL)
+    if ((node = hi_malloc(sizeof(*node))) == NULL)
         return NULL;
     node->value = value;
     if (list->len == 0) {
@@ -103,7 +104,7 @@ hilist *listAddNodeHead(hilist *list, void *value) {
 hilist *listAddNodeTail(hilist *list, void *value) {
     listNode *node;
 
-    if ((node = hi_alloc(sizeof(*node))) == NULL)
+    if ((node = hi_malloc(sizeof(*node))) == NULL)
         return NULL;
     node->value = value;
     if (list->len == 0) {
@@ -123,7 +124,7 @@ hilist *listInsertNode(hilist *list, listNode *old_node, void *value,
                        int after) {
     listNode *node;
 
-    if ((node = hi_alloc(sizeof(*node))) == NULL)
+    if ((node = hi_malloc(sizeof(*node))) == NULL)
         return NULL;
     node->value = value;
     if (after) {
@@ -175,7 +176,7 @@ void listDelNode(hilist *list, listNode *node) {
 listIter *listGetIterator(hilist *list, int direction) {
     listIter *iter;
 
-    if ((iter = hi_alloc(sizeof(*iter))) == NULL)
+    if ((iter = hi_malloc(sizeof(*iter))) == NULL)
         return NULL;
     if (direction == AL_START_HEAD)
         iter->next = list->head;
@@ -280,6 +281,9 @@ listNode *listSearchKey(hilist *list, void *key) {
     listNode *node;
 
     iter = listGetIterator(list, AL_START_HEAD);
+    if (iter == NULL) {
+        return NULL;
+    }
     while ((node = listNext(iter)) != NULL) {
         if (list->match) {
             if (list->match(node->value, key)) {

command.c

@@ -1,5 +1,6 @@
 #include <ctype.h>
 #include <errno.h>
+#include <hiredis/alloc.h>
 
 #include "command.h"
 #include "hiarray.h"
@@ -1099,7 +1100,7 @@ void redis_parse_cmd(struct cmd *r) {
 
                 kpos = hiarray_push(r->keys);
                 if (kpos == NULL) {
-                    goto enomem;
+                    goto oom;
                 }
                 kpos->start = m;
                 kpos->end = p;
@@ -1587,37 +1588,34 @@ void redis_parse_cmd(struct cmd *r) {
     return;
 
 done:
-
     ASSERT(r->type > CMD_UNKNOWN && r->type < CMD_SENTINEL);
-
     r->result = CMD_PARSE_OK;
-
-    return;
-
-enomem:
-
-    r->result = CMD_PARSE_ENOMEM;
-
     return;
 
 error:
-
     r->result = CMD_PARSE_ERROR;
     errno = EINVAL;
     if (r->errstr == NULL) {
-        r->errstr = hi_alloc(100 * sizeof(*r->errstr));
+        r->errstr = hi_malloc(100 * sizeof(*r->errstr));
+        if (r->errstr == NULL) {
+            goto oom;
+        }
     }
 
     len = _scnprintf(
         r->errstr, 100,
         "Parse command error. Cmd type: %d, state: %d, break position: %d.",
         r->type, state, (int)(p - r->cmd));
     r->errstr[len] = '\0';
+    return;
+
+oom:
+    r->result = CMD_PARSE_ENOMEM;
 }
 
 struct cmd *command_get() {
     struct cmd *command;
-    command = hi_alloc(sizeof(struct cmd));
+    command = hi_malloc(sizeof(struct cmd));
     if (command == NULL) {
         return NULL;
     }
@@ -1655,25 +1653,26 @@ void command_destroy(struct cmd *command) {
 
     if (command->cmd != NULL) {
         hi_free(command->cmd);
+        command->cmd = NULL;
     }
 
     if (command->errstr != NULL) {
         hi_free(command->errstr);
+        command->errstr = NULL;
     }
 
     if (command->keys != NULL) {
         command->keys->nelem = 0;
         hiarray_destroy(command->keys);
+        command->keys = NULL;
     }
 
     if (command->frag_seq != NULL) {
         hi_free(command->frag_seq);
         command->frag_seq = NULL;
     }
 
-    if (command->reply != NULL) {
-        freeReplyObject(command->reply);
-    }
+    freeReplyObject(command->reply);
 
     if (command->sub_commands != NULL) {
         listRelease(command->sub_commands);

dict.c

@@ -33,11 +33,12 @@
  * POSSIBILITY OF SUCH DAMAGE.
  */
 
-#include "dict.h"
 #include <assert.h>
 #include <limits.h>
 #include <stdlib.h>
 
+#include "dict.h"
+
 /* -------------------------- private prototypes ---------------------------- */
 
 static int _dictExpandIfNeeded(dict *ht);
@@ -70,7 +71,10 @@ static void _dictReset(dict *ht) {
 
 /* Create a new hash table */
 static dict *dictCreate(dictType *type, void *privDataPtr) {
-    dict *ht = malloc(sizeof(*ht));
+    dict *ht = hi_malloc(sizeof(*ht));
+    if (ht == NULL)
+        return NULL;
+
     _dictInit(ht, type, privDataPtr);
     return ht;
 }
@@ -96,7 +100,9 @@ static int dictExpand(dict *ht, unsigned long size) {
     _dictInit(&n, ht->type, ht->privdata);
     n.size = realsize;
     n.sizemask = realsize - 1;
-    n.table = calloc(realsize, sizeof(dictEntry *));
+    n.table = hi_calloc(realsize, sizeof(dictEntry *));
+    if (n.table == NULL)
+        return DICT_ERR;
 
     /* Copy all the elements from the old to the new table:
      * note that if the old hash table is empty ht->size is zero,
@@ -124,7 +130,7 @@ static int dictExpand(dict *ht, unsigned long size) {
         }
     }
     assert(ht->used == 0);
-    free(ht->table);
+    hi_free(ht->table);
 
     /* Remap the new hashtable in the old */
     *ht = n;
@@ -142,7 +148,10 @@ static int dictAdd(dict *ht, void *key, void *val) {
         return DICT_ERR;
 
     /* Allocates the memory and stores key */
-    entry = malloc(sizeof(*entry));
+    entry = hi_malloc(sizeof(*entry));
+    if (entry == NULL)
+        return DICT_ERR;
+
     entry->next = ht->table[index];
     ht->table[index] = entry;
 
@@ -167,13 +176,13 @@ static int _dictClear(dict *ht) {
             nextHe = he->next;
             dictFreeEntryKey(ht, he);
             dictFreeEntryVal(ht, he);
-            free(he);
+            hi_free(he);
             ht->used--;
             he = nextHe;
         }
     }
     /* Free the table and the allocated cache structure */
-    free(ht->table);
+    hi_free(ht->table);
     /* Re-initialize the table */
     _dictReset(ht);
     return DICT_OK; /* never fails */
@@ -182,7 +191,7 @@ static int _dictClear(dict *ht) {
 /* Clear & Release the hash table */
 static void dictRelease(dict *ht) {
     _dictClear(ht);
-    free(ht);
+    hi_free(ht);
 }
 
 static dictEntry *dictFind(dict *ht, const void *key) {
@@ -202,7 +211,9 @@ static dictEntry *dictFind(dict *ht, const void *key) {
 }
 
 static dictIterator *dictGetIterator(dict *ht) {
-    dictIterator *iter = malloc(sizeof(*iter));
+    dictIterator *iter = hi_malloc(sizeof(*iter));
+    if (iter == NULL)
+        return NULL;
 
     iter->ht = ht;
     iter->index = -1;
@@ -231,7 +242,7 @@ static dictEntry *dictNext(dictIterator *iter) {
     return NULL;
 }
 
-static void dictReleaseIterator(dictIterator *iter) { free(iter); }
+static void dictReleaseIterator(dictIterator *iter) { hi_free(iter); }
 
 /* ------------------------- private functions ------------------------------ */
 

hiarray.c

@@ -1,3 +1,4 @@
+#include <hiredis/alloc.h>
 #include <stdlib.h>
 
 #include "hiarray.h"
@@ -8,12 +9,12 @@ struct hiarray *hiarray_create(uint32_t n, size_t size) {
 
     ASSERT(n != 0 && size != 0);
 
-    a = hi_alloc(sizeof(*a));
+    a = hi_malloc(sizeof(*a));
     if (a == NULL) {
         return NULL;
     }
 
-    a->elem = hi_alloc(n * size);
+    a->elem = hi_malloc(n * size);
     if (a->elem == NULL) {
         hi_free(a);
         return NULL;
@@ -31,27 +32,11 @@ void hiarray_destroy(struct hiarray *a) {
     hi_free(a);
 }
 
-int hiarray_init(struct hiarray *a, uint32_t n, size_t size) {
-    ASSERT(n != 0 && size != 0);
-
-    a->elem = hi_alloc(n * size);
-    if (a->elem == NULL) {
-        return HI_ENOMEM;
-    }
-
-    a->nelem = 0;
-    a->size = size;
-    a->nalloc = n;
-
-    return HI_OK;
-}
-
 void hiarray_deinit(struct hiarray *a) {
     ASSERT(a->nelem == 0);
 
-    if (a->elem != NULL) {
-        hi_free(a->elem);
-    }
+    hi_free(a->elem);
+    a->elem = NULL;
 }
 
 uint32_t hiarray_idx(struct hiarray *a, void *elem) {

hiarray.h

@@ -35,7 +35,6 @@ static inline uint32_t hiarray_n(const struct hiarray *a) { return a->nelem; }
 
 struct hiarray *hiarray_create(uint32_t n, size_t size);
 void hiarray_destroy(struct hiarray *a);
-int hiarray_init(struct hiarray *a, uint32_t n, size_t size);
 void hiarray_deinit(struct hiarray *a);
 
 uint32_t hiarray_idx(struct hiarray *a, void *elem);