diff --git a/src/module.c b/src/module.c index b7108cd6d..8fdbf2ccb 100644 --- a/src/module.c +++ b/src/module.c @@ -2559,19 +2559,14 @@ int RM_StringTruncate(RedisModuleKey *key, size_t newlen) { } else { /* Unshare and resize. */ key->value = dbUnshareStringValue(key->db, key->key, key->value); - if (newlen == 0) { - sdsfree(key->value->ptr); - key->value->ptr = sdsempty(); - } else { - size_t curlen = sdslen(key->value->ptr); - if (newlen > curlen) { - key->value->ptr = sdsgrowzero(key->value->ptr,newlen); - } else if (newlen < curlen) { - sdsrange(key->value->ptr,0,newlen-1); - /* If the string is too wasteful, reallocate it. */ - if (sdslen(key->value->ptr) < sdsavail(key->value->ptr)) - key->value->ptr = sdsRemoveFreeSpace(key->value->ptr); - } + size_t curlen = sdslen(key->value->ptr); + if (newlen > curlen) { + key->value->ptr = sdsgrowzero(key->value->ptr,newlen); + } else if (newlen < curlen) { + sdssubstr(key->value->ptr,0,newlen); + /* If the string is too wasteful, reallocate it. */ + if (sdslen(key->value->ptr) < sdsavail(key->value->ptr)) + key->value->ptr = sdsRemoveFreeSpace(key->value->ptr); } } return REDISMODULE_OK; diff --git a/src/sds.c b/src/sds.c index 31ab2ed3a..036d714d4 100644 --- a/src/sds.c +++ b/src/sds.c @@ -778,6 +778,21 @@ sds sdstrim(sds s, const char *cset) { return s; } +/* Changes the input string to be a subset of the original. + * It does not release the free space in the string, so a call to + * sdsRemoveFreeSpace may be wise after. */ +void sdssubstr(sds s, size_t start, size_t len) { + /* Clamp out of range input */ + size_t oldlen = sdslen(s); + if (start >= oldlen) start = len = 0; + if (len > oldlen-start) len = oldlen-start; + + /* Move the data */ + if (len) memmove(s, s+start, len); + s[len] = 0; + sdssetlen(s,len); +} + /* Turn the string into a smaller (or equal) string containing only the * substring specified by the 'start' and 'end' indexes. * @@ -789,6 +804,11 @@ sds sdstrim(sds s, const char *cset) { * * The string is modified in-place. * + * NOTE: this function can be misleading and can have unexpected behaviour, + * specifically when you want the length of the new string to be 0. + * Having start==end will result in a string with one character. + * please consider using sdssubstr instead. + * * Example: * * s = sdsnew("Hello World"); @@ -796,28 +816,13 @@ sds sdstrim(sds s, const char *cset) { */ void sdsrange(sds s, ssize_t start, ssize_t end) { size_t newlen, len = sdslen(s); - if (len == 0) return; - if (start < 0) { - start = len+start; - if (start < 0) start = 0; - } - if (end < 0) { - end = len+end; - if (end < 0) end = 0; - } + if (start < 0) + start = len + start; + if (end < 0) + end = len + end; newlen = (start > end) ? 0 : (end-start)+1; - if (newlen != 0) { - if (start >= (ssize_t)len) { - newlen = 0; - } else if (end >= (ssize_t)len) { - end = len-1; - newlen = (end-start)+1; - } - } - if (start && newlen) memmove(s, s+start, newlen); - s[newlen] = 0; - sdssetlen(s,newlen); + sdssubstr(s, start, newlen); } /* Apply tolower() to every character of the sds string 's'. */ @@ -1372,6 +1377,18 @@ int sdsTest(int argc, char **argv, int accurate) { test_cond("sdsrange(...,100,100)", sdslen(y) == 0 && memcmp(y,"\0",1) == 0); + sdsfree(y); + y = sdsdup(x); + sdsrange(y,4,6); + test_cond("sdsrange(...,4,6)", + sdslen(y) == 0 && memcmp(y,"\0",1) == 0); + + sdsfree(y); + y = sdsdup(x); + sdsrange(y,3,6); + test_cond("sdsrange(...,3,6)", + sdslen(y) == 1 && memcmp(y,"o\0",2) == 0); + sdsfree(y); sdsfree(x); x = sdsnew("foo"); diff --git a/src/sds.h b/src/sds.h index 8392c2064..9b01b4e4e 100644 --- a/src/sds.h +++ b/src/sds.h @@ -238,6 +238,7 @@ sds sdscatprintf(sds s, const char *fmt, ...); sds sdscatfmt(sds s, char const *fmt, ...); sds sdstrim(sds s, const char *cset); +void sdssubstr(sds s, size_t start, size_t len); void sdsrange(sds s, ssize_t start, ssize_t end); void sdsupdatelen(sds s); void sdsclear(sds s); diff --git a/tests/modules/basics.c b/tests/modules/basics.c index 9786be1b9..2618f45b8 100644 --- a/tests/modules/basics.c +++ b/tests/modules/basics.c @@ -156,6 +156,7 @@ int TestUnlink(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { /* TEST.STRING.TRUNCATE -- Test truncating an existing string object. */ int TestStringTruncate(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + RedisModule_AutoMemory(ctx); REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argc); @@ -208,6 +209,7 @@ int TestStringTruncate(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) int NotifyCallback(RedisModuleCtx *ctx, int type, const char *event, RedisModuleString *key) { + RedisModule_AutoMemory(ctx); /* Increment a counter on the notifications: for each key notified we * increment a counter */ RedisModule_Log(ctx, "notice", "Got event type %d, event %s, key %s", type, @@ -219,6 +221,7 @@ int NotifyCallback(RedisModuleCtx *ctx, int type, const char *event, /* TEST.NOTIFICATIONS -- Test Keyspace Notifications. */ int TestNotifications(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) { + RedisModule_AutoMemory(ctx); REDISMODULE_NOT_USED(argv); REDISMODULE_NOT_USED(argc);