summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOran Agra <oran@redislabs.com>2019-10-06 13:55:21 +0300
committerOran Agra <oran@redislabs.com>2019-10-07 09:09:32 +0300
commitd1a005ab3963c16b65e805675a76f0e40c723158 (patch)
treef923f84ec380a9e91f7e83972de5f825418cfb55
parentbd6044706641fa6370a90dc5347e6cd37f5930eb (diff)
downloadredis-d1a005ab3963c16b65e805675a76f0e40c723158.tar.gz
fix issues found by a static analyzer
cluster.c - stack buffer memory alignment The pointer 'buf' is cast to a more strictly aligned pointer type evict.c - lazyfree_lazy_eviction, lazyfree_lazy_eviction always called defrag.c - bug in dead code server.c - casting was missing parenthesis rax.c - indentation / newline suggested an 'else if' was intended
-rw-r--r--src/cluster.c32
-rw-r--r--src/defrag.c2
-rw-r--r--src/evict.c7
-rw-r--r--src/rax.c3
-rw-r--r--src/server.c2
5 files changed, 24 insertions, 22 deletions
diff --git a/src/cluster.c b/src/cluster.c
index 1e7dcd50e..93be2aa32 100644
--- a/src/cluster.c
+++ b/src/cluster.c
@@ -2140,7 +2140,7 @@ void clusterWriteHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
* full length of the packet. When a whole packet is in memory this function
* will call the function to process the packet. And so forth. */
void clusterReadHandler(aeEventLoop *el, int fd, void *privdata, int mask) {
- char buf[sizeof(clusterMsg)];
+ clusterMsg buf[1];
ssize_t nread;
clusterMsg *hdr;
clusterLink *link = (clusterLink*) privdata;
@@ -2517,7 +2517,8 @@ void clusterBroadcastPong(int target) {
*
* If link is NULL, then the message is broadcasted to the whole cluster. */
void clusterSendPublish(clusterLink *link, robj *channel, robj *message) {
- unsigned char buf[sizeof(clusterMsg)], *payload;
+ unsigned char *payload;
+ clusterMsg buf[1];
clusterMsg *hdr = (clusterMsg*) buf;
uint32_t totlen;
uint32_t channel_len, message_len;
@@ -2537,7 +2538,7 @@ void clusterSendPublish(clusterLink *link, robj *channel, robj *message) {
/* Try to use the local buffer if possible */
if (totlen < sizeof(buf)) {
- payload = buf;
+ payload = (unsigned char*)buf;
} else {
payload = zmalloc(totlen);
memcpy(payload,hdr,sizeof(*hdr));
@@ -2554,7 +2555,7 @@ void clusterSendPublish(clusterLink *link, robj *channel, robj *message) {
decrRefCount(channel);
decrRefCount(message);
- if (payload != buf) zfree(payload);
+ if (payload != (unsigned char*)buf) zfree(payload);
}
/* Send a FAIL message to all the nodes we are able to contact.
@@ -2563,7 +2564,7 @@ void clusterSendPublish(clusterLink *link, robj *channel, robj *message) {
* we switch the node state to CLUSTER_NODE_FAIL and ask all the other
* nodes to do the same ASAP. */
void clusterSendFail(char *nodename) {
- unsigned char buf[sizeof(clusterMsg)];
+ clusterMsg buf[1];
clusterMsg *hdr = (clusterMsg*) buf;
clusterBuildMessageHdr(hdr,CLUSTERMSG_TYPE_FAIL);
@@ -2575,7 +2576,7 @@ void clusterSendFail(char *nodename) {
* slots configuration. The node name, slots bitmap, and configEpoch info
* are included. */
void clusterSendUpdate(clusterLink *link, clusterNode *node) {
- unsigned char buf[sizeof(clusterMsg)];
+ clusterMsg buf[1];
clusterMsg *hdr = (clusterMsg*) buf;
if (link == NULL) return;
@@ -2583,7 +2584,7 @@ void clusterSendUpdate(clusterLink *link, clusterNode *node) {
memcpy(hdr->data.update.nodecfg.nodename,node->name,CLUSTER_NAMELEN);
hdr->data.update.nodecfg.configEpoch = htonu64(node->configEpoch);
memcpy(hdr->data.update.nodecfg.slots,node->slots,sizeof(node->slots));
- clusterSendMessage(link,buf,ntohl(hdr->totlen));
+ clusterSendMessage(link,(unsigned char*)buf,ntohl(hdr->totlen));
}
/* Send a MODULE message.
@@ -2591,7 +2592,8 @@ void clusterSendUpdate(clusterLink *link, clusterNode *node) {
* If link is NULL, then the message is broadcasted to the whole cluster. */
void clusterSendModule(clusterLink *link, uint64_t module_id, uint8_t type,
unsigned char *payload, uint32_t len) {
- unsigned char buf[sizeof(clusterMsg)], *heapbuf;
+ unsigned char *heapbuf;
+ clusterMsg buf[1];
clusterMsg *hdr = (clusterMsg*) buf;
uint32_t totlen;
@@ -2606,7 +2608,7 @@ void clusterSendModule(clusterLink *link, uint64_t module_id, uint8_t type,
/* Try to use the local buffer if possible */
if (totlen < sizeof(buf)) {
- heapbuf = buf;
+ heapbuf = (unsigned char*)buf;
} else {
heapbuf = zmalloc(totlen);
memcpy(heapbuf,hdr,sizeof(*hdr));
@@ -2619,7 +2621,7 @@ void clusterSendModule(clusterLink *link, uint64_t module_id, uint8_t type,
else
clusterBroadcastMessage(heapbuf,totlen);
- if (heapbuf != buf) zfree(heapbuf);
+ if (heapbuf != (unsigned char*)buf) zfree(heapbuf);
}
/* This function gets a cluster node ID string as target, the same way the nodes
@@ -2663,7 +2665,7 @@ void clusterPropagatePublish(robj *channel, robj *message) {
* Note that we send the failover request to everybody, master and slave nodes,
* but only the masters are supposed to reply to our query. */
void clusterRequestFailoverAuth(void) {
- unsigned char buf[sizeof(clusterMsg)];
+ clusterMsg buf[1];
clusterMsg *hdr = (clusterMsg*) buf;
uint32_t totlen;
@@ -2679,7 +2681,7 @@ void clusterRequestFailoverAuth(void) {
/* Send a FAILOVER_AUTH_ACK message to the specified node. */
void clusterSendFailoverAuth(clusterNode *node) {
- unsigned char buf[sizeof(clusterMsg)];
+ clusterMsg buf[1];
clusterMsg *hdr = (clusterMsg*) buf;
uint32_t totlen;
@@ -2687,12 +2689,12 @@ void clusterSendFailoverAuth(clusterNode *node) {
clusterBuildMessageHdr(hdr,CLUSTERMSG_TYPE_FAILOVER_AUTH_ACK);
totlen = sizeof(clusterMsg)-sizeof(union clusterMsgData);
hdr->totlen = htonl(totlen);
- clusterSendMessage(node->link,buf,totlen);
+ clusterSendMessage(node->link,(unsigned char*)buf,totlen);
}
/* Send a MFSTART message to the specified node. */
void clusterSendMFStart(clusterNode *node) {
- unsigned char buf[sizeof(clusterMsg)];
+ clusterMsg buf[1];
clusterMsg *hdr = (clusterMsg*) buf;
uint32_t totlen;
@@ -2700,7 +2702,7 @@ void clusterSendMFStart(clusterNode *node) {
clusterBuildMessageHdr(hdr,CLUSTERMSG_TYPE_MFSTART);
totlen = sizeof(clusterMsg)-sizeof(union clusterMsgData);
hdr->totlen = htonl(totlen);
- clusterSendMessage(node->link,buf,totlen);
+ clusterSendMessage(node->link,(unsigned char*)buf,totlen);
}
/* Vote for the node asking for our vote if there are the conditions. */
diff --git a/src/defrag.c b/src/defrag.c
index 50f6b41f2..e794c8e41 100644
--- a/src/defrag.c
+++ b/src/defrag.c
@@ -374,7 +374,7 @@ long activeDefragSdsListAndDict(list *l, dict *d, int dict_val_type) {
if ((newele = activeDefragStringOb(ele, &defragged)))
de->v.val = newele, defragged++;
} else if (dict_val_type == DEFRAG_SDS_DICT_VAL_VOID_PTR) {
- void *newptr, *ptr = ln->value;
+ void *newptr, *ptr = dictGetVal(de);
if ((newptr = activeDefragAlloc(ptr)))
ln->value = newptr, defragged++;
}
diff --git a/src/evict.c b/src/evict.c
index 176f4c362..71260c040 100644
--- a/src/evict.c
+++ b/src/evict.c
@@ -444,6 +444,7 @@ int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *lev
* Otehrwise if we are over the memory limit, but not enough memory
* was freed to return back under the limit, the function returns C_ERR. */
int freeMemoryIfNeeded(void) {
+ int keys_freed = 0;
/* By default replicas should ignore maxmemory
* and just be masters exact copies. */
if (server.masterhost && server.repl_slave_ignore_maxmemory) return C_OK;
@@ -467,7 +468,7 @@ int freeMemoryIfNeeded(void) {
latencyStartMonitor(latency);
while (mem_freed < mem_tofree) {
- int j, k, i, keys_freed = 0;
+ int j, k, i;
static unsigned int next_db = 0;
sds bestkey = NULL;
int bestdbid;
@@ -598,9 +599,7 @@ int freeMemoryIfNeeded(void) {
mem_freed = mem_tofree;
}
}
- }
-
- if (!keys_freed) {
+ } else {
latencyEndMonitor(latency);
latencyAddSampleIfNeeded("eviction-cycle",latency);
goto cant_free; /* nothing to free... */
diff --git a/src/rax.c b/src/rax.c
index b3c263dc4..be474b058 100644
--- a/src/rax.c
+++ b/src/rax.c
@@ -1791,7 +1791,8 @@ int raxCompare(raxIterator *iter, const char *op, unsigned char *key, size_t key
if (eq && key_len == iter->key_len) return 1;
else if (lt) return iter->key_len < key_len;
else if (gt) return iter->key_len > key_len;
- } if (cmp > 0) {
+ return 0;
+ } else if (cmp > 0) {
return gt ? 1 : 0;
} else /* (cmp < 0) */ {
return lt ? 1 : 0;
diff --git a/src/server.c b/src/server.c
index 593f98f3f..e089865f8 100644
--- a/src/server.c
+++ b/src/server.c
@@ -4278,7 +4278,7 @@ sds genRedisInfoString(char *section) {
if (server.repl_state != REPL_STATE_CONNECTED) {
info = sdscatprintf(info,
"master_link_down_since_seconds:%jd\r\n",
- (intmax_t)server.unixtime-server.repl_down_since);
+ (intmax_t)(server.unixtime-server.repl_down_since));
}
info = sdscatprintf(info,
"slave_priority:%d\r\n"