From 5e3880a492efd6c305d7bde5be44c1de72e15cb0 Mon Sep 17 00:00:00 2001
From: Oran Agra <oran@monfort.co.il>
Date: Mon, 25 Apr 2016 16:49:57 +0300
Subject: [PATCH] various cleanups and minor fixes
---
src/adlist.c | 18 ++++++------------
src/ae_epoll.c | 6 ++----
src/aof.c | 5 ++++-
src/db.c | 2 +-
src/rdb.c | 7 ++++---
src/rio.c | 5 +++--
src/rio.h | 2 ++
src/server.h | 3 +--
tests/support/test.tcl | 6 ------
tests/unit/bitops.tcl | 2 +-
tests/unit/other.tcl | 1 +
tests/unit/scripting.tcl | 27 ++++++++++++++-------------
12 files changed, 39 insertions(+), 45 deletions(-)
diff --git a/src/adlist.c b/src/adlist.c
index b4cc785b..f171d3ec 100644
--- a/src/adlist.c
+++ b/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);
- while((node = listNext(iter)) != NULL) {
+ listRewind(orig, &iter);
+ 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);
- while((node = listNext(iter)) != NULL) {
+ listRewind(list, &iter);
+ 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;
}
diff --git a/src/ae_epoll.c b/src/ae_epoll.c
index da9c7b90..410aac70 100644
--- a/src/ae_epoll.c
+++ b/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);
diff --git a/src/aof.c b/src/aof.c
index 161f527c..9df1e9b9 100644
--- a/src/aof.c
+++ b/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). */
diff --git a/src/db.c b/src/db.c
index d8352ec3..60acb43c 100644
--- a/src/db.c
+++ b/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);
}
diff --git a/src/rdb.c b/src/rdb.c
index 7fc0a443..57b75927 100644
--- a/src/rdb.c
+++ b/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;
@@ -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 */
diff --git a/src/rio.c b/src/rio.c
index 37b2b037..9c7220fc 100644
--- a/src/rio.c
+++ b/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);
diff --git a/src/rio.h b/src/rio.h
index e5fa0cd3..711308ce 100644
--- a/src/rio.h
+++ b/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);
diff --git a/src/server.h b/src/server.h
index c819ccb2..258741a7 100644
--- a/src/server.h
+++ b/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);
diff --git a/tests/support/test.tcl b/tests/support/test.tcl
index 31371c56..d60eb3c4 100644
--- a/tests/support/test.tcl
+++ b/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
}
diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl
index 309a5d46..30aa832c 100644
--- a/tests/unit/bitops.tcl
+++ b/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
diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl
index a53f3f5c..2f577393 100644
--- a/tests/unit/other.tcl
+++ b/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}
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index 0efe86ad..07553b1d 100644
--- a/tests/unit/scripting.tcl
+++ b/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} {