antirez/hiredis — Issue #65: Memory leaks and memory safe code.
Author: ornicum
Created: 2011-11-12 09:55:31+00:00
State: open

Hi all!

I have using hiredis with libev in my project written in C++. Well, I have observing memory leaks when sending to Redis PING-commands only. I have edit hiredis.c and async.c files, correct some errors and add memory-safe code. So, diff-files listing:

async.c.diff
46,47d45
< if (key == NULL)
< return 0;
52,53d49
< if (src == NULL)
< return NULL;
56,57d51
< if (dup == NULL)
< return NULL;
63,64d56
< if (key1 == NULL || key2== NULL)
< return 0;
75,76d66
< if (key == NULL)
< return;
82,83d71
< if (val == NULL)
< return;
86d73
< val = NULL;
100,101d86
< if (ac == NULL)
< return NULL;
135,136d119
< if (ac == NULL)
< return;
143,144d125
< if (ip == NULL)
< return NULL;
146,147d126
< if (c == NULL)
< return NULL;
149,150d127
< if (ac == NULL)
< return NULL;
156,157d132
< if (path == NULL)
< return NULL;
159,160d133
< if (c == NULL)
< return NULL;
162,163d134
< if (ac == NULL)
< return NULL;
169,170d139
< if (ac == NULL || fn == NULL)
< return REDIS_ERR;
184,185d152
< if (ac == NULL || fn == NULL)
< return REDIS_ERR;
195,196d161
< if (list == NULL || source == NULL)
< return REDIS_ERR;
201,202d165
< if (cb == NULL)
< return REDIS_ERR;
218,219d180
< if (list == NULL || target == NULL)
< return REDIS_ERR;
236,237d196
< if (ac == NULL || cb == NULL)
< return;
248,249d206
< if (ac != NULL)
< return;
298,299d254
< if (ac == NULL)
< return;
308,309d262
< if (ac == NULL)
< return;
336,337d288
< if (ac == NULL)
< return;
345,346d295
< if (ac == NULL || reply == NULL || dstcb == NULL)
< return REDIS_ERR;
394,395d342
< if (ac == NULL)
< return;
463,464d409
< if (ac == NULL)
< return;
477,478d421
< if (ac == NULL)
< return;
506,507d448
< if (start == NULL || str == NULL || len == NULL)
< return NULL;
525,526d465
< if (ac == NULL || fn == NULL || cmd == NULL)
< return REDIS_ERR;
587,588d525
< if (ac == NULL || fn == NULL || format == NULL)
< return -1;
599,600d535
< if (ac == NULL || fn == NULL || format == NULL)
< return -1;
610,611d544
< if (ac == NULL || fn == NULL)
< return -1;

hiredis.c.diff
62c62
< redisReply *r = calloc(1,sizeof(redisReply));
---
> redisReply *r = calloc(1,sizeof(*r));
73,74d72
< if (reply == NULL)
< return;
87d84
< r->element = NULL;
93c90
< if (r->str != NULL) {
---
> if (r->str != NULL)
95,96d91
< r->str = NULL;
< }
100d94
< r = NULL;
104,105d97
< if (task == NULL || str == NULL)
< return NULL;
138,139d129
< if (task == NULL)
< return NULL;
147c137
< r->element = calloc(elements,sizeof(redisReply));
---
> r->element = calloc(elements,sizeof(redisReply*));
165,166d154
< if (task == NULL)
< return NULL;
184,185d171
< if (task == NULL)
< return NULL;
201,202d186
< if (r == NULL || str == NULL)
< return;
223c207
< len = len < (strlen(r->errstr)-1) ? len : (strlen(r->errstr)-1);
---
> len = len < (sizeof(r->errstr)-1) ? len : (sizeof(r->errstr)-1);
229,230d212
< if (buf == NULL)
< return 0;
255,256d236
< if (r == NULL)
< return;
266,267d245
< if (r == NULL)
< return;
272,273d249
< if (r == NULL)
< return NULL;
285,286d260
< if (s == NULL)
< return NULL;
315,316d288
< if (s == NULL)
< return 0;
344,345d315
< if (r == NULL || _len == NULL)
< return NULL;
361,362d330
< if (r == NULL)
< return;
388,389d355
< if (r == NULL)
< return REDIS_ERR;
424,425d389
< if (r == NULL)
< return REDIS_ERR;
479,480d442
< if (r == NULL)
< return REDIS_ERR;
546,547d507
< if (r == NULL)
< return REDIS_ERR;
609d568
< r = NULL;
618,619d576
< if (r == NULL)
< return;
625d581
< r = NULL;
629,630d584
< if (r == NULL || buf == NULL)
< return REDIS_ERR;
663,664d616
< if (r == NULL)
< return REDIS_ERR;
894d845
< curargv = NULL;
902d852
< curargv = NULL;
904c854
< if (curarg != NULL) {
---
> if (curarg != NULL)
906,907d855
< curarg = NULL;
< }
911c859
< if (cmd != NULL) {
---
> if (cmd != NULL)
913,914d860
< cmd = NULL;
< }
932,933d877
< if (target == NULL || format == NULL)
< return 0;
948,949d891
< if (target == NULL || argv == NULL || argvlen == NULL)
< return 0;
984,985d925
< if (c == NULL)
< return;
991c931
< len = len < (strlen(c->errstr)-1) ? len : (strlen(c->errstr)-1);
---
> len = len < (sizeof(c->errstr)-1) ? len : (sizeof(c->errstr)-1);
997c937
< strerror_r(errno,c->errstr,strlen(c->errstr));
---
> strerror_r(errno,c->errstr,sizeof(c->errstr));
1016,1017d955
< if (c == NULL)
< return;
1025d962
< c = NULL;
1032,1033d968
< if (ip == NULL)
< return NULL;
1035,1036d969
< if (c == NULL)
< return NULL;
1043,1044d975
< if (ip == NULL)
< return NULL;
1046,1047d976
< if (c == NULL)
< return NULL;
1054,1055d982
< if (ip == NULL)
< return NULL;
1057,1058d983
< if (c == NULL)
< return NULL;
1065,1066d989
< if (path == NULL)
< return NULL;
1074,1075d996
< if (path == NULL)
< return NULL;
1083,1084d1003
< if (path == NULL)
< return NULL;
1093,1094d1011
< if (c == NULL)
< return REDIS_ERR;
1106,1107d1022
< if (c == NULL)
< return REDIS_ERR;
1145,1146d1059
< if (c == NULL || done == NULL)
< return REDIS_ERR;
1178,1179d1090
< if (c == NULL || reply == NULL)
< return REDIS_ERR;
1188,1189d1098
< if (c == NULL || reply == NULL)
< return REDIS_ERR;
1227,1228d1135
< if (c == NULL || cmd == NULL)
< return REDIS_ERR;
1242,1243d1148
< if (c == NULL || format == NULL)
< return REDIS_ERR;
1255d1159
< cmd = NULL;
1260d1163
< cmd = NULL;
1265,1266d1167
< if (c == NULL || format == NULL)
< return REDIS_ERR;
1277,1278d1177
< if (c == NULL || argv == NULL || argvlen == NULL)
< return REDIS_ERR;
1290d1188
< cmd = NULL;
1295d1192
< cmd = NULL;
1311,1312d1207
< if (c == NULL)
< return NULL;
1324,1325d1218
< if (c == NULL || format == NULL)
< return NULL;
1332,1333d1224
< if (c == NULL || format == NULL)
< return NULL;
1343,1344d1233
< if (c == NULL || argv == NULL || argvlen == NULL)
< return NULL;

And another thing. When I try to free PING-command reply, my program core dumped because memory is clean already but pointers is not NULL. Please, fix this.

Thanks!

Best regards,

Andrey Shestakov