From 7bef042c0375cd5189b77abb879ca2e3a8c4d6d4 Mon Sep 17 00:00:00 2001
From: michael-grunder <michael.grunder@gmail.com>
Date: Sun, 20 May 2018 10:44:19 -0700
Subject: Use string2ll from Redis

This commit pulls string2ll from Redis (with permission from Antirez)
as strtoll is 2-3x slower and even worse vs the original version in
hiredis that didn't check for overflow at all.

By using string2ll there is almost no measurable performance impact
of overflow detection even in integer parsing heavy workloads (e.g.
INCRBY commands).
---
 read.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 71 insertions(+), 17 deletions(-)

(limited to 'read.c')

diff --git a/read.c b/read.c
index 39c21b8..ef483f7 100644
--- a/read.c
+++ b/read.c
@@ -142,24 +142,78 @@ static char *seekNewline(char *s, size_t len) {
     return NULL;
 }
 
-/* Read a long long value starting at *s, under the assumption that it will be
- * terminated by \r\n. Returns REDIS_ERR for unexpected inputs or if the
- * resulting value would be greater than LLONG_MAX. */
-static int readLongLong(char *s, long long* val) {
-    char* end;
-    errno = 0;
+/* Convert a string into a long long. Returns REDIS_OK if the string could be
+ * parsed into a (non-overflowing) long long, REDIS_ERR otherwise. The value
+ * will be set to the parsed value when appropriate.
+ *
+ * Note that this function demands that the string strictly represents
+ * a long long: no spaces or other characters before or after the string
+ * representing the number are accepted, nor zeroes at the start if not
+ * for the string "0" representing the zero number.
+ *
+ * Because of its strictness, it is safe to use this function to check if
+ * you can convert a string into a long long, and obtain back the string
+ * from the number without any loss in the string representation. */
+int string2ll(const char *s, size_t slen, long long *value) {
+    const char *p = s;
+    size_t plen = 0;
+    int negative = 0;
+    unsigned long long v;
+
+    if (plen == slen)
+        return REDIS_ERR;
+
+    /* Special case: first and only digit is 0. */
+    if (slen == 1 && p[0] == '0') {
+        if (value != NULL) *value = 0;
+        return REDIS_OK;
+    }
 
-    long long v = strtoll(s, &end, 10);
+    if (p[0] == '-') {
+        negative = 1;
+        p++; plen++;
 
-    if(s == end || ((v == LLONG_MAX || v == LLONG_MIN) && errno == ERANGE)
-        || (*end != '\r' || *(end+1) != '\n')) {
+        /* Abort on only a negative sign. */
+        if (plen == slen)
+            return REDIS_ERR;
+    }
+
+    /* First digit should be 1-9, otherwise the string should just be 0. */
+    if (p[0] >= '1' && p[0] <= '9') {
+        v = p[0]-'0';
+        p++; plen++;
+    } else if (p[0] == '0' && slen == 1) {
+        *value = 0;
+        return REDIS_OK;
+    } else {
         return REDIS_ERR;
     }
 
-    if(val) {
-        *val = v;
+    while (plen < slen && p[0] >= '0' && p[0] <= '9') {
+        if (v > (ULLONG_MAX / 10)) /* Overflow. */
+            return REDIS_ERR;
+        v *= 10;
+
+        if (v > (ULLONG_MAX - (p[0]-'0'))) /* Overflow. */
+            return REDIS_ERR;
+        v += p[0]-'0';
+
+        p++; plen++;
     }
 
+    /* Return if not all bytes were used. */
+    if (plen < slen)
+        return REDIS_ERR;
+
+    if (negative) {
+        if (v > ((unsigned long long)(-(LLONG_MIN+1))+1)) /* Overflow. */
+            return REDIS_ERR;
+        if (value != NULL) *value = -v;
+    } else {
+        if (v > LLONG_MAX) /* Overflow. */
+            return REDIS_ERR;
+        if (value != NULL) *value = v;
+    }
     return REDIS_OK;
 }
 
@@ -213,7 +267,7 @@ static int processLineItem(redisReader *r) {
         if (cur->type == REDIS_REPLY_INTEGER) {
             if (r->fn && r->fn->createInteger) {
                 long long v;
-                if(readLongLong(p, &v) == REDIS_ERR) {
+                if (string2ll(p, len, &v) == REDIS_ERR) {
                     __redisReaderSetError(r,REDIS_ERR_PROTOCOL,
                             "Bad integer value");
                     return REDIS_ERR;
@@ -258,7 +312,7 @@ static int processBulkItem(redisReader *r) {
         p = r->buf+r->pos;
         bytelen = s-(r->buf+r->pos)+2; /* include \r\n */
 
-        if (readLongLong(p, &len) == REDIS_ERR) {
+        if (string2ll(p, bytelen - 2, &len) == REDIS_ERR) {
             __redisReaderSetError(r,REDIS_ERR_PROTOCOL,
                     "Bad bulk string length");
             return REDIS_ERR;
@@ -313,7 +367,7 @@ static int processMultiBulkItem(redisReader *r) {
     void *obj;
     char *p;
     long long elements;
-    int root = 0;
+    int root = 0, len;
 
     /* Set error for nested multi bulks with depth > 7 */
     if (r->ridx == 8) {
@@ -322,8 +376,8 @@ static int processMultiBulkItem(redisReader *r) {
         return REDIS_ERR;
     }
 
-    if ((p = readLine(r,NULL)) != NULL) {
-        if(readLongLong(p, &elements) == REDIS_ERR) {
+    if ((p = readLine(r,&len)) != NULL) {
+        if (string2ll(p, len, &elements) == REDIS_ERR) {
             __redisReaderSetError(r,REDIS_ERR_PROTOCOL,
                     "Bad multi-bulk length");
             return REDIS_ERR;
@@ -331,7 +385,7 @@ static int processMultiBulkItem(redisReader *r) {
 
         root = (r->ridx == 0);
 
-        if(elements < -1 || elements > INT_MAX) {
+        if (elements < -1 || elements > INT_MAX) {
             __redisReaderSetError(r,REDIS_ERR_PROTOCOL,
                     "Multi-bulk length out of range");
             return REDIS_ERR;
-- 
cgit v1.2.3