From e0df883f4597212fa11b37047989c7b663c5b80a Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 5 Dec 2017 15:59:56 +0100
Subject: [PATCH] add linkClient(): adds the client and caches the list node.

We have this operation in two places: when caching the master and
when linking a new client after the client creation. By having an API
for this we avoid incurring in errors when modifying one of the two
places forgetting the other. The function is also a good place where to
document why we cache the linked list node.

Related to #4497 and #4210.
---
 src/networking.c  | 18 ++++++++++++------
 src/replication.c |  3 +--
 src/server.h      |  1 +
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/src/networking.c b/src/networking.c
index b1235ed41..3f880fe06 100644
--- a/src/networking.c
+++ b/src/networking.c
@@ -67,6 +67,16 @@ int listMatchObjects(void *a, void *b) {
     return equalStringObjects(a,b);
 }
 
+/* This function links the client to the global linked list of clients.
+ * unlinkClient() does the opposite, among other things. */
+void linkClient(client *c) {
+    listAddNodeTail(server.clients,c);
+    /* Note that we remember the linked list node where the client is stored,
+     * this way removing the client in unlinkClient() will not require
+     * a linear scan, but just a constant time operation. */
+    c->client_list_node = listLast(server.clients);
+}
+
 client *createClient(int fd) {
     client *c = zmalloc(sizeof(client));
 
@@ -134,14 +144,10 @@ client *createClient(int fd) {
     c->pubsub_channels = dictCreate(&objectKeyPointerValueDictType,NULL);
     c->pubsub_patterns = listCreate();
     c->peerid = NULL;
+    c->client_list_node = NULL;
     listSetFreeMethod(c->pubsub_patterns,decrRefCountVoid);
     listSetMatchMethod(c->pubsub_patterns,listMatchObjects);
-    if (fd != -1) {
-        listAddNodeTail(server.clients,c);
-        c->client_list_node = listLast(server.clients);
-    } else {
-        c->client_list_node = NULL;
-    }
+    if (fd != -1) linkClient(c);
     initClientMultiState(c);
     return c;
 }
diff --git a/src/replication.c b/src/replication.c
index 1207e060b..064d2bece 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -2205,8 +2205,7 @@ void replicationResurrectCachedMaster(int newfd) {
     server.repl_state = REPL_STATE_CONNECTED;
 
     /* Re-add to the list of clients. */
-    listAddNodeTail(server.clients,server.master);
-    server.master->client_list_node = listLast(server.clients);
+    linkClient(server.master);
     if (aeCreateFileEvent(server.el, newfd, AE_READABLE,
                           readQueryFromClient, server.master)) {
         serverLog(LL_WARNING,"Error resurrecting the cached master, impossible to add the readable handler: %s", strerror(errno));
diff --git a/src/server.h b/src/server.h
index afae8c58c..4731c22ec 100644
--- a/src/server.h
+++ b/src/server.h
@@ -1399,6 +1399,7 @@ int handleClientsWithPendingWrites(void);
 int clientHasPendingReplies(client *c);
 void unlinkClient(client *c);
 int writeToClient(int fd, client *c, int handler_installed);
+void linkClient(client *c);
 
 #ifdef __GNUC__
 void addReplyErrorFormat(client *c, const char *fmt, ...)