SCAN code refactored to parse cursor first.

The previous implementation of SCAN parsed the cursor in the generic
function implementing SCAN, SSCAN, HSCAN and ZSCAN.

The actual higher-level command implementation only checked for empty
keys and return ASAP in that case. The result was that inverting the
arguments of, for instance, SSCAN for example and write:

    SSCAN 0 key

Instead of

    SSCAN key 0

Resulted into no error, since 0 is a non-existing key name very likely.
Just the iterator returned no elements at all.

In order to fix this issue the code was refactored to extract the
function to parse the cursor and return the error. Every higher level
command implementation now parses the cursor and later checks if the key
exist or not.

src/db.c

@@ -340,6 +340,25 @@ void scanCallback(void *privdata, const dictEntry *de) {
     if (val) listAddNodeTail(keys, val);
 }
 
+/* Try to parse a SCAN cursor stored ad object 'o':
+ * if the cursor is valid, store it as unsigned integer into *cursor and
+ * returns REDIS_OK. Otherwise return REDIS_ERR and send an error to the
+ * client. */
+int parseScanCursorOrReply(redisClient *c, robj *o, unsigned long *cursor) {
+    char *eptr;
+
+    /* Use strtoul() because we need an *unsigned* long, so
+     * getLongLongFromObject() does not cover the whole cursor space. */
+    errno = 0;
+    *cursor = strtoul(o->ptr, &eptr, 10);
+    if (isspace(((char*)o->ptr)[0]) || eptr[0] != '\0' || errno == ERANGE)
+    {
+        addReplyError(c, "invalid cursor");
+        return REDIS_ERR;
+    }
+    return REDIS_OK;
+}
+
 /* This command implements SCAN, HSCAN and SSCAN commands.
  * If object 'o' is passed, then it must be an Hash or Set object, otherwise
  * if 'o' is NULL the command will operate on the dictionary associated with
@@ -351,13 +370,12 @@ void scanCallback(void *privdata, const dictEntry *de) {
  *
  * In the case of an Hash object the function returns both the field and value
  * of every element on the Hash. */
-void scanGenericCommand(redisClient *c, robj *o) {
+void scanGenericCommand(redisClient *c, robj *o, unsigned long cursor) {
     int rv;
     int i, j;
-    char buf[REDIS_LONGSTR_SIZE], *eptr;
+    char buf[REDIS_LONGSTR_SIZE];
     list *keys = listCreate();
     listNode *node, *nextnode;
-    unsigned long cursor = 0;
     long count = 10;
     sds pat;
     int patlen, use_pattern = 0;
@@ -371,16 +389,6 @@ void scanGenericCommand(redisClient *c, robj *o) {
     /* Set i to the first option argument. The previous one is the cursor. */
     i = (o == NULL) ? 2 : 3; /* Skip the key argument if needed. */
 
-    /* Use strtoul() because we need an *unsigned* long, so
-     * getLongLongFromObject() does not cover the whole cursor space. */
-    errno = 0;
-    cursor = strtoul(c->argv[i-1]->ptr, &eptr, 10);
-    if (isspace(((char*)c->argv[i-1])[0]) || eptr[0] != '\0' || errno == ERANGE)
-    {
-        addReplyError(c, "invalid cursor");
-        goto cleanup;
-    }
-
     /* Step 1: Parse options. */
     while (i < c->argc) {
         j = c->argc - i;
@@ -538,7 +546,9 @@ cleanup:
 
 /* The SCAN command completely relies on scanGenericCommand. */
 void scanCommand(redisClient *c) {
-    scanGenericCommand(c,NULL);
+    unsigned long cursor;
+    if (parseScanCursorOrReply(c,c->argv[1],&cursor) == REDIS_ERR) return;
+    scanGenericCommand(c,NULL,cursor);
 }
 
 void dbsizeCommand(redisClient *c) {

src/redis.h

@@ -1167,7 +1167,8 @@ int selectDb(redisClient *c, int id);
 void signalModifiedKey(redisDb *db, robj *key);
 void signalFlushedDb(int dbid);
 unsigned int GetKeysInSlot(unsigned int hashslot, robj **keys, unsigned int count);
-void scanGenericCommand(redisClient *c, robj *o);
+void scanGenericCommand(redisClient *c, robj *o, unsigned long cursor);
+int parseScanCursorOrReply(redisClient *c, robj *o, unsigned long *cursor);
 
 /* API to get key arguments from commands */
 #define REDIS_GETKEYS_ALL 0

src/t_hash.c

@@ -765,8 +765,10 @@ void hexistsCommand(redisClient *c) {
 
 void hscanCommand(redisClient *c) {
     robj *o;
+    unsigned long cursor;
 
+    if (parseScanCursorOrReply(c,c->argv[2],&cursor) == REDIS_ERR) return;
     if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.emptyscan)) == NULL ||
         checkType(c,o,REDIS_HASH)) return;
-    scanGenericCommand(c,o);
+    scanGenericCommand(c,o,cursor);
 }

src/t_set.c

@@ -914,8 +914,10 @@ void sdiffstoreCommand(redisClient *c) {
 
 void sscanCommand(redisClient *c) {
     robj *set;
+    unsigned long cursor;
 
+    if (parseScanCursorOrReply(c,c->argv[2],&cursor) == REDIS_ERR) return;
     if ((set = lookupKeyReadOrReply(c,c->argv[1],shared.emptyscan)) == NULL ||
         checkType(c,set,REDIS_SET)) return;
-    scanGenericCommand(c,set);
+    scanGenericCommand(c,set,cursor);
 }

src/t_zset.c

@@ -2207,8 +2207,10 @@ void zrevrankCommand(redisClient *c) {
 
 void zscanCommand(redisClient *c) {
     robj *o;
+    unsigned long cursor;
 
+    if (parseScanCursorOrReply(c,c->argv[2],&cursor) == REDIS_ERR) return;
     if ((o = lookupKeyReadOrReply(c,c->argv[1],shared.emptyscan)) == NULL ||
         checkType(c,o,REDIS_ZSET)) return;
-    scanGenericCommand(c,o);
+    scanGenericCommand(c,o,cursor);
 }