From c139b6812d3ca292665a76fc32a44edffd87042e Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 9 Dec 2010 16:39:33 +0100 Subject: [PATCH 1/9] Add commands SETBIT/GETBIT --- src/redis.c | 2 ++ src/redis.h | 2 ++ src/sds.c | 26 ++++++++++++++ src/sds.h | 1 + src/t_string.c | 82 ++++++++++++++++++++++++++++++++++++++++++ tests/unit/basic.tcl | 84 ++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 197 insertions(+) diff --git a/src/redis.c b/src/redis.c index 5b39c011f..6d803269e 100644 --- a/src/redis.c +++ b/src/redis.c @@ -78,6 +78,8 @@ struct redisCommand readonlyCommandTable[] = { {"strlen",strlenCommand,2,0,NULL,1,1,1}, {"del",delCommand,-2,0,NULL,0,0,0}, {"exists",existsCommand,2,0,NULL,1,1,1}, + {"setbit",setbitCommand,4,REDIS_CMD_DENYOOM,NULL,1,1,1}, + {"getbit",getbitCommand,3,0,NULL,1,1,1}, {"incr",incrCommand,2,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"decr",decrCommand,2,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"mget",mgetCommand,-2,0,NULL,1,-1,1}, diff --git a/src/redis.h b/src/redis.h index e012db4c4..b1f992d3e 100644 --- a/src/redis.h +++ b/src/redis.h @@ -887,6 +887,8 @@ void setexCommand(redisClient *c); void getCommand(redisClient *c); void delCommand(redisClient *c); void existsCommand(redisClient *c); +void setbitCommand(redisClient *c); +void getbitCommand(redisClient *c); void incrCommand(redisClient *c); void decrCommand(redisClient *c); void incrbyCommand(redisClient *c); diff --git a/src/sds.c b/src/sds.c index 2d063c4a4..ff4772857 100644 --- a/src/sds.c +++ b/src/sds.c @@ -155,6 +155,32 @@ sds sdscpy(sds s, char *t) { return sdscpylen(s, t, strlen(t)); } +sds sdssetbit(sds s, size_t bit, int on) { + struct sdshdr *sh = (void*)(s-(sizeof(struct sdshdr))); + int byte = bit >> 3; + int reqlen = byte+1; + + if (reqlen > sh->len) { + size_t totlen; + + s = sdsMakeRoomFor(s,reqlen-sh->len); + if (s == NULL) return NULL; + sh = (void*)(s-(sizeof(struct sdshdr))); + + /* Make sure added region doesn't contain garbage */ + totlen = sh->len+sh->free; + memset(s+sh->len,0,sh->free+1); + sh->len = reqlen; + sh->free = totlen-sh->len; + } + + bit = 7 - (bit & 0x7); + on &= 0x1; + s[byte] |= on << bit; + s[byte] &= ~((!on) << bit); + return s; +} + sds sdscatvprintf(sds s, const char *fmt, va_list ap) { va_list cpy; char *buf, *t; diff --git a/src/sds.h b/src/sds.h index ae0f84fb5..61ef36b62 100644 --- a/src/sds.h +++ b/src/sds.h @@ -53,6 +53,7 @@ sds sdscatlen(sds s, void *t, size_t len); sds sdscat(sds s, char *t); sds sdscpylen(sds s, char *t, size_t len); sds sdscpy(sds s, char *t); +sds sdssetbit(sds s, size_t bit, int on); sds sdscatvprintf(sds s, const char *fmt, va_list ap); #ifdef __GNUC__ diff --git a/src/t_string.c b/src/t_string.c index 39ee506d5..4b6fe7920 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -1,3 +1,4 @@ +#include #include "redis.h" /*----------------------------------------------------------------------------- @@ -80,6 +81,87 @@ void getsetCommand(redisClient *c) { removeExpire(c->db,c->argv[1]); } +static int getBitOffsetFromArgument(redisClient *c, robj *o, size_t *offset) { + long long loffset; + char *err = "bit offset is not an integer or out of range"; + + if (getLongLongFromObjectOrReply(c,o,&loffset,err) != REDIS_OK) + return REDIS_ERR; + + /* Limit offset to SIZE_T_MAX or 1GB in bytes */ + if ((loffset < 0) || + ((unsigned long long)loffset >= (unsigned)SIZE_T_MAX) || + ((unsigned long long)loffset >> 3) >= (1024*1024*1024)) + { + addReplyError(c,err); + return REDIS_ERR; + } + + *offset = (size_t)loffset; + return REDIS_OK; +} + +void setbitCommand(redisClient *c) { + robj *o; + size_t bitoffset; + int on; + + if (getBitOffsetFromArgument(c,c->argv[2],&bitoffset) != REDIS_OK) + return; + + on = ((sds)c->argv[3]->ptr)[0] - '0'; + if (sdslen(c->argv[3]->ptr) != 1 || (on & ~1)) { + addReplyError(c,"bit should be 0 or 1"); + return; + } + + o = lookupKeyWrite(c->db,c->argv[1]); + if (o == NULL) { + sds value = sdssetbit(sdsempty(),bitoffset,on); + o = createObject(REDIS_STRING,value); + dbAdd(c->db,c->argv[1],o); + } else { + if (checkType(c,o,REDIS_STRING)) return; + + /* Create a copy when the object is shared or encoded. */ + if (o->refcount != 1 || o->encoding != REDIS_ENCODING_RAW) { + robj *decoded = getDecodedObject(o); + o = createStringObject(decoded->ptr, sdslen(decoded->ptr)); + decrRefCount(decoded); + dbReplace(c->db,c->argv[1],o); + } + + o->ptr = sdssetbit(o->ptr,bitoffset,on); + } + touchWatchedKey(c->db,c->argv[1]); + server.dirty++; + addReply(c,shared.cone); +} + +void getbitCommand(redisClient *c) { + robj *o; + size_t bitoffset, byte, bitmask; + int on = 0; + char llbuf[32]; + + if (getBitOffsetFromArgument(c,c->argv[2],&bitoffset) != REDIS_OK) + return; + + if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL || + checkType(c,o,REDIS_STRING)) return; + + byte = bitoffset >> 3; + bitmask = 1 << (7 - (bitoffset & 0x7)); + if (o->encoding != REDIS_ENCODING_RAW) { + if (byte < (size_t)ll2string(llbuf,sizeof(llbuf),(long)o->ptr)) + on = llbuf[byte] & bitmask; + } else { + if (byte < sdslen(o->ptr)) + on = ((sds)o->ptr)[byte] & bitmask; + } + addReply(c, on ? shared.cone : shared.czero); +} + void mgetCommand(redisClient *c) { int j; diff --git a/tests/unit/basic.tcl b/tests/unit/basic.tcl index 4c6662c67..3032cca90 100644 --- a/tests/unit/basic.tcl +++ b/tests/unit/basic.tcl @@ -374,4 +374,88 @@ start_server {tags {"basic"}} { r set mystring "foozzz0123456789 baz" r strlen mystring } + + test "SETBIT against non-existing key" { + r del mykey + + # Setting 2nd bit to on is integer 64, ascii "@" + assert_equal 1 [r setbit mykey 1 1] + assert_equal "@" [r get mykey] + } + + test "SETBIT against string-encoded key" { + # Single byte with 2nd bit set + r set mykey "@" + + # 64 + 32 = 96 => ascii "`" (backtick) + assert_equal 1 [r setbit mykey 2 1] + assert_equal "`" [r get mykey] + } + + test "SETBIT against integer-encoded key" { + r set mykey 1 + assert_encoding int mykey + + # Ascii "1" is integer 49 = 00 11 00 01 + # Setting 7th bit = 51 => ascii "3" + assert_equal 1 [r setbit mykey 6 1] + assert_equal "3" [r get mykey] + } + + test "SETBIT against key with wrong type" { + r del mykey + r lpush mykey "foo" + assert_error "*wrong kind*" {r setbit mykey 0 1} + } + + test "SETBIT with out of range bit offset" { + r del mykey + assert_error "*out of range*" {r setbit mykey [expr 8*1024*1024*1024] 1} + assert_error "*out of range*" {r setbit mykey -1 1} + } + + test "SETBIT with non-bit argument" { + r del mykey + assert_error "*0 or 1*" {r setbit mykey 0 -1} + assert_error "*0 or 1*" {r setbit mykey 0 2} + assert_error "*0 or 1*" {r setbit mykey 0 10} + assert_error "*0 or 1*" {r setbit mykey 0 01} + } + + test "GETBIT against non-existing key" { + r del mykey + assert_equal 0 [r getbit mykey 0] + } + + test "GETBIT against string-encoded key" { + # Single byte with 2nd and 3rd bit set + r set mykey "`" + + # In-range + assert_equal 0 [r getbit mykey 0] + assert_equal 1 [r getbit mykey 1] + assert_equal 1 [r getbit mykey 2] + assert_equal 0 [r getbit mykey 3] + + # Out-range + assert_equal 0 [r getbit mykey 8] + assert_equal 0 [r getbit mykey 100] + assert_equal 0 [r getbit mykey 10000] + } + + test "GETBIT against integer-encoded key" { + r set mykey 1 + assert_encoding int mykey + + # Ascii "1" is integer 49 = 00 11 00 01 + assert_equal 0 [r getbit mykey 0] + assert_equal 0 [r getbit mykey 1] + assert_equal 1 [r getbit mykey 2] + assert_equal 1 [r getbit mykey 3] + + # Out-range + assert_equal 0 [r getbit mykey 8] + assert_equal 0 [r getbit mykey 100] + assert_equal 0 [r getbit mykey 10000] + } } From 73dbf7c9421ad5de7a6f327e210c8a6f7858844e Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 9 Dec 2010 17:16:10 +0100 Subject: [PATCH 2/9] Enforce maximum string value length of 512MB --- src/t_string.c | 30 +++++++++++++++++------------- tests/unit/basic.tcl | 2 +- 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 4b6fe7920..b77bcf0a0 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -91,7 +91,7 @@ static int getBitOffsetFromArgument(redisClient *c, robj *o, size_t *offset) { /* Limit offset to SIZE_T_MAX or 1GB in bytes */ if ((loffset < 0) || ((unsigned long long)loffset >= (unsigned)SIZE_T_MAX) || - ((unsigned long long)loffset >> 3) >= (1024*1024*1024)) + ((unsigned long long)loffset >> 3) >= (512*1024*1024)) { addReplyError(c,err); return REDIS_ERR; @@ -263,7 +263,7 @@ void decrbyCommand(redisClient *c) { void appendCommand(redisClient *c) { int retval; size_t totlen; - robj *o; + robj *o, *append; o = lookupKeyWrite(c->db,c->argv[1]); c->argv[2] = tryObjectEncoding(c->argv[2]); @@ -277,23 +277,27 @@ void appendCommand(redisClient *c) { addReply(c,shared.wrongtypeerr); return; } - /* If the object is specially encoded or shared we have to make - * a copy */ + + append = getDecodedObject(c->argv[2]); + if (o->encoding == REDIS_ENCODING_RAW && + (sdslen(o->ptr) + sdslen(append->ptr)) > 512*1024*1024) + { + addReplyError(c,"string exceeds maximum allowed size (512MB)"); + decrRefCount(append); + return; + } + + /* If the object is shared or encoded, we have to make a copy */ if (o->refcount != 1 || o->encoding != REDIS_ENCODING_RAW) { robj *decoded = getDecodedObject(o); - o = createStringObject(decoded->ptr, sdslen(decoded->ptr)); decrRefCount(decoded); dbReplace(c->db,c->argv[1],o); } - /* APPEND! */ - if (c->argv[2]->encoding == REDIS_ENCODING_RAW) { - o->ptr = sdscatlen(o->ptr, - c->argv[2]->ptr, sdslen(c->argv[2]->ptr)); - } else { - o->ptr = sdscatprintf(o->ptr, "%ld", - (unsigned long) c->argv[2]->ptr); - } + + /* Append the value */ + o->ptr = sdscatlen(o->ptr,append->ptr,sdslen(append->ptr)); + decrRefCount(append); totlen = sdslen(o->ptr); } touchWatchedKey(c->db,c->argv[1]); diff --git a/tests/unit/basic.tcl b/tests/unit/basic.tcl index 3032cca90..8b1512f3a 100644 --- a/tests/unit/basic.tcl +++ b/tests/unit/basic.tcl @@ -410,7 +410,7 @@ start_server {tags {"basic"}} { test "SETBIT with out of range bit offset" { r del mykey - assert_error "*out of range*" {r setbit mykey [expr 8*1024*1024*1024] 1} + assert_error "*out of range*" {r setbit mykey [expr 4*1024*1024*1024] 1} assert_error "*out of range*" {r setbit mykey -1 1} } From 09586f8b7a79bda7b938db20d7c165668e7800a0 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 10 Dec 2010 11:58:14 +0100 Subject: [PATCH 3/9] Add generic function to grow an sds value Move logic concerned with setting a bit in an sds to the SETBIT command instead of keeping it in sds.c. The function to grow an sds can and will be reused for a command to set a range within a string value. --- src/sds.c | 45 +++++++++++++++++++------------------------- src/sds.h | 2 +- src/t_string.c | 26 +++++++++++++++++-------- tests/unit/basic.tcl | 8 ++++---- 4 files changed, 42 insertions(+), 39 deletions(-) diff --git a/src/sds.c b/src/sds.c index ff4772857..de6201910 100644 --- a/src/sds.c +++ b/src/sds.c @@ -116,6 +116,25 @@ static sds sdsMakeRoomFor(sds s, size_t addlen) { return newsh->buf; } +/* Grow the sds to have the specified length. Bytes that were not part of + * the original length of the sds will be set to NULL. */ +sds sdsgrowsafe(sds s, size_t len) { + struct sdshdr *sh = (void*)(s-(sizeof(struct sdshdr))); + size_t totlen, curlen = sh->len; + + if (len <= curlen) return s; + s = sdsMakeRoomFor(s,len-curlen); + if (s == NULL) return NULL; + + /* Make sure added region doesn't contain garbage */ + sh = (void*)(s-(sizeof(struct sdshdr))); + memset(s+curlen,0,(len-curlen+1)); /* also set trailing NULL byte */ + totlen = sh->len+sh->free; + sh->len = len; + sh->free = totlen-sh->len; + return s; +} + sds sdscatlen(sds s, void *t, size_t len) { struct sdshdr *sh; size_t curlen = sdslen(s); @@ -155,32 +174,6 @@ sds sdscpy(sds s, char *t) { return sdscpylen(s, t, strlen(t)); } -sds sdssetbit(sds s, size_t bit, int on) { - struct sdshdr *sh = (void*)(s-(sizeof(struct sdshdr))); - int byte = bit >> 3; - int reqlen = byte+1; - - if (reqlen > sh->len) { - size_t totlen; - - s = sdsMakeRoomFor(s,reqlen-sh->len); - if (s == NULL) return NULL; - sh = (void*)(s-(sizeof(struct sdshdr))); - - /* Make sure added region doesn't contain garbage */ - totlen = sh->len+sh->free; - memset(s+sh->len,0,sh->free+1); - sh->len = reqlen; - sh->free = totlen-sh->len; - } - - bit = 7 - (bit & 0x7); - on &= 0x1; - s[byte] |= on << bit; - s[byte] &= ~((!on) << bit); - return s; -} - sds sdscatvprintf(sds s, const char *fmt, va_list ap) { va_list cpy; char *buf, *t; diff --git a/src/sds.h b/src/sds.h index 61ef36b62..44880183f 100644 --- a/src/sds.h +++ b/src/sds.h @@ -49,11 +49,11 @@ size_t sdslen(const sds s); sds sdsdup(const sds s); void sdsfree(sds s); size_t sdsavail(sds s); +sds sdsgrowsafe(sds s, size_t len); sds sdscatlen(sds s, void *t, size_t len); sds sdscat(sds s, char *t); sds sdscpylen(sds s, char *t, size_t len); sds sdscpy(sds s, char *t); -sds sdssetbit(sds s, size_t bit, int on); sds sdscatvprintf(sds s, const char *fmt, va_list ap); #ifdef __GNUC__ diff --git a/src/t_string.c b/src/t_string.c index b77bcf0a0..b41a1522b 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -103,22 +103,26 @@ static int getBitOffsetFromArgument(redisClient *c, robj *o, size_t *offset) { void setbitCommand(redisClient *c) { robj *o; + char *err = "bit is not an integer or out of range"; size_t bitoffset; - int on; + long long bitvalue; + int byte, bit, on; if (getBitOffsetFromArgument(c,c->argv[2],&bitoffset) != REDIS_OK) return; - on = ((sds)c->argv[3]->ptr)[0] - '0'; - if (sdslen(c->argv[3]->ptr) != 1 || (on & ~1)) { - addReplyError(c,"bit should be 0 or 1"); + if (getLongLongFromObjectOrReply(c,c->argv[3],&bitvalue,err) != REDIS_OK) + return; + + /* A bit can only be set to be on or off... */ + if (bitvalue & ~1) { + addReplyError(c,err); return; } o = lookupKeyWrite(c->db,c->argv[1]); if (o == NULL) { - sds value = sdssetbit(sdsempty(),bitoffset,on); - o = createObject(REDIS_STRING,value); + o = createObject(REDIS_STRING,sdsempty()); dbAdd(c->db,c->argv[1],o); } else { if (checkType(c,o,REDIS_STRING)) return; @@ -130,9 +134,15 @@ void setbitCommand(redisClient *c) { decrRefCount(decoded); dbReplace(c->db,c->argv[1],o); } - - o->ptr = sdssetbit(o->ptr,bitoffset,on); } + + byte = bitoffset >> 3; + bit = 7 - (bitoffset & 0x7); + on = bitvalue & 0x1; + o->ptr = sdsgrowsafe(o->ptr,byte+1); + ((char*)o->ptr)[byte] |= on << bit; + ((char*)o->ptr)[byte] &= ~((!on) << bit); + touchWatchedKey(c->db,c->argv[1]); server.dirty++; addReply(c,shared.cone); diff --git a/tests/unit/basic.tcl b/tests/unit/basic.tcl index 8b1512f3a..1324246f1 100644 --- a/tests/unit/basic.tcl +++ b/tests/unit/basic.tcl @@ -416,10 +416,10 @@ start_server {tags {"basic"}} { test "SETBIT with non-bit argument" { r del mykey - assert_error "*0 or 1*" {r setbit mykey 0 -1} - assert_error "*0 or 1*" {r setbit mykey 0 2} - assert_error "*0 or 1*" {r setbit mykey 0 10} - assert_error "*0 or 1*" {r setbit mykey 0 01} + assert_error "*out of range*" {r setbit mykey 0 -1} + assert_error "*out of range*" {r setbit mykey 0 2} + assert_error "*out of range*" {r setbit mykey 0 10} + assert_error "*out of range*" {r setbit mykey 0 20} } test "GETBIT against non-existing key" { From cb33445bcd710232bd6dbf0e8c76a9bbefd02b37 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 10 Dec 2010 12:06:24 +0100 Subject: [PATCH 4/9] Typo --- src/t_string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/t_string.c b/src/t_string.c index b41a1522b..dcb37e553 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -88,7 +88,7 @@ static int getBitOffsetFromArgument(redisClient *c, robj *o, size_t *offset) { if (getLongLongFromObjectOrReply(c,o,&loffset,err) != REDIS_OK) return REDIS_ERR; - /* Limit offset to SIZE_T_MAX or 1GB in bytes */ + /* Limit offset to SIZE_T_MAX or 512MB in bytes */ if ((loffset < 0) || ((unsigned long long)loffset >= (unsigned)SIZE_T_MAX) || ((unsigned long long)loffset >> 3) >= (512*1024*1024)) From cc8322919a3325238e41403c67f765cf07404710 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 10 Dec 2010 12:16:16 +0100 Subject: [PATCH 5/9] Change function name to match what it does --- src/sds.c | 6 +++--- src/sds.h | 2 +- src/t_string.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/sds.c b/src/sds.c index de6201910..da049f6ce 100644 --- a/src/sds.c +++ b/src/sds.c @@ -117,8 +117,8 @@ static sds sdsMakeRoomFor(sds s, size_t addlen) { } /* Grow the sds to have the specified length. Bytes that were not part of - * the original length of the sds will be set to NULL. */ -sds sdsgrowsafe(sds s, size_t len) { + * the original length of the sds will be set to zero. */ +sds sdsgrowzero(sds s, size_t len) { struct sdshdr *sh = (void*)(s-(sizeof(struct sdshdr))); size_t totlen, curlen = sh->len; @@ -128,7 +128,7 @@ sds sdsgrowsafe(sds s, size_t len) { /* Make sure added region doesn't contain garbage */ sh = (void*)(s-(sizeof(struct sdshdr))); - memset(s+curlen,0,(len-curlen+1)); /* also set trailing NULL byte */ + memset(s+curlen,0,(len-curlen+1)); /* also set trailing \0 byte */ totlen = sh->len+sh->free; sh->len = len; sh->free = totlen-sh->len; diff --git a/src/sds.h b/src/sds.h index 44880183f..91a387821 100644 --- a/src/sds.h +++ b/src/sds.h @@ -49,7 +49,7 @@ size_t sdslen(const sds s); sds sdsdup(const sds s); void sdsfree(sds s); size_t sdsavail(sds s); -sds sdsgrowsafe(sds s, size_t len); +sds sdsgrowzero(sds s, size_t len); sds sdscatlen(sds s, void *t, size_t len); sds sdscat(sds s, char *t); sds sdscpylen(sds s, char *t, size_t len); diff --git a/src/t_string.c b/src/t_string.c index dcb37e553..3b91f5e6d 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -139,7 +139,7 @@ void setbitCommand(redisClient *c) { byte = bitoffset >> 3; bit = 7 - (bitoffset & 0x7); on = bitvalue & 0x1; - o->ptr = sdsgrowsafe(o->ptr,byte+1); + o->ptr = sdsgrowzero(o->ptr,byte+1); ((char*)o->ptr)[byte] |= on << bit; ((char*)o->ptr)[byte] &= ~((!on) << bit); From fae765c34084a71a8793b84fad12dadcad3502aa Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 14 Dec 2010 10:31:11 +0100 Subject: [PATCH 6/9] Don't decode object on STRLEN when not necessary --- src/t_string.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/t_string.c b/src/t_string.c index 3b91f5e6d..d537c8b90 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -358,7 +358,14 @@ void strlenCommand(redisClient *c) { if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.czero)) == NULL || checkType(c,o,REDIS_STRING)) return; - o = getDecodedObject(o); - addReplyLongLong(c,sdslen(o->ptr)); - decrRefCount(o); + if (o->encoding == REDIS_ENCODING_RAW) { + addReplyLongLong(c,sdslen(o->ptr)); + } else if (o->encoding == REDIS_ENCODING_INT) { + char llbuf[32]; + int len = ll2string(llbuf,sizeof(llbuf),(long)o->ptr); + addReplyLongLong(c,len); + } else { + redisPanic("Unknown string encoding"); + } } + From 641289670caa66e7b0005d372a8287475efa7b70 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 14 Dec 2010 14:20:51 +0100 Subject: [PATCH 7/9] Add SETRANGE command implementation and tests --- src/redis.c | 1 + src/redis.h | 1 + src/t_string.c | 85 +++++++++++++++++++++++++++++++++++ tests/unit/basic.tcl | 104 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+) diff --git a/src/redis.c b/src/redis.c index 6d803269e..aee5ce837 100644 --- a/src/redis.c +++ b/src/redis.c @@ -80,6 +80,7 @@ struct redisCommand readonlyCommandTable[] = { {"exists",existsCommand,2,0,NULL,1,1,1}, {"setbit",setbitCommand,4,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"getbit",getbitCommand,3,0,NULL,1,1,1}, + {"setrange",setrangeCommand,4,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"incr",incrCommand,2,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"decr",decrCommand,2,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"mget",mgetCommand,-2,0,NULL,1,-1,1}, diff --git a/src/redis.h b/src/redis.h index b1f992d3e..865409937 100644 --- a/src/redis.h +++ b/src/redis.h @@ -889,6 +889,7 @@ void delCommand(redisClient *c); void existsCommand(redisClient *c); void setbitCommand(redisClient *c); void getbitCommand(redisClient *c); +void setrangeCommand(redisClient *c); void incrCommand(redisClient *c); void decrCommand(redisClient *c); void incrbyCommand(redisClient *c); diff --git a/src/t_string.c b/src/t_string.c index d537c8b90..e442a49fd 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -5,6 +5,14 @@ * String Commands *----------------------------------------------------------------------------*/ +static int checkStringLength(redisClient *c, long long size) { + if (size > 512*1024*1024) { + addReplyError(c,"string exceeds maximum allowed size (512MB)"); + return REDIS_ERR; + } + return REDIS_OK; +} + void setGenericCommand(redisClient *c, int nx, robj *key, robj *val, robj *expire) { int retval; long seconds = 0; /* initialized to avoid an harmness warning */ @@ -172,6 +180,83 @@ void getbitCommand(redisClient *c) { addReply(c, on ? shared.cone : shared.czero); } +void setrangeCommand(redisClient *c) { + robj *o; + long offset; + sds value = c->argv[3]->ptr; + + if (getLongFromObjectOrReply(c,c->argv[2],&offset,NULL) != REDIS_OK) + return; + + o = lookupKeyWrite(c->db,c->argv[1]); + if (o == NULL) { + /* Negative offset is always 0 for non-existing keys */ + if (offset < 0) offset = 0; + + /* Return 0 when setting nothing on a non-existing string */ + if (sdslen(value) == 0) { + addReply(c,shared.czero); + return; + } + + /* Return when the resulting string exceeds allowed size */ + if (checkStringLength(c,offset+sdslen(value)) != REDIS_OK) + return; + + o = createObject(REDIS_STRING,sdsempty()); + dbAdd(c->db,c->argv[1],o); + } else { + int olen; + + /* Key exists, check type */ + if (checkType(c,o,REDIS_STRING)) + return; + + /* Find out existing value length */ + if (o->encoding == REDIS_ENCODING_INT) { + char llbuf[32]; + olen = ll2string(llbuf,sizeof(llbuf),(long)o->ptr); + } else { + olen = sdslen(o->ptr); + } + + /* Return existing string length when setting nothing */ + if (sdslen(value) == 0) { + addReplyLongLong(c,olen); + return; + } + + /* Convert negative indexes. Note that for SETRANGE, the meaning of a + * negative index is a little different than for other commands. + * Here, an offset of -1 points to the trailing NULL byte of the + * string instead of the last character. */ + if (offset < 0) { + offset = olen+1+offset; + if (offset < 0) offset = 0; + } + + /* Return when the resulting string exceeds allowed size */ + if (checkStringLength(c,offset+sdslen(value)) != REDIS_OK) + return; + + /* Create a copy when the object is shared or encoded. */ + if (o->refcount != 1 || o->encoding != REDIS_ENCODING_RAW) { + robj *decoded = getDecodedObject(o); + o = createStringObject(decoded->ptr, sdslen(decoded->ptr)); + decrRefCount(decoded); + dbReplace(c->db,c->argv[1],o); + } + } + + if (sdslen(value) > 0) { + o->ptr = sdsgrowzero(o->ptr,offset+sdslen(value)); + memcpy((char*)o->ptr+offset,value,sdslen(value)); + touchWatchedKey(c->db,c->argv[1]); + server.dirty++; + } + addReplyLongLong(c,sdslen(o->ptr)); +} + void mgetCommand(redisClient *c) { int j; diff --git a/tests/unit/basic.tcl b/tests/unit/basic.tcl index 1324246f1..ecd2040aa 100644 --- a/tests/unit/basic.tcl +++ b/tests/unit/basic.tcl @@ -458,4 +458,108 @@ start_server {tags {"basic"}} { assert_equal 0 [r getbit mykey 100] assert_equal 0 [r getbit mykey 10000] } + + test "SETRANGE against non-existing key" { + r del mykey + assert_equal 3 [r setrange mykey 0 foo] + assert_equal "foo" [r get mykey] + + r del mykey + assert_equal 0 [r setrange mykey 0 ""] + assert_equal 0 [r exists mykey] + + r del mykey + assert_equal 4 [r setrange mykey 1 foo] + assert_equal "\000foo" [r get mykey] + + r del mykey + assert_equal 3 [r setrange mykey -1 foo] + assert_equal "foo" [r get mykey] + + r del mykey + assert_equal 3 [r setrange mykey -100 foo] + assert_equal "foo" [r get mykey] + } + + test "SETRANGE against string-encoded key" { + r set mykey "foo" + assert_equal 3 [r setrange mykey 0 b] + assert_equal "boo" [r get mykey] + + r set mykey "foo" + assert_equal 3 [r setrange mykey 0 ""] + assert_equal "foo" [r get mykey] + + r set mykey "foo" + assert_equal 3 [r setrange mykey 1 b] + assert_equal "fbo" [r get mykey] + + r set mykey "foo" + assert_equal 6 [r setrange mykey -1 bar] + assert_equal "foobar" [r get mykey] + + r set mykey "foo" + assert_equal 5 [r setrange mykey -2 bar] + assert_equal "fobar" [r get mykey] + + r set mykey "foo" + assert_equal 3 [r setrange mykey -20 bar] + assert_equal "bar" [r get mykey] + + r set mykey "foo" + assert_equal 7 [r setrange mykey 4 bar] + assert_equal "foo\000bar" [r get mykey] + } + + test "SETRANGE against integer-encoded key" { + r set mykey 1234 + assert_encoding int mykey + assert_equal 4 [r setrange mykey 0 2] + assert_encoding raw mykey + assert_equal 2234 [r get mykey] + + # Shouldn't change encoding when nothing is set + r set mykey 1234 + assert_encoding int mykey + assert_equal 4 [r setrange mykey 0 ""] + assert_encoding int mykey + assert_equal 1234 [r get mykey] + + r set mykey 1234 + assert_encoding int mykey + assert_equal 4 [r setrange mykey 1 3] + assert_encoding raw mykey + assert_equal 1334 [r get mykey] + + r set mykey 1234 + assert_encoding int mykey + assert_equal 5 [r setrange mykey -1 5] + assert_encoding raw mykey + assert_equal 12345 [r get mykey] + + r set mykey 1234 + assert_encoding int mykey + assert_equal 4 [r setrange mykey -2 5] + assert_encoding raw mykey + assert_equal 1235 [r get mykey] + + r set mykey 1234 + assert_encoding int mykey + assert_equal 6 [r setrange mykey 5 2] + assert_encoding raw mykey + assert_equal "1234\0002" [r get mykey] + } + + test "SETRANGE against key with wrong type" { + r del mykey + r lpush mykey "foo" + assert_error "*wrong kind*" {r setrange mykey 0 bar} + } + + test "SETRANGE with out of range offset" { + r del mykey + assert_error "*maximum allowed size*" {r setrange mykey [expr 512*1024*1024-4] world} + r set mykey "hello" + assert_error "*maximum allowed size*" {r setrange mykey [expr 512*1024*1024-4] world} + } } From c247d94dff8f143267abf01d3a9cbe9f1317b9c2 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 14 Dec 2010 15:10:58 +0100 Subject: [PATCH 8/9] Refactor and rename SUBSTR to GETRANGE SUBSTR is renamed to GETRANGE to have better consistency between command names (with SETRANGE as its dual). GETRANGE is still aliased as SUBSTR. --- src/redis.c | 3 +- src/redis.h | 2 +- src/t_string.c | 74 ++++++++++++++++++++++---------------------- tests/unit/basic.tcl | 36 +++++++++++++++++++++ tests/unit/other.tcl | 36 --------------------- 5 files changed, 76 insertions(+), 75 deletions(-) diff --git a/src/redis.c b/src/redis.c index aee5ce837..035ccea8c 100644 --- a/src/redis.c +++ b/src/redis.c @@ -74,13 +74,14 @@ struct redisCommand readonlyCommandTable[] = { {"setnx",setnxCommand,3,REDIS_CMD_DENYOOM,NULL,0,0,0}, {"setex",setexCommand,4,REDIS_CMD_DENYOOM,NULL,0,0,0}, {"append",appendCommand,3,REDIS_CMD_DENYOOM,NULL,1,1,1}, - {"substr",substrCommand,4,0,NULL,1,1,1}, {"strlen",strlenCommand,2,0,NULL,1,1,1}, {"del",delCommand,-2,0,NULL,0,0,0}, {"exists",existsCommand,2,0,NULL,1,1,1}, {"setbit",setbitCommand,4,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"getbit",getbitCommand,3,0,NULL,1,1,1}, {"setrange",setrangeCommand,4,REDIS_CMD_DENYOOM,NULL,1,1,1}, + {"getrange",getrangeCommand,4,0,NULL,1,1,1}, + {"substr",getrangeCommand,4,0,NULL,1,1,1}, {"incr",incrCommand,2,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"decr",decrCommand,2,REDIS_CMD_DENYOOM,NULL,1,1,1}, {"mget",mgetCommand,-2,0,NULL,1,-1,1}, diff --git a/src/redis.h b/src/redis.h index 865409937..927a4f1dd 100644 --- a/src/redis.h +++ b/src/redis.h @@ -890,6 +890,7 @@ void existsCommand(redisClient *c); void setbitCommand(redisClient *c); void getbitCommand(redisClient *c); void setrangeCommand(redisClient *c); +void getrangeCommand(redisClient *c); void incrCommand(redisClient *c); void decrCommand(redisClient *c); void incrbyCommand(redisClient *c); @@ -967,7 +968,6 @@ void discardCommand(redisClient *c); void blpopCommand(redisClient *c); void brpopCommand(redisClient *c); void appendCommand(redisClient *c); -void substrCommand(redisClient *c); void strlenCommand(redisClient *c); void zrankCommand(redisClient *c); void zrevrankCommand(redisClient *c); diff --git a/src/t_string.c b/src/t_string.c index e442a49fd..b71bfe99b 100644 --- a/src/t_string.c +++ b/src/t_string.c @@ -257,6 +257,43 @@ void setrangeCommand(redisClient *c) { addReplyLongLong(c,sdslen(o->ptr)); } +void getrangeCommand(redisClient *c) { + robj *o; + long start, end; + char *str, llbuf[32]; + size_t strlen; + + if (getLongFromObjectOrReply(c,c->argv[2],&start,NULL) != REDIS_OK) + return; + if (getLongFromObjectOrReply(c,c->argv[3],&end,NULL) != REDIS_OK) + return; + if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.nullbulk)) == NULL || + checkType(c,o,REDIS_STRING)) return; + + if (o->encoding == REDIS_ENCODING_INT) { + str = llbuf; + strlen = ll2string(llbuf,sizeof(llbuf),(long)o->ptr); + } else { + str = o->ptr; + strlen = sdslen(str); + } + + /* Convert negative indexes */ + if (start < 0) start = strlen+start; + if (end < 0) end = strlen+end; + if (start < 0) start = 0; + if (end < 0) end = 0; + if ((unsigned)end >= strlen) end = strlen-1; + + /* Precondition: end >= 0 && end < strlen, so the only condition where + * nothing can be returned is: start > end. */ + if (start > end) { + addReply(c,shared.nullbulk); + } else { + addReplyBulkCBuffer(c,(char*)str+start,end-start+1); + } +} + void mgetCommand(redisClient *c) { int j; @@ -400,43 +437,6 @@ void appendCommand(redisClient *c) { addReplyLongLong(c,totlen); } -void substrCommand(redisClient *c) { - robj *o; - long start = atoi(c->argv[2]->ptr); - long end = atoi(c->argv[3]->ptr); - size_t rangelen, strlen; - sds range; - - if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.nullbulk)) == NULL || - checkType(c,o,REDIS_STRING)) return; - - o = getDecodedObject(o); - strlen = sdslen(o->ptr); - - /* convert negative indexes */ - if (start < 0) start = strlen+start; - if (end < 0) end = strlen+end; - if (start < 0) start = 0; - if (end < 0) end = 0; - - /* indexes sanity checks */ - if (start > end || (size_t)start >= strlen) { - /* Out of range start or start > end result in null reply */ - addReply(c,shared.nullbulk); - decrRefCount(o); - return; - } - if ((size_t)end >= strlen) end = strlen-1; - rangelen = (end-start)+1; - - /* Return the result */ - addReplySds(c,sdscatprintf(sdsempty(),"$%zu\r\n",rangelen)); - range = sdsnewlen((char*)o->ptr+start,rangelen); - addReplySds(c,range); - addReply(c,shared.crlf); - decrRefCount(o); -} - void strlenCommand(redisClient *c) { robj *o; diff --git a/tests/unit/basic.tcl b/tests/unit/basic.tcl index ecd2040aa..f082b05da 100644 --- a/tests/unit/basic.tcl +++ b/tests/unit/basic.tcl @@ -562,4 +562,40 @@ start_server {tags {"basic"}} { r set mykey "hello" assert_error "*maximum allowed size*" {r setrange mykey [expr 512*1024*1024-4] world} } + + test {SUBSTR basics} { + set res {} + r set foo "Hello World" + lappend res [r substr foo 0 3] + lappend res [r substr foo 0 -1] + lappend res [r substr foo -4 -1] + lappend res [r substr foo 5 3] + lappend res [r substr foo 5 5000] + lappend res [r substr foo -5000 10000] + set _ $res + } {Hell {Hello World} orld {} { World} {Hello World}} + + test {SUBSTR against integer encoded values} { + r set foo 123 + r substr foo 0 -2 + } {12} + + test {SUBSTR fuzzing} { + set err {} + for {set i 0} {$i < 1000} {incr i} { + set bin [randstring 0 1024 binary] + set _start [set start [randomInt 1500]] + set _end [set end [randomInt 1500]] + if {$_start < 0} {set _start "end-[abs($_start)-1]"} + if {$_end < 0} {set _end "end-[abs($_end)-1]"} + set s1 [string range $bin $_start $_end] + r set bin $bin + set s2 [r substr bin $start $end] + if {$s1 != $s2} { + set err "String mismatch" + break + } + } + set _ $err + } {} } diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl index 2e6c0ae17..c142ba7f0 100644 --- a/tests/unit/other.tcl +++ b/tests/unit/other.tcl @@ -216,42 +216,6 @@ start_server {tags {"other"}} { set _ $err } {} - test {SUBSTR basics} { - set res {} - r set foo "Hello World" - lappend res [r substr foo 0 3] - lappend res [r substr foo 0 -1] - lappend res [r substr foo -4 -1] - lappend res [r substr foo 5 3] - lappend res [r substr foo 5 5000] - lappend res [r substr foo -5000 10000] - set _ $res - } {Hell {Hello World} orld {} { World} {Hello World}} - - test {SUBSTR against integer encoded values} { - r set foo 123 - r substr foo 0 -2 - } {12} - - test {SUBSTR fuzzing} { - set err {} - for {set i 0} {$i < 1000} {incr i} { - set bin [randstring 0 1024 binary] - set _start [set start [randomInt 1500]] - set _end [set end [randomInt 1500]] - if {$_start < 0} {set _start "end-[abs($_start)-1]"} - if {$_end < 0} {set _end "end-[abs($_end)-1]"} - set s1 [string range $bin $_start $_end] - r set bin $bin - set s2 [r substr bin $start $end] - if {$s1 != $s2} { - set err "String mismatch" - break - } - } - set _ $err - } {} - # Leave the user with a clean DB before to exit test {FLUSHDB} { set aux {} From 2bc467ba66325d6af028a40edd9a17566678d800 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 14 Dec 2010 15:35:35 +0100 Subject: [PATCH 9/9] Add test cases for GETRANGE against integer-encoded strings --- tests/unit/basic.tcl | 54 ++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/tests/unit/basic.tcl b/tests/unit/basic.tcl index f082b05da..7d5667726 100644 --- a/tests/unit/basic.tcl +++ b/tests/unit/basic.tcl @@ -563,39 +563,39 @@ start_server {tags {"basic"}} { assert_error "*maximum allowed size*" {r setrange mykey [expr 512*1024*1024-4] world} } - test {SUBSTR basics} { - set res {} - r set foo "Hello World" - lappend res [r substr foo 0 3] - lappend res [r substr foo 0 -1] - lappend res [r substr foo -4 -1] - lappend res [r substr foo 5 3] - lappend res [r substr foo 5 5000] - lappend res [r substr foo -5000 10000] - set _ $res - } {Hell {Hello World} orld {} { World} {Hello World}} + test "GETRANGE against non-existing key" { + r del mykey + assert_equal "" [r getrange mykey 0 -1] + } - test {SUBSTR against integer encoded values} { - r set foo 123 - r substr foo 0 -2 - } {12} + test "GETRANGE against string value" { + r set mykey "Hello World" + assert_equal "Hell" [r getrange mykey 0 3] + assert_equal "Hello World" [r getrange mykey 0 -1] + assert_equal "orld" [r getrange mykey -4 -1] + assert_equal "" [r getrange mykey 5 3] + assert_equal " World" [r getrange mykey 5 5000] + assert_equal "Hello World" [r getrange mykey -5000 10000] + } - test {SUBSTR fuzzing} { - set err {} + test "GETRANGE against integer-encoded value" { + r set mykey 1234 + assert_equal "123" [r getrange mykey 0 2] + assert_equal "1234" [r getrange mykey 0 -1] + assert_equal "234" [r getrange mykey -3 -1] + assert_equal "" [r getrange mykey 5 3] + assert_equal "4" [r getrange mykey 3 5000] + assert_equal "1234" [r getrange mykey -5000 10000] + } + + test "GETRANGE fuzzing" { for {set i 0} {$i < 1000} {incr i} { - set bin [randstring 0 1024 binary] + r set bin [set bin [randstring 0 1024 binary]] set _start [set start [randomInt 1500]] set _end [set end [randomInt 1500]] if {$_start < 0} {set _start "end-[abs($_start)-1]"} if {$_end < 0} {set _end "end-[abs($_end)-1]"} - set s1 [string range $bin $_start $_end] - r set bin $bin - set s2 [r substr bin $start $end] - if {$s1 != $s2} { - set err "String mismatch" - break - } + assert_equal [string range $bin $_start $_end] [r getrange bin $start $end] } - set _ $err - } {} + } }