Merge pull request #3191 from oranagra/minor_fix

Minor fixes found during merge and code review

src/adlist.c

@@ -242,7 +242,7 @@ listNode *listNext(listIter *iter)
 list *listDup(list *orig)
 {
     list *copy;
-    listIter *iter;
+    listIter iter;
     listNode *node;
 
     if ((copy = listCreate()) == NULL)
@@ -250,26 +250,23 @@ list *listDup(list *orig)
     copy->dup = orig->dup;
     copy->free = orig->free;
     copy->match = orig->match;
-    iter = listGetIterator(orig, AL_START_HEAD);
+    listRewind(orig, &iter);
-    while((node = listNext(iter)) != NULL) {
+    while((node = listNext(&iter)) != NULL) {
         void *value;
 
         if (copy->dup) {
             value = copy->dup(node->value);
             if (value == NULL) {
                 listRelease(copy);
-                listReleaseIterator(iter);
                 return NULL;
             }
         } else
             value = node->value;
         if (listAddNodeTail(copy, value) == NULL) {
             listRelease(copy);
-            listReleaseIterator(iter);
             return NULL;
         }
     }
-    listReleaseIterator(iter);
     return copy;
 }
 
@@ -284,24 +281,21 @@ list *listDup(list *orig)
  * NULL is returned. */
 listNode *listSearchKey(list *list, void *key)
 {
-    listIter *iter;
+    listIter iter;
     listNode *node;
 
-    iter = listGetIterator(list, AL_START_HEAD);
+    listRewind(list, &iter);
-    while((node = listNext(iter)) != NULL) {
+    while((node = listNext(&iter)) != NULL) {
         if (list->match) {
             if (list->match(node->value, key)) {
-                listReleaseIterator(iter);
                 return node;
             }
         } else {
             if (key == node->value) {
-                listReleaseIterator(iter);
                 return node;
             }
         }
     }
-    listReleaseIterator(iter);
     return NULL;
 }
 

src/ae_epoll.c

@@ -72,7 +72,7 @@ static void aeApiFree(aeEventLoop *eventLoop) {
 
 static int aeApiAddEvent(aeEventLoop *eventLoop, int fd, int mask) {
     aeApiState *state = eventLoop->apidata;
-    struct epoll_event ee;
+    struct epoll_event ee = {0}; /* avoid valgrind warning */
     /* If the fd was already monitored for some event, we need a MOD
      * operation. Otherwise we need an ADD operation. */
     int op = eventLoop->events[fd].mask == AE_NONE ?
@@ -82,7 +82,6 @@ static int aeApiAddEvent(aeEventLoop *eventLoop, int fd, int mask) {
     mask |= eventLoop->events[fd].mask; /* Merge old events */
     if (mask & AE_READABLE) ee.events |= EPOLLIN;
     if (mask & AE_WRITABLE) ee.events |= EPOLLOUT;
-    ee.data.u64 = 0; /* avoid valgrind warning */
     ee.data.fd = fd;
     if (epoll_ctl(state->epfd,op,fd,&ee) == -1) return -1;
     return 0;
@@ -90,13 +89,12 @@ static int aeApiAddEvent(aeEventLoop *eventLoop, int fd, int mask) {
 
 static void aeApiDelEvent(aeEventLoop *eventLoop, int fd, int delmask) {
     aeApiState *state = eventLoop->apidata;
-    struct epoll_event ee;
+    struct epoll_event ee = {0}; /* avoid valgrind warning */
     int mask = eventLoop->events[fd].mask & (~delmask);
 
     ee.events = 0;
     if (mask & AE_READABLE) ee.events |= EPOLLIN;
     if (mask & AE_WRITABLE) ee.events |= EPOLLOUT;
-    ee.data.u64 = 0; /* avoid valgrind warning */
     ee.data.fd = fd;
     if (mask != AE_NONE) {
         epoll_ctl(state->epfd,EPOLL_CTL_MOD,fd,&ee);

src/aof.c

@@ -721,6 +721,7 @@ loaded_ok: /* DB loaded, cleanup and return C_OK to the caller. */
 
 readerr: /* Read error. If feof(fp) is true, fall through to unexpected EOF. */
     if (!feof(fp)) {
+        if (fakeClient) freeFakeClient(fakeClient); /* avoid valgrind warning */
         serverLog(LL_WARNING,"Unrecoverable error reading the append only file: %s", strerror(errno));
         exit(1);
     }
@@ -750,10 +751,12 @@ uxeof: /* Unexpected AOF end of file. */
             }
         }
     }
+    if (fakeClient) freeFakeClient(fakeClient); /* avoid valgrind warning */
     serverLog(LL_WARNING,"Unexpected end of file reading the append only file. You can: 1) Make a backup of your AOF file, then use ./redis-check-aof --fix <filename>. 2) Alternatively you can set the 'aof-load-truncated' configuration option to yes and restart the server.");
     exit(1);
 
 fmterr: /* Format error. */
+    if (fakeClient) freeFakeClient(fakeClient); /* avoid valgrind warning */
     serverLog(LL_WARNING,"Bad file format reading the append only file: make a backup of your AOF file, then use ./redis-check-aof --fix <filename>");
     exit(1);
 }
@@ -763,7 +766,7 @@ fmterr: /* Format error. */
  * ------------------------------------------------------------------------- */
 
 /* Delegate writing an object to writing a bulk string or bulk long long.
- * This is not placed in rio.c since that adds the redis.h dependency. */
+ * This is not placed in rio.c since that adds the server.h dependency. */
 int rioWriteBulkObject(rio *r, robj *obj) {
     /* Avoid using getDecodedObject to help copy-on-write (we are often
      * in a child process when this function is called). */

src/config.c

@@ -928,6 +928,8 @@ void configSetCommand(client *c) {
       "lazyfree-lazy-server-del",server.lazyfree_lazy_server_del) {
     } config_set_bool_field(
       "slave-lazy-flush",server.repl_slave_lazy_flush) {
+    } config_set_bool_field(
+      "no-appendfsync-on-rewrite",server.aof_no_fsync_on_rewrite) {
 
     /* Numerical fields.
      * config_set_numerical_field(name,var,min,max) */

src/db.c

@@ -116,7 +116,7 @@ void dbAdd(redisDb *db, robj *key, robj *val) {
     sds copy = sdsdup(key->ptr);
     int retval = dictAdd(db->dict, copy, val);
 
-    serverAssertWithInfo(NULL,key,retval == C_OK);
+    serverAssertWithInfo(NULL,key,retval == DICT_OK);
     if (val->type == OBJ_LIST) signalListAsReady(db, key);
     if (server.cluster_enabled) slotToKeyAdd(key);
  }

src/debug.c

@@ -250,11 +250,13 @@ void computeDatasetDigest(unsigned char *final) {
     }
 }
 
+#if defined(USE_JEMALLOC)
 void inputCatSds(void *result, const char *str) {
     /* result is actually a (sds *), so re-cast it here */
     sds *info = (sds *)result;
     *info = sdscat(*info, str);
 }
+#endif
 
 void debugCommand(client *c) {
     if (!strcasecmp(c->argv[1]->ptr,"segfault")) {
@@ -397,6 +399,7 @@ void debugCommand(client *c) {
             snprintf(buf,sizeof(buf),"value:%lu",j);
             val = createStringObject(buf,strlen(buf));
             dbAdd(c->db,key,val);
+            signalModifiedKey(c->db,key);
             decrRefCount(key);
         }
         addReply(c,shared.ok);
@@ -474,8 +477,20 @@ void debugCommand(client *c) {
             sds info = sdsempty();
             je_malloc_stats_print(inputCatSds, &info, NULL);
             addReplyBulkSds(c, info);
+        } else if (!strcasecmp(c->argv[2]->ptr, "purge")) {
+            char tmp[32];
+            unsigned narenas = 0;
+            size_t sz = sizeof(unsigned);
+            if (!je_mallctl("arenas.narenas", &narenas, &sz, NULL, 0)) {
+                sprintf(tmp, "arena.%d.purge", narenas);
+                if (!je_mallctl(tmp, NULL, 0, NULL, 0)) {
+                    addReply(c, shared.ok);
+                    return;
+                }
+            }
+            addReplyError(c, "Error purging dirty pages");
         } else {
-            addReplyErrorFormat(c, "Valid jemalloc debug fields: info");
+            addReplyErrorFormat(c, "Valid jemalloc debug fields: info, purge");
         }
 #else
         addReplyErrorFormat(c, "jemalloc support not available");

src/dict.c

@@ -424,7 +424,7 @@ static int dictGenericDelete(dict *d, const void *key, int nofree)
         he = d->ht[table].table[idx];
         prevHe = NULL;
         while(he) {
-            if (dictCompareKeys(d, key, he->key)) {
+            if (key==he->key || dictCompareKeys(d, key, he->key)) {
                 /* Unlink the element from the list */
                 if (prevHe)
                     prevHe->next = he->next;
@@ -494,14 +494,14 @@ dictEntry *dictFind(dict *d, const void *key)
     dictEntry *he;
     unsigned int h, idx, table;
 
-    if (d->ht[0].size == 0) return NULL; /* We don't have a table at all */
+    if (d->ht[0].used + d->ht[1].used == 0) return NULL; /* dict is empty */
     if (dictIsRehashing(d)) _dictRehashStep(d);
     h = dictHashKey(d, key);
     for (table = 0; table <= 1; table++) {
         idx = h & d->ht[table].sizemask;
         he = d->ht[table].table[idx];
         while(he) {
-            if (dictCompareKeys(d, key, he->key))
+            if (key==he->key || dictCompareKeys(d, key, he->key))
                 return he;
             he = he->next;
         }
@@ -981,7 +981,7 @@ static int _dictKeyIndex(dict *d, const void *key)
         /* Search if this slot does not already contain the given key */
         he = d->ht[table].table[idx];
         while(he) {
-            if (dictCompareKeys(d, key, he->key))
+            if (key==he->key || dictCompareKeys(d, key, he->key))
                 return -1;
             he = he->next;
         }

src/networking.c

@@ -1204,10 +1204,10 @@ int processMultibulkBuffer(client *c) {
             {
                 c->argv[c->argc++] = createObject(OBJ_STRING,c->querybuf);
                 sdsIncrLen(c->querybuf,-2); /* remove CRLF */
-                c->querybuf = sdsempty();
                 /* Assume that if we saw a fat argument we'll see another one
                  * likely... */
-                c->querybuf = sdsMakeRoomFor(c->querybuf,c->bulklen+2);
+                c->querybuf = sdsnewlen(NULL,c->bulklen+2);
+                sdsclear(c->querybuf);
                 pos = 0;
             } else {
                 c->argv[c->argc++] =
@@ -1268,6 +1268,9 @@ void processInputBuffer(client *c) {
             /* Only reset the client when the command was executed. */
             if (processCommand(c) == C_OK)
                 resetClient(c);
+            /* freeMemoryIfNeeded may flush slave output buffers. This may result
+             * into a slave, that may be the active client, to be freed. */
+            if (server.current_client == NULL) break;
         }
     }
     server.current_client = NULL;
@@ -1440,9 +1443,8 @@ sds getAllClientsInfoString(void) {
     listNode *ln;
     listIter li;
     client *client;
-    sds o = sdsempty();
+    sds o = sdsnewlen(NULL,200*listLength(server.clients));
-
+    sdsclear(o);
-    o = sdsMakeRoomFor(o,200*listLength(server.clients));
     listRewind(server.clients,&li);
     while ((ln = listNext(&li)) != NULL) {
         client = listNodeValue(ln);
@@ -1884,7 +1886,7 @@ int clientsArePaused(void) {
  * and so forth.
  *
  * It calls the event loop in order to process a few events. Specifically we
- * try to call the event loop for times as long as we receive acknowledge that
+ * try to call the event loop 4 times as long as we receive acknowledge that
  * some event was processed, in order to go forward with the accept, read,
  * write, close sequence needed to serve a client.
  *

src/rdb.c

@@ -391,9 +391,9 @@ int rdbSaveStringObject(rio *rdb, robj *obj) {
  *               efficient. When this flag is passed the function
  *               no longer guarantees that obj->ptr is an SDS string.
  * RDB_LOAD_PLAIN: Return a plain string allocated with zmalloc()
- *                 instead of a Redis object.
+ *                 instead of a Redis object with an sds in it.
  * RDB_LOAD_SDS: Return an SDS string instead of a Redis object.
- */
+*/
 void *rdbGenericLoadStringObject(rio *rdb, int flags) {
     int encode = flags & RDB_LOAD_ENC;
     int plain = flags & RDB_LOAD_PLAIN;
@@ -779,7 +779,7 @@ int rdbSaveRio(rio *rdb, int *error) {
         db_size = (dictSize(db->dict) <= UINT32_MAX) ?
                                 dictSize(db->dict) :
                                 UINT32_MAX;
-        expires_size = (dictSize(db->dict) <= UINT32_MAX) ?
+        expires_size = (dictSize(db->expires) <= UINT32_MAX) ?
                                 dictSize(db->expires) :
                                 UINT32_MAX;
         if (rdbSaveType(rdb,RDB_OPCODE_RESIZEDB) == -1) goto werr;
@@ -1594,7 +1594,7 @@ int rdbSaveToSlavesSockets(void) {
             clientids[numfds] = slave->id;
             fds[numfds++] = slave->fd;
             replicationSetupSlaveForFullResync(slave,getPsyncInitialOffset());
-            /* Put the socket in non-blocking mode to simplify RDB transfer.
+            /* Put the socket in blocking mode to simplify RDB transfer.
              * We'll restore it when the children returns (since duped socket
              * will share the O_NONBLOCK attribute with the parent). */
             anetBlock(NULL,slave->fd);
@@ -1668,6 +1668,7 @@ int rdbSaveToSlavesSockets(void) {
             zfree(msg);
         }
         zfree(clientids);
+        rioFreeFdset(&slave_sockets);
         exitFromChild((retval == C_OK) ? 0 : 1);
     } else {
         /* Parent */

src/redis-cli.c

@@ -787,7 +787,7 @@ static int cliSendCommand(int argc, char **argv, int repeat) {
     output_raw = 0;
     if (!strcasecmp(command,"info") ||
         (argc >= 2 && !strcasecmp(command,"debug") &&
-                      (!strcasecmp(argv[1],"jemalloc") ||
+                      ((!strcasecmp(argv[1],"jemalloc") && !strcasecmp(argv[2],"info")) ||
                        !strcasecmp(argv[1],"htstats"))) ||
         (argc == 2 && !strcasecmp(command,"cluster") &&
                       (!strcasecmp(argv[1],"nodes") ||

src/rio.c

@@ -163,7 +163,7 @@ void rioInitWithFile(rio *r, FILE *fp) {
  * The function returns success as long as we are able to correctly write
  * to at least one file descriptor.
  *
- * When buf is NULL adn len is 0, the function performs a flush operation
+ * When buf is NULL and len is 0, the function performs a flush operation
  * if there is some pending buffer, so this function is also used in order
  * to implement rioFdsetFlush(). */
 static size_t rioFdsetWrite(rio *r, const void *buf, size_t len) {
@@ -176,7 +176,7 @@ static size_t rioFdsetWrite(rio *r, const void *buf, size_t len) {
      * a given size, we actually write to the sockets. */
     if (len) {
         r->io.fdset.buf = sdscatlen(r->io.fdset.buf,buf,len);
-        len = 0; /* Prevent entering the while belove if we don't flush. */
+        len = 0; /* Prevent entering the while below if we don't flush. */
         if (sdslen(r->io.fdset.buf) > PROTO_IOBUF_LEN) doflush = 1;
     }
 
@@ -276,6 +276,7 @@ void rioInitWithFdset(rio *r, int *fds, int numfds) {
     r->io.fdset.buf = sdsempty();
 }
 
+/* release the rio stream. */
 void rioFreeFdset(rio *r) {
     zfree(r->io.fdset.fds);
     zfree(r->io.fdset.state);

src/rio.h

@@ -128,6 +128,8 @@ void rioInitWithFile(rio *r, FILE *fp);
 void rioInitWithBuffer(rio *r, sds s);
 void rioInitWithFdset(rio *r, int *fds, int numfds);
 
+void rioFreeFdset(rio *r);
+
 size_t rioWriteBulkCount(rio *r, char prefix, int count);
 size_t rioWriteBulkString(rio *r, const char *buf, size_t len);
 size_t rioWriteBulkLongLong(rio *r, long long l);

src/server.h

@@ -487,7 +487,7 @@ typedef struct redisObject {
     _var.type = OBJ_STRING; \
     _var.encoding = OBJ_ENCODING_RAW; \
     _var.ptr = _ptr; \
-} while(0);
+} while(0)
 
 /* To improve the quality of the LRU approximation we take a set of keys
  * that are good candidate for eviction across freeMemoryIfNeeded() calls.
@@ -1130,7 +1130,6 @@ void copyClientOutputBuffer(client *dst, client *src);
 void *dupClientReplyValue(void *o);
 void getClientsMaxBuffers(unsigned long *longest_output_list,
                           unsigned long *biggest_input_buffer);
-void formatPeerId(char *peerid, size_t peerid_len, char *ip, int port);
 char *getClientPeerId(client *client);
 sds catClientInfoString(sds s, client *client);
 sds getAllClientsInfoString(void);

tests/support/test.tcl

@@ -37,13 +37,7 @@ proc assert_error {pattern code} {
 }
 
 proc assert_encoding {enc key} {
-    # Swapped out values don't have an encoding, so make sure that
-    # the value is swapped in before checking the encoding.
     set dbg [r debug object $key]
-    while {[string match "* swapped at:*" $dbg]} {
-        r debug swapin $key
-        set dbg [r debug object $key]
-    }
     assert_match "* encoding:$enc *" $dbg
 }
 

tests/unit/bitops.tcl

@@ -88,7 +88,7 @@ start_server {tags {"bitops"}} {
     } {ERR*syntax*}
 
     test {BITCOUNT regression test for github issue #582} {
-        r del str
+        r del foo
         r setbit foo 0 1
         if {[catch {r bitcount foo 0 4294967296} e]} {
             assert_match {*ERR*out of range*} $e

tests/unit/other.tcl

@@ -194,6 +194,7 @@ start_server {tags {"other"}} {
     }
 
     test {APPEND basics} {
+        r del foo
         list [r append foo bar] [r get foo] \
              [r append foo 100] [r get foo]
     } {3 bar 6 bar100}

tests/unit/scripting.tcl

@@ -62,18 +62,19 @@ start_server {tags {"scripting"}} {
     } {NOSCRIPT*}
 
     test {EVAL - Redis integer -> Lua type conversion} {
+        r set x 0
         r eval {
-            local foo = redis.pcall('incr','x')
+            local foo = redis.pcall('incr',KEYS[1])
             return {type(foo),foo}
-        } 0
+        } 1 x
     } {number 1}
 
     test {EVAL - Redis bulk -> Lua type conversion} {
         r set mykey myval
         r eval {
-            local foo = redis.pcall('get','mykey')
+            local foo = redis.pcall('get',KEYS[1])
             return {type(foo),foo}
-        } 0
+        } 1 mykey
     } {string myval}
 
     test {EVAL - Redis multi bulk -> Lua type conversion} {
@@ -82,39 +83,39 @@ start_server {tags {"scripting"}} {
         r rpush mylist b
         r rpush mylist c
         r eval {
-            local foo = redis.pcall('lrange','mylist',0,-1)
+            local foo = redis.pcall('lrange',KEYS[1],0,-1)
             return {type(foo),foo[1],foo[2],foo[3],# foo}
-        } 0
+        } 1 mylist
     } {table a b c 3}
 
     test {EVAL - Redis status reply -> Lua type conversion} {
         r eval {
-            local foo = redis.pcall('set','mykey','myval')
+            local foo = redis.pcall('set',KEYS[1],'myval')
             return {type(foo),foo['ok']}
-        } 0
+        } 1 mykey
     } {table OK}
 
     test {EVAL - Redis error reply -> Lua type conversion} {
         r set mykey myval
         r eval {
-            local foo = redis.pcall('incr','mykey')
+            local foo = redis.pcall('incr',KEYS[1])
             return {type(foo),foo['err']}
-        } 0
+        } 1 mykey
     } {table {ERR value is not an integer or out of range}}
 
     test {EVAL - Redis nil bulk reply -> Lua type conversion} {
         r del mykey
         r eval {
-            local foo = redis.pcall('get','mykey')
+            local foo = redis.pcall('get',KEYS[1])
             return {type(foo),foo == false}
-        } 0
+        } 1 mykey
     } {boolean 1}
 
     test {EVAL - Is the Lua client using the currently selected DB?} {
         r set mykey "this is DB 9"
         r select 10
         r set mykey "this is DB 10"
-        r eval {return redis.pcall('get','mykey')} 0
+        r eval {return redis.pcall('get',KEYS[1])} 1 mykey
     } {this is DB 10}
 
     test {EVAL - SELECT inside Lua should not affect the caller} {