Tests for string2ll; move isObject* to object.c

src/Makefile

@@ -125,8 +125,7 @@ t_string.o: t_string.c redis.h fmacros.h config.h ae.h sds.h dict.h \
   adlist.h zmalloc.h anet.h zipmap.h ziplist.h intset.h version.h
 t_zset.o: t_zset.c redis.h fmacros.h config.h ae.h sds.h dict.h adlist.h \
   zmalloc.h anet.h zipmap.h ziplist.h intset.h version.h
-util.o: util.c redis.h fmacros.h config.h ae.h sds.h dict.h adlist.h \
-  zmalloc.h anet.h zipmap.h ziplist.h intset.h version.h
+util.o: util.c util.h
 cluster.o: redis.h fmacros.h config.h ae.h sds.h dict.h adlist.h \
   zmalloc.h anet.h zipmap.h ziplist.h intset.h version.h
 ziplist.o: ziplist.c zmalloc.h ziplist.h

src/object.c

@@ -202,6 +202,16 @@ int checkType(redisClient *c, robj *o, int type) {
     return 0;
 }
 
+int isObjectRepresentableAsLongLong(robj *o, long long *llval) {
+    redisAssert(o->type == REDIS_STRING);
+    if (o->encoding == REDIS_ENCODING_INT) {
+        if (llval) *llval = (long) o->ptr;
+        return REDIS_OK;
+    } else {
+        return string2ll(o->ptr,sdslen(o->ptr),llval) ? REDIS_OK : REDIS_ERR;
+    }
+}
+
 /* Try to encode a string object in order to save space */
 robj *tryObjectEncoding(robj *o) {
     long value;
@@ -219,7 +229,7 @@ robj *tryObjectEncoding(robj *o) {
     redisAssert(o->type == REDIS_STRING);
 
     /* Check if we can represent this string as a long integer */
-    if (isStringRepresentableAsLong(s,&value) == REDIS_ERR) return o;
+    if (!string2l(s,sdslen(s),&value)) return o;
 
     /* Ok, this object can be encoded...
      *

src/redis.h

@@ -30,6 +30,7 @@
 #include "ziplist.h" /* Compact list data structure */
 #include "intset.h" /* Compact integer set structure */
 #include "version.h"
+#include "util.h"
 
 /* Error codes */
 #define REDIS_OK                0
@@ -852,6 +853,7 @@ void freeHashObject(robj *o);
 robj *createObject(int type, void *ptr);
 robj *createStringObject(char *ptr, size_t len);
 robj *dupStringObject(robj *o);
+int isObjectRepresentableAsLongLong(robj *o, long long *llongval);
 robj *tryObjectEncoding(robj *o);
 robj *getDecodedObject(robj *o);
 size_t stringObjectLen(robj *o);
@@ -1028,18 +1030,6 @@ int pubsubUnsubscribeAllPatterns(redisClient *c, int notify);
 void freePubsubPattern(void *p);
 int listMatchPubsubPattern(void *a, void *b);
 
-/* Utility functions */
-int stringmatchlen(const char *pattern, int patternLen,
-        const char *string, int stringLen, int nocase);
-int stringmatch(const char *pattern, const char *string, int nocase);
-long long memtoll(const char *p, int *err);
-int ll2string(char *s, size_t len, long long value);
-int string2ll(char *s, size_t len, long long *value);
-int d2string(char *s, size_t len, double value);
-int isStringRepresentableAsLong(sds s, long *longval);
-int isStringRepresentableAsLongLong(sds s, long long *longval);
-int isObjectRepresentableAsLongLong(robj *o, long long *llongval);
-
 /* Configuration */
 void loadServerConfig(char *filename);
 void appendServerSaveParams(time_t seconds, int changes);

src/util.c

-#include "redis.h"
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
 #include <ctype.h>
 #include <limits.h>
 #include <math.h>
+#include "util.h"
 
 /* Glob-style pattern matching. */
 int stringmatchlen(const char *pattern, int patternLen,
@@ -213,6 +216,12 @@ int string2ll(char *s, size_t slen, long long *value) {
     if (plen == slen)
         return 0;
 
+    /* Special case: first and only digit is 0. */
+    if (slen == 1 && p[0] == '0') {
+        if (value != NULL) *value = 0;
+        return 1;
+    }
+
     if (p[0] == '-') {
         negative = 1;
         p++; plen++;
@@ -250,7 +259,7 @@ int string2ll(char *s, size_t slen, long long *value) {
         return 0;
 
     if (negative) {
-        if (v > (-(unsigned long long)LLONG_MIN)) /* Overflow. */
+        if (v > ((unsigned long long)(-(LLONG_MIN+1))+1)) /* Overflow. */
             return 0;
         if (value != NULL) *value = -v;
     } else {
@@ -261,6 +270,22 @@ int string2ll(char *s, size_t slen, long long *value) {
     return 1;
 }
 
+/* Convert a string into a long. Returns 1 if the string could be parsed into a
+ * (non-overflowing) long, 0 otherwise. The value will be set to the parsed
+ * value when appropriate. */
+int string2l(char *s, size_t slen, long *lval) {
+    long long llval;
+
+    if (!string2ll(s,slen,&llval))
+        return 0;
+
+    if (llval < LONG_MIN || llval > LONG_MAX)
+        return 0;
+
+    *lval = (long)llval;
+    return 1;
+}
+
 /* Convert a double to a string representation. Returns the number of bytes
  * required. The representation should always be parsable by stdtod(3). */
 int d2string(char *buf, size_t len, double value) {
@@ -300,44 +325,108 @@ int d2string(char *buf, size_t len, double value) {
     return len;
 }
 
-/* Check if the sds string 's' can be represented by a long long
- * (that is, is a number that fits into long without any other space or
- * character before or after the digits, so that converting this number
- * back to a string will result in the same bytes as the original string).
- *
- * If so, the function returns REDIS_OK and *llongval is set to the value
- * of the number. Otherwise REDIS_ERR is returned */
-int isStringRepresentableAsLongLong(sds s, long long *llongval) {
-    char buf[32], *endptr;
-    long long value;
-    int slen;
-
-    value = strtoll(s, &endptr, 10);
-    if (endptr[0] != '\0') return REDIS_ERR;
-    slen = ll2string(buf,32,value);
-
-    /* If the number converted back into a string is not identical
-     * then it's not possible to encode the string as integer */
-    if (sdslen(s) != (unsigned)slen || memcmp(buf,s,slen)) return REDIS_ERR;
-    if (llongval) *llongval = value;
-    return REDIS_OK;
+#ifdef UTIL_TEST_MAIN
+#include <assert.h>
+
+void test_string2ll(void) {
+    char buf[32];
+    long long v;
+
+    /* May not start with +. */
+    strcpy(buf,"+1");
+    assert(string2ll(buf,strlen(buf),&v) == 0);
+
+    /* May not start with 0. */
+    strcpy(buf,"01");
+    assert(string2ll(buf,strlen(buf),&v) == 0);
+
+    strcpy(buf,"-1");
+    assert(string2ll(buf,strlen(buf),&v) == 1);
+    assert(v == -1);
+
+    strcpy(buf,"0");
+    assert(string2ll(buf,strlen(buf),&v) == 1);
+    assert(v == 0);
+
+    strcpy(buf,"1");
+    assert(string2ll(buf,strlen(buf),&v) == 1);
+    assert(v == 1);
+
+    strcpy(buf,"99");
+    assert(string2ll(buf,strlen(buf),&v) == 1);
+    assert(v == 99);
+
+    strcpy(buf,"-99");
+    assert(string2ll(buf,strlen(buf),&v) == 1);
+    assert(v == -99);
+
+    strcpy(buf,"-9223372036854775808");
+    assert(string2ll(buf,strlen(buf),&v) == 1);
+    assert(v == LLONG_MIN);
+
+    strcpy(buf,"-9223372036854775809"); /* overflow */
+    assert(string2ll(buf,strlen(buf),&v) == 0);
+
+    strcpy(buf,"9223372036854775807");
+    assert(string2ll(buf,strlen(buf),&v) == 1);
+    assert(v == LLONG_MAX);
+
+    strcpy(buf,"9223372036854775808"); /* overflow */
+    assert(string2ll(buf,strlen(buf),&v) == 0);
 }
 
-int isStringRepresentableAsLong(sds s, long *longval) {
-    long long ll;
+void test_string2l(void) {
+    char buf[32];
+    long v;
+
+    /* May not start with +. */
+    strcpy(buf,"+1");
+    assert(string2l(buf,strlen(buf),&v) == 0);
+
+    /* May not start with 0. */
+    strcpy(buf,"01");
+    assert(string2l(buf,strlen(buf),&v) == 0);
+
+    strcpy(buf,"-1");
+    assert(string2l(buf,strlen(buf),&v) == 1);
+    assert(v == -1);
+
+    strcpy(buf,"0");
+    assert(string2l(buf,strlen(buf),&v) == 1);
+    assert(v == 0);
+
+    strcpy(buf,"1");
+    assert(string2l(buf,strlen(buf),&v) == 1);
+    assert(v == 1);
+
+    strcpy(buf,"99");
+    assert(string2l(buf,strlen(buf),&v) == 1);
+    assert(v == 99);
 
-    if (isStringRepresentableAsLongLong(s,&ll) == REDIS_ERR) return REDIS_ERR;
-    if (ll < LONG_MIN || ll > LONG_MAX) return REDIS_ERR;
-    *longval = (long)ll;
-    return REDIS_OK;
+    strcpy(buf,"-99");
+    assert(string2l(buf,strlen(buf),&v) == 1);
+    assert(v == -99);
+
+#if LONG_MAX != LLONG_MAX
+    strcpy(buf,"-2147483648");
+    assert(string2l(buf,strlen(buf),&v) == 1);
+    assert(v == LONG_MIN);
+
+    strcpy(buf,"-2147483649"); /* overflow */
+    assert(string2l(buf,strlen(buf),&v) == 0);
+
+    strcpy(buf,"2147483647");
+    assert(string2l(buf,strlen(buf),&v) == 1);
+    assert(v == LONG_MAX);
+
+    strcpy(buf,"2147483648"); /* overflow */
+    assert(string2l(buf,strlen(buf),&v) == 0);
+#endif
 }
 
-int isObjectRepresentableAsLongLong(robj *o, long long *llongval) {
-    redisAssert(o->type == REDIS_STRING);
-    if (o->encoding == REDIS_ENCODING_INT) {
-        if (llongval) *llongval = (long) o->ptr;
-        return REDIS_OK;
-    } else {
-        return isStringRepresentableAsLongLong(o->ptr,llongval);
-    }
+int main(int argc, char **argv) {
+    test_string2ll();
+    test_string2l();
+    return 0;
 }
+#endif

src/util.h

+#ifndef __REDIS_UTIL_H
+#define __REDIS_UTIL_H
+
+int stringmatchlen(const char *p, int plen, const char *s, int slen, int nocase);
+int stringmatch(const char *p, const char *s, int nocase);
+long long memtoll(const char *p, int *err);
+int ll2string(char *s, size_t len, long long value);
+int string2ll(char *s, size_t slen, long long *value);
+int string2l(char *s, size_t slen, long *value);
+int d2string(char *buf, size_t len, double value);
+
+#endif