From c170365dcf4698080aeac32e4209ebb2ef7c6c6c Mon Sep 17 00:00:00 2001 From: Yash Ladha <201551061@iiitvadodara.ac.in> Date: Thu, 12 Nov 2020 14:25:51 +0530 Subject: [PATCH] cleanup: move list pop logic to single function (#7997) BLPOP when there are elements in the list works in the same way as LPOP does. Due to this they also does the same repetitive action and logic for the same is written at two different places. This is a bad code practice as the one needs the context to change the BLPOP list pop code as well when the LPOP code gets changed. Separated the generic logic from LPOP to a function that is being used by the BLPOP code as well. --- src/server.h | 1 + src/t_list.c | 43 +++++++++++++++++++------------------------ 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/server.h b/src/server.h index 1ae96aee5..8c179293a 100644 --- a/src/server.h +++ b/src/server.h @@ -1793,6 +1793,7 @@ void listTypeDelete(listTypeIterator *iter, listTypeEntry *entry); void listTypeConvert(robj *subject, int enc); void unblockClientWaitingData(client *c); void popGenericCommand(client *c, int where); +void listElementsRemoved(client *c, robj *key, int where, robj *o); /* MULTI/EXEC/WATCH... */ void unwatchAllKeys(client *c); diff --git a/src/t_list.c b/src/t_list.c index e7a69b1e9..25dda3178 100644 --- a/src/t_list.c +++ b/src/t_list.c @@ -363,26 +363,30 @@ void lsetCommand(client *c) { } } -void popGenericCommand(client *c, int where) { - robj *o = lookupKeyWriteOrReply(c,c->argv[1],shared.null[c->resp]); - if (o == NULL || checkType(c,o,OBJ_LIST)) return; +void listElementsRemoved(client *c, robj *key, int where, robj *o) { + char *event = (where == LIST_HEAD) ? "lpop" : "rpop"; - robj *value = listTypePop(o,where); + notifyKeyspaceEvent(NOTIFY_LIST, event, key, c->db->id); + if (listTypeLength(o) == 0) { + notifyKeyspaceEvent(NOTIFY_GENERIC, "del", key, c->db->id); + dbDelete(c->db, key); + } + signalModifiedKey(c, c->db, key); + server.dirty++; +} + +void popGenericCommand(client *c, int where) { + robj *o = lookupKeyWriteOrReply(c, c->argv[1], shared.null[c->resp]); + if (o == NULL || checkType(c, o, OBJ_LIST)) + return; + + robj *value = listTypePop(o, where); if (value == NULL) { addReplyNull(c); } else { - char *event = (where == LIST_HEAD) ? "lpop" : "rpop"; - addReplyBulk(c,value); decrRefCount(value); - notifyKeyspaceEvent(NOTIFY_LIST,event,c->argv[1],c->db->id); - if (listTypeLength(o) == 0) { - notifyKeyspaceEvent(NOTIFY_GENERIC,"del", - c->argv[1],c->db->id); - dbDelete(c->db,c->argv[1]); - } - signalModifiedKey(c,c->db,c->argv[1]); - server.dirty++; + listElementsRemoved(c,c->argv[1],where,o); } } @@ -859,7 +863,6 @@ void blockingPopGenericCommand(client *c, int where) { } else { if (listTypeLength(o) != 0) { /* Non empty list, this is like a normal [LR]POP. */ - char *event = (where == LIST_HEAD) ? "lpop" : "rpop"; robj *value = listTypePop(o,where); serverAssert(value != NULL); @@ -867,15 +870,7 @@ void blockingPopGenericCommand(client *c, int where) { addReplyBulk(c,c->argv[j]); addReplyBulk(c,value); decrRefCount(value); - notifyKeyspaceEvent(NOTIFY_LIST,event, - c->argv[j],c->db->id); - if (listTypeLength(o) == 0) { - dbDelete(c->db,c->argv[j]); - notifyKeyspaceEvent(NOTIFY_GENERIC,"del", - c->argv[j],c->db->id); - } - signalModifiedKey(c,c->db,c->argv[j]); - server.dirty++; + listElementsRemoved(c,c->argv[j],where,o); /* Replicate it as an [LR]POP instead of B[LR]POP. */ rewriteClientCommandVector(c,2,