Skip to content

Commit 1dab60d

Browse files
committed
Hash new implementation memleaks fixed.
1 parent 97ba4e3 commit 1dab60d

File tree

2 files changed

+52
-12
lines changed

2 files changed

+52
-12
lines changed

src/server.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1334,7 +1334,6 @@ void hashTypeConvert(robj *o, int enc);
13341334
void hashTypeTryConversion(robj *subject, robj **argv, int start, int end);
13351335
void hashTypeTryObjectEncoding(robj *subject, robj **o1, robj **o2);
13361336
int hashTypeExists(robj *o, sds key);
1337-
int hashTypeSet(robj *o, sds key, sds value);
13381337
int hashTypeDelete(robj *o, sds key);
13391338
unsigned long hashTypeLength(robj *o);
13401339
hashTypeIterator *hashTypeInitIterator(robj *subject);

src/t_hash.c

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,27 @@ int hashTypeExists(robj *o, sds field) {
179179
}
180180

181181
/* Add a new field, overwrite the old with the new value if it already exists.
182-
* Return 0 on insert and 1 on update. The key and value SDS strings are copied
183-
* if needed, so the caller retains ownership of the strings passed. */
184-
int hashTypeSet(robj *o, sds field, sds value) {
182+
* Return 0 on insert and 1 on update.
183+
*
184+
* By default, the key and value SDS strings are copied if needed, so the
185+
* caller retains ownership of the strings passed. However this behavior
186+
* can be effected by passing appropriate flags (possibly bitwise OR-ed):
187+
*
188+
* HASH_SET_TAKE_FIELD -- The SDS field ownership passes to the function.
189+
* HASH_SET_TAKE_VALUE -- The SDS value ownership passes to the function.
190+
*
191+
* When the flags are used the caller does not need to release the passed
192+
* SDS string(s). It's up to the function to use the string to create a new
193+
* entry or to free the SDS string before returning to the caller.
194+
*
195+
* HASH_SET_COPY corresponds to no flags passed, and means the default
196+
* semantics of copying the values if needed.
197+
*
198+
*/
199+
#define HASH_SET_TAKE_FIELD (1<<0)
200+
#define HASH_SET_TAKE_VALUE (1<<1)
201+
#define HASH_SET_COPY 0
202+
int hashTypeSet(robj *o, sds field, sds value, int flags) {
185203
int update = 0;
186204

187205
if (o->encoding == OBJ_ENCODING_ZIPLIST) {
@@ -222,14 +240,37 @@ int hashTypeSet(robj *o, sds field, sds value) {
222240
dictEntry *de = dictFind(o->ptr,field);
223241
if (de) {
224242
sdsfree(dictGetVal(de));
225-
dictGetVal(de) = sdsdup(value);
243+
if (flags & HASH_SET_TAKE_VALUE) {
244+
dictGetVal(de) = value;
245+
value = NULL;
246+
} else {
247+
dictGetVal(de) = sdsdup(value);
248+
}
226249
update = 1;
227250
} else {
228-
dictAdd(o->ptr,sdsdup(field),sdsdup(value));
251+
sds f,v;
252+
if (flags & HASH_SET_TAKE_FIELD) {
253+
f = field;
254+
field = NULL;
255+
} else {
256+
f = sdsdup(field);
257+
}
258+
if (flags & HASH_SET_TAKE_VALUE) {
259+
v = value;
260+
value = NULL;
261+
} else {
262+
v = sdsdup(value);
263+
}
264+
dictAdd(o->ptr,f,v);
229265
}
230266
} else {
231267
serverPanic("Unknown hash encoding");
232268
}
269+
270+
/* Free SDS strings we did not referenced elsewhere if the flags
271+
* want this function to be responsible. */
272+
if (flags & HASH_SET_TAKE_FIELD && field) sdsfree(field);
273+
if (flags & HASH_SET_TAKE_VALUE && value) sdsfree(value);
233274
return update;
234275
}
235276

@@ -476,7 +517,7 @@ void hsetCommand(client *c) {
476517

477518
if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return;
478519
hashTypeTryConversion(o,c->argv,2,3);
479-
update = hashTypeSet(o,c->argv[2]->ptr,c->argv[3]->ptr);
520+
update = hashTypeSet(o,c->argv[2]->ptr,c->argv[3]->ptr,HASH_SET_COPY);
480521
addReply(c, update ? shared.czero : shared.cone);
481522
signalModifiedKey(c->db,c->argv[1]);
482523
notifyKeyspaceEvent(NOTIFY_HASH,"hset",c->argv[1],c->db->id);
@@ -491,7 +532,7 @@ void hsetnxCommand(client *c) {
491532
if (hashTypeExists(o, c->argv[2]->ptr)) {
492533
addReply(c, shared.czero);
493534
} else {
494-
hashTypeSet(o,c->argv[2]->ptr,c->argv[3]->ptr);
535+
hashTypeSet(o,c->argv[2]->ptr,c->argv[3]->ptr,HASH_SET_COPY);
495536
addReply(c, shared.cone);
496537
signalModifiedKey(c->db,c->argv[1]);
497538
notifyKeyspaceEvent(NOTIFY_HASH,"hset",c->argv[1],c->db->id);
@@ -511,7 +552,7 @@ void hmsetCommand(client *c) {
511552
if ((o = hashTypeLookupWriteOrCreate(c,c->argv[1])) == NULL) return;
512553
hashTypeTryConversion(o,c->argv,2,c->argc-1);
513554
for (i = 2; i < c->argc; i += 2) {
514-
hashTypeSet(o,c->argv[i]->ptr,c->argv[i+1]->ptr);
555+
hashTypeSet(o,c->argv[i]->ptr,c->argv[i+1]->ptr,HASH_SET_COPY);
515556
}
516557
addReply(c, shared.ok);
517558
signalModifiedKey(c->db,c->argv[1]);
@@ -547,7 +588,7 @@ void hincrbyCommand(client *c) {
547588
}
548589
value += incr;
549590
new = sdsfromlonglong(value);
550-
hashTypeSet(o,c->argv[2]->ptr,new);
591+
hashTypeSet(o,c->argv[2]->ptr,new,HASH_SET_TAKE_VALUE);
551592
addReplyLongLong(c,value);
552593
signalModifiedKey(c->db,c->argv[1]);
553594
notifyKeyspaceEvent(NOTIFY_HASH,"hincrby",c->argv[1],c->db->id);
@@ -582,8 +623,8 @@ void hincrbyfloatCommand(client *c) {
582623
char buf[256];
583624
int len = ld2string(buf,sizeof(buf),value,1);
584625
new = sdsnewlen(buf,len);
585-
hashTypeSet(o,c->argv[2]->ptr,new);
586-
addReplyBulkSds(c,new);
626+
hashTypeSet(o,c->argv[2]->ptr,new,HASH_SET_TAKE_VALUE);
627+
addReplyBulkCBuffer(c,buf,len);
587628
signalModifiedKey(c->db,c->argv[1]);
588629
notifyKeyspaceEvent(NOTIFY_HASH,"hincrbyfloat",c->argv[1],c->db->id);
589630
server.dirty++;

0 commit comments

Comments
 (0)