From 814be4f5bd62b4f66281879b3035a20ad84bb498 Mon Sep 17 00:00:00 2001 From: Henri Doreau Date: Tue, 22 Jan 2013 10:16:30 +0100 Subject: [PATCH 1/2] Made connect functions return NULL on alloc failures. Updated documentation and examples accordingly. --- README.md | 2 +- async.c | 20 ++++++++++++++++---- example.c | 9 +++++++-- hiredis.c | 42 ++++++++++++++++++++++++++++++++++++------ test.c | 5 ++++- 5 files changed, 64 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 62fe106..8d788f2 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,7 @@ After trying to connect to Redis using `redisConnect` you should check the `err` field to see if establishing the connection was successful: redisContext *c = redisConnect("127.0.0.1", 6379); - if (c->err) { + if (c != NULL && c->err) { printf("Error: %s\n", c->errstr); // handle error } diff --git a/async.c b/async.c index f65f869..b302d82 100644 --- a/async.c +++ b/async.c @@ -142,15 +142,27 @@ static void __redisAsyncCopyError(redisAsyncContext *ac) { } redisAsyncContext *redisAsyncConnect(const char *ip, int port) { - redisContext *c = redisConnectNonBlock(ip,port); - redisAsyncContext *ac = redisAsyncInitialize(c); + redisContext *c; + redisAsyncContext *ac; + + c = redisConnectNonBlock(ip,port); + if (c == NULL) + return NULL; + + ac = redisAsyncInitialize(c); __redisAsyncCopyError(ac); return ac; } redisAsyncContext *redisAsyncConnectUnix(const char *path) { - redisContext *c = redisConnectUnixNonBlock(path); - redisAsyncContext *ac = redisAsyncInitialize(c); + redisContext *c; + redisAsyncContext *ac; + + c = redisConnectUnixNonBlock(path); + if (c == NULL) + return NULL; + + ac = redisAsyncInitialize(c); __redisAsyncCopyError(ac); return ac; } diff --git a/example.c b/example.c index 378ef71..d9d7271 100644 --- a/example.c +++ b/example.c @@ -11,8 +11,13 @@ int main(void) { struct timeval timeout = { 1, 500000 }; // 1.5 seconds c = redisConnectWithTimeout((char*)"127.0.0.1", 6379, timeout); - if (c->err) { - printf("Connection error: %s\n", c->errstr); + if (c == NULL || c->err) { + if (c) { + printf("Connection error: %s\n", c->errstr); + redisFree(c); + } else { + printf("Connection error: can't allocate redis context\n"); + } exit(1); } diff --git a/hiredis.c b/hiredis.c index 4709ee3..b021105 100644 --- a/hiredis.c +++ b/hiredis.c @@ -1014,42 +1014,72 @@ void redisFree(redisContext *c) { * context will be set to the return value of the error function. * When no set of reply functions is given, the default set will be used. */ redisContext *redisConnect(const char *ip, int port) { - redisContext *c = redisContextInit(); + redisContext *c; + + c = redisContextInit(); + if (c == NULL) + return NULL; + c->flags |= REDIS_BLOCK; redisContextConnectTcp(c,ip,port,NULL); return c; } redisContext *redisConnectWithTimeout(const char *ip, int port, struct timeval tv) { - redisContext *c = redisContextInit(); + redisContext *c; + + c = redisContextInit(); + if (c == NULL) + return NULL; + c->flags |= REDIS_BLOCK; redisContextConnectTcp(c,ip,port,&tv); return c; } redisContext *redisConnectNonBlock(const char *ip, int port) { - redisContext *c = redisContextInit(); + redisContext *c; + + c = redisContextInit(); + if (c == NULL) + return NULL; + c->flags &= ~REDIS_BLOCK; redisContextConnectTcp(c,ip,port,NULL); return c; } redisContext *redisConnectUnix(const char *path) { - redisContext *c = redisContextInit(); + redisContext *c; + + c = redisContextInit(); + if (c == NULL) + return NULL; + c->flags |= REDIS_BLOCK; redisContextConnectUnix(c,path,NULL); return c; } redisContext *redisConnectUnixWithTimeout(const char *path, struct timeval tv) { - redisContext *c = redisContextInit(); + redisContext *c; + + c = redisContextInit(); + if (c == NULL) + return NULL; + c->flags |= REDIS_BLOCK; redisContextConnectUnix(c,path,&tv); return c; } redisContext *redisConnectUnixNonBlock(const char *path) { - redisContext *c = redisContextInit(); + redisContext *c; + + c = redisContextInit(); + if (c == NULL) + return NULL; + c->flags &= ~REDIS_BLOCK; redisContextConnectUnix(c,path,NULL); return c; diff --git a/test.c b/test.c index 737ad1c..6786003 100644 --- a/test.c +++ b/test.c @@ -88,7 +88,10 @@ static redisContext *connect(struct config config) { assert(NULL); } - if (c->err) { + if (c == NULL) { + printf("Connection error: can't allocate redis context\n"); + exit(1); + } else if (c->err) { printf("Connection error: %s\n", c->errstr); exit(1); } From d7e3268f48b457cb52336d264f8860b336faea9f Mon Sep 17 00:00:00 2001 From: Henri Doreau Date: Tue, 22 Jan 2013 15:53:17 +0100 Subject: [PATCH 2/2] Prevent AsyncConnect from crashing on memory allocation failures. --- async.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/async.c b/async.c index b302d82..83d88ec 100644 --- a/async.c +++ b/async.c @@ -102,7 +102,12 @@ static dictType callbackDict = { }; static redisAsyncContext *redisAsyncInitialize(redisContext *c) { - redisAsyncContext *ac = realloc(c,sizeof(redisAsyncContext)); + redisAsyncContext *ac; + + ac = realloc(c,sizeof(redisAsyncContext)); + if (ac == NULL) + return NULL; + c = &(ac->c); /* The regular connect functions will always set the flag REDIS_CONNECTED. @@ -150,6 +155,11 @@ redisAsyncContext *redisAsyncConnect(const char *ip, int port) { return NULL; ac = redisAsyncInitialize(c); + if (ac == NULL) { + redisFree(c); + return NULL; + } + __redisAsyncCopyError(ac); return ac; } @@ -194,6 +204,9 @@ static int __redisPushCallback(redisCallbackList *list, redisCallback *source) { /* Copy callback from stack to heap */ cb = malloc(sizeof(*cb)); + if (cb == NULL) + return REDIS_ERR_OOM; + if (source != NULL) { memcpy(cb,source,sizeof(*cb)); cb->next = NULL;