From 62a4b817c6e83eedf96a451f45dd943099258fd0 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 b1235ed4..3f880fe0 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 1207e060..064d2bec 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 afae8c58..4731c22e 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, ...)