Redis version 2.2.6

This reverts commit 1b905277bb.
Moving all the new stuff in 2.4 branch.

00-RELEASENOTES

@@ -12,6 +12,35 @@ for 2.0.
 CHANGELOG
 ---------
 
+What's new in Redis 2.2.6
+=========================
+
+* Fixed bug #543. If you saw Redis instances crashing on List operations
+  (only happening with a non-default max entry size ziplist setting in
+  redis.conf) it was almost certainly this problem.
+* Fixed a bug with replication where SLAVEOF NO ONE caused a slave to close the
+  connection with all its slaves.
+
+What's new in Redis 2.2.5
+=========================
+
+* Fixed a crash occurring when loading an AOF containing the SPOP command.
+
+What's new in Redis 2.2.4
+=========================
+
+* Return value of OBJECT DEBUG against sorted sets fixed, now is "skiplist".
+
+What's new in Redis 2.2.3
+=========================
+
+* Fixed issue #503. MONITOR + QUIT (and other combinations) could crash
+  the server.
+* OBJECT command implemented. See http://redis.io/commands/object
+* Fixed a problem in redis-cli related to escapes in the form "\x..".
+* Fixed a minor memory leak in redis-cli
+* Saved RDB on SIGTERM on archs where it was not working properly.
+
 What's new in Redis 2.2.2
 =========================
 

src/aof.c

@@ -285,9 +285,11 @@ int loadAppendOnlyFile(char *filename) {
         /* The fake client should not have a reply */
         redisAssert(fakeClient->bufpos == 0 && listLength(fakeClient->reply) == 0);
 
-        /* Clean up, ready for the next command */
-        for (j = 0; j < argc; j++) decrRefCount(argv[j]);
-        zfree(argv);
+        /* Clean up. Command code may have changed argv/argc so we use the
+         * argv/argc of the client instead of the local variables. */
+        for (j = 0; j < fakeClient->argc; j++)
+            decrRefCount(fakeClient->argv[j]);
+        zfree(fakeClient->argv);
 
         /* Handle swapping while loading big datasets when VM is on */
         force_swapout = 0;

src/networking.c

@@ -60,9 +60,6 @@ redisClient *createClient(int fd) {
 /* Set the event loop to listen for write events on the client's socket.
  * Typically gets called every time a reply is built. */
 int _installWriteEvent(redisClient *c) {
-    /* When CLOSE_AFTER_REPLY is set, no more replies may be added! */
-    redisAssert(!(c->flags & REDIS_CLOSE_AFTER_REPLY));
-
     if (c->fd <= 0) return REDIS_ERR;
     if (c->bufpos == 0 && listLength(c->reply) == 0 &&
         (c->replstate == REDIS_REPL_NONE ||
@@ -88,9 +85,15 @@ robj *dupLastObjectIfNeeded(list *reply) {
     return listNodeValue(ln);
 }
 
+/* -----------------------------------------------------------------------------
+ * Low level functions to add more data to output buffers.
+ * -------------------------------------------------------------------------- */
+
 int _addReplyToBuffer(redisClient *c, char *s, size_t len) {
     size_t available = sizeof(c->buf)-c->bufpos;
 
+    if (c->flags & REDIS_CLOSE_AFTER_REPLY) return REDIS_OK;
+
     /* If there already are entries in the reply list, we cannot
      * add anything more to the static buffer. */
     if (listLength(c->reply) > 0) return REDIS_ERR;
@@ -105,6 +108,9 @@ int _addReplyToBuffer(redisClient *c, char *s, size_t len) {
 
 void _addReplyObjectToList(redisClient *c, robj *o) {
     robj *tail;
+
+    if (c->flags & REDIS_CLOSE_AFTER_REPLY) return;
+
     if (listLength(c->reply) == 0) {
         incrRefCount(o);
         listAddNodeTail(c->reply,o);
@@ -128,6 +134,12 @@ void _addReplyObjectToList(redisClient *c, robj *o) {
  * needed it will be free'd, otherwise it ends up in a robj. */
 void _addReplySdsToList(redisClient *c, sds s) {
     robj *tail;
+
+    if (c->flags & REDIS_CLOSE_AFTER_REPLY) {
+        sdsfree(s);
+        return;
+    }
+
     if (listLength(c->reply) == 0) {
         listAddNodeTail(c->reply,createObject(REDIS_STRING,s));
     } else {
@@ -148,6 +160,9 @@ void _addReplySdsToList(redisClient *c, sds s) {
 
 void _addReplyStringToList(redisClient *c, char *s, size_t len) {
     robj *tail;
+
+    if (c->flags & REDIS_CLOSE_AFTER_REPLY) return;
+
     if (listLength(c->reply) == 0) {
         listAddNodeTail(c->reply,createStringObject(s,len));
     } else {
@@ -165,6 +180,11 @@ void _addReplyStringToList(redisClient *c, char *s, size_t len) {
     }
 }
 
+/* -----------------------------------------------------------------------------
+ * Higher level functions to queue data on the client output buffer.
+ * The following functions are the ones that commands implementations will call.
+ * -------------------------------------------------------------------------- */
+
 void addReply(redisClient *c, robj *obj) {
     if (_installWriteEvent(c) != REDIS_OK) return;
     redisAssert(!server.vm_enabled || obj->storage == REDIS_VM_MEMORY);
@@ -503,10 +523,16 @@ void freeClient(redisClient *c) {
          * close the connection with all our slaves if we have any, so
          * when we'll resync with the master the other slaves will sync again
          * with us as well. Note that also when the slave is not connected
-         * to the master it will keep refusing connections by other slaves. */
-        while (listLength(server.slaves)) {
-            ln = listFirst(server.slaves);
-            freeClient((redisClient*)ln->value);
+         * to the master it will keep refusing connections by other slaves.
+         *
+         * We do this only if server.masterhost != NULL. If it is NULL this
+         * means the user called SLAVEOF NO ONE and we are freeing our
+         * link with the master, so no need to close link with slaves. */
+        if (server.masterhost != NULL) {
+            while (listLength(server.slaves)) {
+                ln = listFirst(server.slaves);
+                freeClient((redisClient*)ln->value);
+            }
         }
     }
     /* Release memory */

src/object.c

@@ -93,10 +93,13 @@ robj *createHashObject(void) {
 
 robj *createZsetObject(void) {
     zset *zs = zmalloc(sizeof(*zs));
+    robj *o;
 
     zs->dict = dictCreate(&zsetDictType,NULL);
     zs->zsl = zslCreate();
-    return createObject(REDIS_ZSET,zs);
+    o = createObject(REDIS_ZSET,zs);
+    o->encoding = REDIS_ENCODING_SKIPLIST;
+    return o;
 }
 
 void freeStringObject(robj *o) {
@@ -425,6 +428,7 @@ char *strEncoding(int encoding) {
     case REDIS_ENCODING_LINKEDLIST: return "linkedlist";
     case REDIS_ENCODING_ZIPLIST: return "ziplist";
     case REDIS_ENCODING_INTSET: return "intset";
+    case REDIS_ENCODING_SKIPLIST: return "skiplist";
     default: return "unknown";
     }
 }
@@ -439,3 +443,42 @@ unsigned long estimateObjectIdleTime(robj *o) {
                     REDIS_LRU_CLOCK_RESOLUTION;
     }
 }
+
+/* This is an helper function for the DEBUG command. We need to lookup keys
+ * without any modification of LRU or other parameters. */
+robj *objectCommandLookup(redisClient *c, robj *key) {
+    dictEntry *de;
+
+    if ((de = dictFind(c->db->dict,key->ptr)) == NULL) return NULL;
+    return (robj*) dictGetEntryVal(de);
+}
+
+robj *objectCommandLookupOrReply(redisClient *c, robj *key, robj *reply) {
+    robj *o = objectCommandLookup(c,key);
+
+    if (!o) addReply(c, reply);
+    return o;
+}
+
+/* Object command allows to inspect the internals of an Redis Object.
+ * Usage: OBJECT <verb> ... arguments ... */
+void objectCommand(redisClient *c) {
+    robj *o;
+
+    if (!strcasecmp(c->argv[1]->ptr,"refcount") && c->argc == 3) {
+        if ((o = objectCommandLookupOrReply(c,c->argv[2],shared.nullbulk))
+                == NULL) return;
+        addReplyLongLong(c,o->refcount);
+    } else if (!strcasecmp(c->argv[1]->ptr,"encoding") && c->argc == 3) {
+        if ((o = objectCommandLookupOrReply(c,c->argv[2],shared.nullbulk))
+                == NULL) return;
+        addReplyBulkCString(c,strEncoding(o->encoding));
+    } else if (!strcasecmp(c->argv[1]->ptr,"idletime") && c->argc == 3) {
+        if ((o = objectCommandLookupOrReply(c,c->argv[2],shared.nullbulk))
+                == NULL) return;
+        addReplyLongLong(c,estimateObjectIdleTime(o));
+    } else {
+        addReplyError(c,"Syntax error. Try OBJECT (refcount|encoding|idletime)");
+    }
+}
+

src/redis-cli.c

@@ -65,6 +65,7 @@ static struct config {
     char *historyfile;
     int raw_output; /* output mode per command */
     sds mb_delim;
+    char prompt[32];
 } config;
 
 static void usage();
@@ -85,6 +86,13 @@ static long long mstime(void) {
     return mst;
 }
 
+static void cliRefreshPrompt(void) {
+    if (config.dbnum == 0)
+        snprintf(config.prompt,sizeof(config.prompt),"redis> ");
+    else
+        snprintf(config.prompt,sizeof(config.prompt),"redis:%d> ",config.dbnum);
+}
+
 /*------------------------------------------------------------------------------
  * Help functions
  *--------------------------------------------------------------------------- */
@@ -264,11 +272,9 @@ static int cliAuth() {
 /* Send SELECT dbnum to the server */
 static int cliSelect() {
     redisReply *reply;
-    char dbnum[16];
     if (config.dbnum == 0) return REDIS_OK;
 
-    snprintf(dbnum,sizeof(dbnum),"%d",config.dbnum);
-    reply = redisCommand(context,"SELECT %s",dbnum);
+    reply = redisCommand(context,"SELECT %d",config.dbnum);
     if (reply != NULL) {
         freeReplyObject(reply);
         return REDIS_OK;
@@ -489,9 +495,19 @@ static int cliSendCommand(int argc, char **argv, int repeat) {
             }
         }
 
-        if (cliReadReply(output_raw) != REDIS_OK)
+        if (cliReadReply(output_raw) != REDIS_OK) {
+            free(argvlen);
             return REDIS_ERR;
+        } else {
+            /* Store database number when SELECT was successfully executed. */
+            if (!strcasecmp(command,"select") && argc == 2) {
+                config.dbnum = atoi(argv[1]);
+                cliRefreshPrompt();
+            }
+        }
     }
+
+    free(argvlen);
     return REDIS_OK;
 }
 
@@ -616,7 +632,8 @@ static void repl() {
     config.interactive = 1;
     linenoiseSetCompletionCallback(completionCallback);
 
-    while((line = linenoise(context ? "redis> " : "not connected> ")) != NULL) {
+    cliRefreshPrompt();
+    while((line = linenoise(context ? config.prompt : "not connected> ")) != NULL) {
         if (line[0] != '\0') {
             argv = sdssplitargs(line,&argc);
             linenoiseHistoryAdd(line);

src/redis.c

@@ -187,7 +187,8 @@ struct redisCommand readonlyCommandTable[] = {
     {"punsubscribe",punsubscribeCommand,-1,0,NULL,0,0,0},
     {"publish",publishCommand,3,REDIS_CMD_FORCE_REPLICATION,NULL,0,0,0},
     {"watch",watchCommand,-2,0,NULL,0,0,0},
-    {"unwatch",unwatchCommand,1,0,NULL,0,0,0}
+    {"unwatch",unwatchCommand,1,0,NULL,0,0,0},
+    {"object",objectCommand,-2,0,NULL,0,0,0}
 };
 
 /*============================ Utility functions ============================ */
@@ -837,7 +838,7 @@ void initServer() {
 
     signal(SIGHUP, SIG_IGN);
     signal(SIGPIPE, SIG_IGN);
-    setupSigSegvAction();
+    setupSignalHandlers();
 
     if (server.syslog_enabled) {
         openlog(server.syslog_ident, LOG_PID | LOG_NDELAY | LOG_NOWAIT,
@@ -1574,10 +1575,8 @@ int main(int argc, char **argv) {
     return 0;
 }
 
-/* ============================= Backtrace support ========================= */
-
 #ifdef HAVE_BACKTRACE
-void *getMcontextEip(ucontext_t *uc) {
+static void *getMcontextEip(ucontext_t *uc) {
 #if defined(__FreeBSD__)
     return (void*) uc->uc_mcontext.mc_eip;
 #elif defined(__dietlibc__)
@@ -1605,7 +1604,7 @@ void *getMcontextEip(ucontext_t *uc) {
 #endif
 }
 
-void segvHandler(int sig, siginfo_t *info, void *secret) {
+static void sigsegvHandler(int sig, siginfo_t *info, void *secret) {
     void *trace[100];
     char **messages = NULL;
     int i, trace_size = 0;
@@ -1644,37 +1643,35 @@ void segvHandler(int sig, siginfo_t *info, void *secret) {
     sigaction (sig, &act, NULL);
     kill(getpid(),sig);
 }
+#endif /* HAVE_BACKTRACE */
 
-void sigtermHandler(int sig) {
+static void sigtermHandler(int sig) {
     REDIS_NOTUSED(sig);
 
-    redisLog(REDIS_WARNING,"SIGTERM received, scheduling shutting down...");
+    redisLog(REDIS_WARNING,"Received SIGTERM, scheduling shutdown...");
     server.shutdown_asap = 1;
 }
 
-void setupSigSegvAction(void) {
+void setupSignalHandlers(void) {
     struct sigaction act;
 
-    sigemptyset (&act.sa_mask);
-    /* When the SA_SIGINFO flag is set in sa_flags then sa_sigaction
-     * is used. Otherwise, sa_handler is used */
-    act.sa_flags = SA_NODEFER | SA_ONSTACK | SA_RESETHAND | SA_SIGINFO;
-    act.sa_sigaction = segvHandler;
-    sigaction (SIGSEGV, &act, NULL);
-    sigaction (SIGBUS, &act, NULL);
-    sigaction (SIGFPE, &act, NULL);
-    sigaction (SIGILL, &act, NULL);
-    sigaction (SIGBUS, &act, NULL);
-
+    /* When the SA_SIGINFO flag is set in sa_flags then sa_sigaction is used.
+     * Otherwise, sa_handler is used. */
+    sigemptyset(&act.sa_mask);
     act.sa_flags = SA_NODEFER | SA_ONSTACK | SA_RESETHAND;
     act.sa_handler = sigtermHandler;
-    sigaction (SIGTERM, &act, NULL);
+    sigaction(SIGTERM, &act, NULL);
+
+#ifdef HAVE_BACKTRACE
+    sigemptyset(&act.sa_mask);
+    act.sa_flags = SA_NODEFER | SA_ONSTACK | SA_RESETHAND | SA_SIGINFO;
+    act.sa_sigaction = sigsegvHandler;
+    sigaction(SIGSEGV, &act, NULL);
+    sigaction(SIGBUS, &act, NULL);
+    sigaction(SIGFPE, &act, NULL);
+    sigaction(SIGILL, &act, NULL);
+#endif
     return;
 }
 
-#else /* HAVE_BACKTRACE */
-void setupSigSegvAction(void) {
-}
-#endif /* HAVE_BACKTRACE */
-
 /* The End */

src/redis.h

@@ -81,6 +81,7 @@
 #define REDIS_ENCODING_LINKEDLIST 4 /* Encoded as regular linked list */
 #define REDIS_ENCODING_ZIPLIST 5 /* Encoded as ziplist */
 #define REDIS_ENCODING_INTSET 6  /* Encoded as intset */
+#define REDIS_ENCODING_SKIPLIST 7  /* Encoded as skiplist */
 
 /* Object types only used for dumping to disk */
 #define REDIS_EXPIRETIME 253
@@ -777,7 +778,7 @@ zskiplistNode *zslInsert(zskiplist *zsl, double score, robj *obj);
 /* Core functions */
 void freeMemoryIfNeeded(void);
 int processCommand(redisClient *c);
-void setupSigSegvAction(void);
+void setupSignalHandlers(void);
 struct redisCommand *lookupCommand(sds name);
 struct redisCommand *lookupCommandByCString(char *s);
 void call(redisClient *c, struct redisCommand *cmd);
@@ -1008,6 +1009,7 @@ void punsubscribeCommand(redisClient *c);
 void publishCommand(redisClient *c);
 void watchCommand(redisClient *c);
 void unwatchCommand(redisClient *c);
+void objectCommand(redisClient *c);
 
 #if defined(__GNUC__)
 void *calloc(size_t count, size_t size) __attribute__ ((deprecated));

src/sds.c

@@ -26,6 +26,12 @@
  * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
  * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
  * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * History:
+ *
+ * - 22 March 2011: History section created on top of sds.c
+ * - 22 March 2011: Fixed a problem with "\xab" escapes convertion in
+ *                  function sdssplitargs().
  */
 
 #define SDS_ABORT_ON_OOM
@@ -412,6 +418,37 @@ sds sdscatrepr(sds s, char *p, size_t len) {
     return sdscatlen(s,"\"",1);
 }
 
+/* Helper function for sdssplitargs() that returns non zero if 'c'
+ * is a valid hex digit. */
+int is_hex_digit(char c) {
+    return (c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') ||
+           (c >= 'A' && c <= 'F');
+}
+
+/* Helper function for sdssplitargs() that converts an hex digit into an
+ * integer from 0 to 15 */
+int hex_digit_to_int(char c) {
+    switch(c) {
+    case '0': return 0;
+    case '1': return 1;
+    case '2': return 2;
+    case '3': return 3;
+    case '4': return 4;
+    case '5': return 5;
+    case '6': return 6;
+    case '7': return 7;
+    case '8': return 8;
+    case '9': return 9;
+    case 'a': case 'A': return 10;
+    case 'b': case 'B': return 11;
+    case 'c': case 'C': return 12;
+    case 'd': case 'D': return 13;
+    case 'e': case 'E': return 14;
+    case 'f': case 'F': return 15;
+    default: return 0;
+    }
+}
+
 /* Split a line into arguments, where every argument can be in the
  * following programming-language REPL-alike form:
  *
@@ -441,7 +478,17 @@ sds *sdssplitargs(char *line, int *argc) {
             if (current == NULL) current = sdsempty();
             while(!done) {
                 if (inq) {
-                    if (*p == '\\' && *(p+1)) {
+                    if (*p == '\\' && *(p+1) == 'x' &&
+                                             is_hex_digit(*(p+2)) &&
+                                             is_hex_digit(*(p+3)))
+                    {
+                        unsigned char byte;
+
+                        byte = (hex_digit_to_int(*(p+2))*16)+
+                                hex_digit_to_int(*(p+3));
+                        current = sdscatlen(current,(char*)&byte,1);
+                        p += 3;
+                    } else if (*p == '\\' && *(p+1)) {
                         char c;
 
                         p++;

src/version.h

@@ -1 +1 @@
-#define REDIS_VERSION "2.2.2"
+#define REDIS_VERSION "2.2.6"

src/ziplist.c

@@ -398,12 +398,17 @@ static unsigned char *__ziplistCascadeUpdate(unsigned char *zl, unsigned char *p
             offset = p-zl;
             extra = rawlensize-next.prevrawlensize;
             zl = ziplistResize(zl,curlen+extra);
-            ZIPLIST_TAIL_OFFSET(zl) += extra;
             p = zl+offset;
 
-            /* Move the tail to the back. */
+            /* Current pointer and offset for next element. */
             np = p+rawlen;
             noffset = np-zl;
+
+            /* Update tail offset when next element is not the tail element. */
+            if ((zl+ZIPLIST_TAIL_OFFSET(zl)) != np)
+                ZIPLIST_TAIL_OFFSET(zl) += extra;
+
+            /* Move the tail to the back. */
             memmove(np+rawlensize,
                 np+next.prevrawlensize,
                 curlen-noffset-next.prevrawlensize-1);
@@ -877,7 +882,7 @@ void pop(unsigned char *zl, int where) {
     }
 }
 
-void randstring(char *target, unsigned int min, unsigned int max) {
+int randstring(char *target, unsigned int min, unsigned int max) {
     int p, len = min+rand()%(max-min+1);
     int minval, maxval;
     switch(rand() % 3) {
@@ -899,10 +904,9 @@ void randstring(char *target, unsigned int min, unsigned int max) {
 
     while(p < len)
         target[p++] = minval+rand()%(maxval-minval+1);
-    return;
+    return len;
 }
 
-
 int main(int argc, char **argv) {
     unsigned char *zl, *p;
     unsigned char *entry;
@@ -1235,6 +1239,7 @@ int main(int argc, char **argv) {
         int i,j,len,where;
         unsigned char *p;
         char buf[1024];
+        int buflen;
         list *ref;
         listNode *refnode;
 
@@ -1243,10 +1248,6 @@ int main(int argc, char **argv) {
         unsigned int slen;
         long long sval;
 
-        /* In the regression for the cascade bug, it was triggered
-         * with a random seed of 2. */
-        srand(2);
-
         for (i = 0; i < 20000; i++) {
             zl = ziplistNew();
             ref = listCreate();
@@ -1256,31 +1257,32 @@ int main(int argc, char **argv) {
             /* Create lists */
             for (j = 0; j < len; j++) {
                 where = (rand() & 1) ? ZIPLIST_HEAD : ZIPLIST_TAIL;
-                switch(rand() % 4) {
-                case 0:
-                    sprintf(buf,"%lld",(0LL + rand()) >> 20);
-                    break;
-                case 1:
-                    sprintf(buf,"%lld",(0LL + rand()));
-                    break;
-                case 2:
-                    sprintf(buf,"%lld",(0LL + rand()) << 20);
-                    break;
-                case 3:
-                    randstring(buf,0,256);
-                break;
-                default:
-                    assert(NULL);
+                if (rand() % 2) {
+                    buflen = randstring(buf,1,sizeof(buf)-1);
+                } else {
+                    switch(rand() % 3) {
+                    case 0:
+                        buflen = sprintf(buf,"%lld",(0LL + rand()) >> 20);
+                        break;
+                    case 1:
+                        buflen = sprintf(buf,"%lld",(0LL + rand()));
+                        break;
+                    case 2:
+                        buflen = sprintf(buf,"%lld",(0LL + rand()) << 20);
+                        break;
+                    default:
+                        assert(NULL);
+                    }
                 }
 
                 /* Add to ziplist */
-                zl = ziplistPush(zl, (unsigned char*)buf, strlen(buf), where);
+                zl = ziplistPush(zl, (unsigned char*)buf, buflen, where);
 
                 /* Add to reference list */
                 if (where == ZIPLIST_HEAD) {
-                    listAddNodeHead(ref,sdsnew(buf));
+                    listAddNodeHead(ref,sdsnewlen(buf, buflen));
                 } else if (where == ZIPLIST_TAIL) {
-                    listAddNodeTail(ref,sdsnew(buf));
+                    listAddNodeTail(ref,sdsnewlen(buf, buflen));
                 } else {
                     assert(NULL);
                 }
@@ -1295,12 +1297,13 @@ int main(int argc, char **argv) {
 
                 assert(ziplistGet(p,&sstr,&slen,&sval));
                 if (sstr == NULL) {
-                    sprintf(buf,"%lld",sval);
+                    buflen = sprintf(buf,"%lld",sval);
                 } else {
-                    memcpy(buf,sstr,slen);
-                    buf[slen] = '\0';
+                    buflen = slen;
+                    memcpy(buf,sstr,buflen);
+                    buf[buflen] = '\0';
                 }
-                assert(strcmp(buf,listNodeValue(refnode)) == 0);
+                assert(memcmp(buf,listNodeValue(refnode),buflen) == 0);
             }
             zfree(zl);
             listRelease(ref);

tests/integration/aof.tcl

@@ -31,13 +31,14 @@ tags {"aof"} {
     }
 
     start_server_aof [list dir $server_path] {
-        test {Unfinished MULTI: Server should not have been started} {
-            is_alive $srv
-        } {0}
+        test "Unfinished MULTI: Server should not have been started" {
+            assert_equal 0 [is_alive $srv]
+        }
 
-        test {Unfinished MULTI: Server should have logged an error} {
-            exec cat [dict get $srv stdout] | tail -n1
-        } {*Unexpected end of file reading the append only file*}
+        test "Unfinished MULTI: Server should have logged an error" {
+            set result [exec cat [dict get $srv stdout] | tail -n1]
+            assert_match "*Unexpected end of file reading the append only file*" $result
+        }
     }
 
     ## Test that the server exits when the AOF contains a short read
@@ -47,36 +48,57 @@ tags {"aof"} {
     }
 
     start_server_aof [list dir $server_path] {
-        test {Short read: Server should not have been started} {
-            is_alive $srv
-        } {0}
+        test "Short read: Server should not have been started" {
+            assert_equal 0 [is_alive $srv]
+        }
 
-        test {Short read: Server should have logged an error} {
-            exec cat [dict get $srv stdout] | tail -n1
-        } {*Bad file format reading the append only file*}
+        test "Short read: Server should have logged an error" {
+            set result [exec cat [dict get $srv stdout] | tail -n1]
+            assert_match "*Bad file format reading the append only file*" $result
+        }
     }
 
     ## Test that redis-check-aof indeed sees this AOF is not valid
-    test {Short read: Utility should confirm the AOF is not valid} {
+    test "Short read: Utility should confirm the AOF is not valid" {
         catch {
             exec src/redis-check-aof $aof_path
-        } str
-        set _ $str
-    } {*not valid*}
+        } result
+        assert_match "*not valid*" $result
+    }
 
-    test {Short read: Utility should be able to fix the AOF} {
-        exec echo y | src/redis-check-aof --fix $aof_path
-    } {*Successfully truncated AOF*}
+    test "Short read: Utility should be able to fix the AOF" {
+        set result [exec echo y | src/redis-check-aof --fix $aof_path]
+        assert_match "*Successfully truncated AOF*" $result
+    }
 
     ## Test that the server can be started using the truncated AOF
     start_server_aof [list dir $server_path] {
-        test {Fixed AOF: Server should have been started} {
-            is_alive $srv
-        } {1}
+        test "Fixed AOF: Server should have been started" {
+            assert_equal 1 [is_alive $srv]
+        }
 
-        test {Fixed AOF: Keyspace should contain values that were parsable} {
+        test "Fixed AOF: Keyspace should contain values that were parsable" {
             set client [redis [dict get $srv host] [dict get $srv port]]
-            list [$client get foo] [$client get bar]
-        } {hello {}}
+            assert_equal "hello" [$client get foo]
+            assert_equal "" [$client get bar]
+        }
+    }
+
+    ## Test that SPOP (that modifies the client its argc/argv) is correctly free'd
+    create_aof {
+        append_to_aof [formatCommand sadd set foo]
+        append_to_aof [formatCommand sadd set bar]
+        append_to_aof [formatCommand spop set]
+    }
+
+    start_server_aof [list dir $server_path] {
+        test "AOF+SPOP: Server should have been started" {
+            assert_equal 1 [is_alive $srv]
+        }
+
+        test "AOF+SPOP: Set should have 1 member" {
+            set client [redis [dict get $srv host] [dict get $srv port]]
+            assert_equal 1 [$client scard set]
+        }
     }
 }