summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authormichael-grunder <michael.grunder@gmail.com>2018-05-20 10:44:19 -0700
committermichael-grunder <michael.grunder@gmail.com>2018-05-20 10:44:19 -0700
commit7bef042c0375cd5189b77abb879ca2e3a8c4d6d4 (patch)
tree5bba29b208dc734c6cb2bb1f2524bf70920f0f2b
parent109197585762986502d3a8fa628acc1b82b68cf3 (diff)
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).
-rw-r--r--read.c88
1 files changed, 71 insertions, 17 deletions
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;