Adjustments to recent RM_StringTruncate fix (#3718) (#9125)

- Introduce a new sdssubstr api as a building block for sdsrange.
  The API of sdsrange is many times hard to work with and also has
  corner case that cause bugs. sdsrange is easy to work with and also
  simplifies the implementation of sdsrange.
- Revert the fix to RM_StringTruncate and just use sdssubstr instead of
  sdsrange.
- Solve valgrind warnings from the new tests introduced by the previous
  PR.
This commit is contained in:
Oran Agra 2021-06-22 17:22:40 +03:00 committed by GitHub
parent a49b766860
commit ae418eca24
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 49 additions and 33 deletions

View File

@ -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;

View File

@ -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");

View File

@ -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);

View File

@ -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);