From f7f68c654a90c4d3e84121c4555a56682b02b2de Mon Sep 17 00:00:00 2001 From: Ping Xie <11568491+PingXie@users.noreply.github.com> Date: Wed, 16 Feb 2022 13:35:49 -0800 Subject: [PATCH] Use sds for clusterNode.hostname (#10290) * Provide a fallback static_assert implementation * Use sds for clusterNode.hostname --- src/cluster.c | 48 +++++++++++++++++++++--------------------------- src/cluster.h | 7 ++----- src/server.h | 4 ++++ 3 files changed, 27 insertions(+), 32 deletions(-) diff --git a/src/cluster.c b/src/cluster.c index fed3c2072..72438837a 100644 --- a/src/cluster.c +++ b/src/cluster.c @@ -243,10 +243,9 @@ int clusterLoadConfig(char *filename) { if (hostname) { *hostname = '\0'; hostname++; - zfree(n->hostname); - n->hostname = zstrdup(hostname); - } else { - n->hostname = NULL; + n->hostname = sdscpy(n->hostname, hostname); + } else if (sdslen(n->hostname) != 0) { + sdsclear(n->hostname); } /* The plaintext port for client in a TLS cluster (n->pport) is not @@ -570,20 +569,15 @@ void clusterUpdateMyselfIp(void) { /* Update the hostname for the specified node with the provided C string. */ static void updateAnnouncedHostname(clusterNode *node, char *new) { - if (!node->hostname && !new) { - return; - } - /* Previous and new hostname are the same, no need to update. */ - if (new && node->hostname && !strcmp(new, node->hostname)) { + if (new && !strcmp(new, node->hostname)) { return; } - if (node->hostname) zfree(node->hostname); if (new) { - node->hostname = zstrdup(new); - } else { - node->hostname = NULL; + node->hostname = sdscpy(node->hostname, new); + } else if (sdslen(node->hostname) != 0) { + sdsclear(node->hostname); } } @@ -959,7 +953,7 @@ clusterNode *createClusterNode(char *nodename, int flags) { node->link = NULL; node->inbound_link = NULL; memset(node->ip,0,sizeof(node->ip)); - node->hostname = NULL; + node->hostname = sdsempty(); node->port = 0; node->cport = 0; node->pport = 0; @@ -1125,7 +1119,7 @@ void freeClusterNode(clusterNode *n) { nodename = sdsnewlen(n->name, CLUSTER_NAMELEN); serverAssert(dictDelete(server.cluster->nodes,nodename) == DICT_OK); sdsfree(nodename); - zfree(n->hostname); + sdsfree(n->hostname); /* Release links and associated data structures. */ if (n->link) freeClusterLink(n->link); @@ -1947,9 +1941,9 @@ static clusterMsgPingExt *getNextPingExt(clusterMsgPingExt *ext) { * will be 8 byte padded. */ int getHostnamePingExtSize() { /* If hostname is not set, we don't send this extension */ - if (!myself->hostname) return 0; + if (sdslen(myself->hostname) == 0) return 0; - int totlen = sizeof(clusterMsgPingExt) + EIGHT_BYTE_ALIGN(strlen(myself->hostname) + 1); + int totlen = sizeof(clusterMsgPingExt) + EIGHT_BYTE_ALIGN(sdslen(myself->hostname) + 1); return totlen; } @@ -1958,19 +1952,18 @@ int getHostnamePingExtSize() { * will return the amount of bytes written. */ int writeHostnamePingExt(clusterMsgPingExt **cursor) { /* If hostname is not set, we don't send this extension */ - if (!myself->hostname) return 0; + if (sdslen(myself->hostname) == 0) return 0; /* Add the hostname information at the extension cursor */ clusterMsgPingExtHostname *ext = &(*cursor)->ext[0].hostname; - size_t hostname_len = strlen(myself->hostname); - memcpy(ext->hostname, myself->hostname, hostname_len); + memcpy(ext->hostname, myself->hostname, sdslen(myself->hostname)); uint32_t extension_size = getHostnamePingExtSize(); /* Move the write cursor */ (*cursor)->type = CLUSTERMSG_EXT_TYPE_HOSTNAME; (*cursor)->length = htonl(extension_size); /* Make sure the string is NULL terminated by adding 1 */ - *cursor = (clusterMsgPingExt *) (ext->hostname + EIGHT_BYTE_ALIGN(strlen(myself->hostname) + 1)); + *cursor = (clusterMsgPingExt *) (ext->hostname + EIGHT_BYTE_ALIGN(sdslen(myself->hostname) + 1)); return extension_size; } @@ -2975,7 +2968,7 @@ void clusterSendPing(clusterLink *link, int type) { /* Set the initial extension position */ clusterMsgPingExt *cursor = getInitialPingExt(hdr, gossipcount); /* Add in the extensions */ - if (myself->hostname) { + if (sdslen(myself->hostname) != 0) { hdr->mflags[0] |= CLUSTERMSG_FLAG0_EXT_DATA; totlen += writeHostnamePingExt(&cursor); extensions++; @@ -3959,7 +3952,8 @@ void clusterCron(void) { iteration++; /* Number of times this function was called so far. */ - updateAnnouncedHostname(myself, server.cluster_announce_hostname); + clusterUpdateMyselfHostname(); + /* The handshake timeout is the time after which a handshake node that was * not turned into a normal node is removed from the nodes. Usually it is * just the NODE_TIMEOUT value, but when NODE_TIMEOUT is too small we use @@ -4578,7 +4572,7 @@ sds clusterGenNodeDescription(clusterNode *node, int use_pport) { /* Node coordinates */ ci = sdscatlen(sdsempty(),node->name,CLUSTER_NAMELEN); - if (node->hostname) { + if (sdslen(node->hostname) != 0) { ci = sdscatfmt(ci," %s:%i@%i,%s ", node->ip, port, @@ -4804,7 +4798,7 @@ void addReplyClusterLinksDescription(client *c) { const char *getPreferredEndpoint(clusterNode *n) { switch(server.cluster_preferred_endpoint_type) { case CLUSTER_ENDPOINT_TYPE_IP: return n->ip; - case CLUSTER_ENDPOINT_TYPE_HOSTNAME: return n->hostname ? n->hostname : "?"; + case CLUSTER_ENDPOINT_TYPE_HOSTNAME: return (sdslen(n->hostname) != 0) ? n->hostname : "?"; case CLUSTER_ENDPOINT_TYPE_UNKNOWN_ENDPOINT: return ""; } return "unknown"; @@ -4898,7 +4892,7 @@ void addNodeToNodeReply(client *c, clusterNode *node) { if (server.cluster_preferred_endpoint_type == CLUSTER_ENDPOINT_TYPE_IP) { addReplyBulkCString(c, node->ip); } else if (server.cluster_preferred_endpoint_type == CLUSTER_ENDPOINT_TYPE_HOSTNAME) { - addReplyBulkCString(c, node->hostname ? node->hostname : "?"); + addReplyBulkCString(c, sdslen(node->hostname) != 0 ? node->hostname : "?"); } else if (server.cluster_preferred_endpoint_type == CLUSTER_ENDPOINT_TYPE_UNKNOWN_ENDPOINT) { addReplyNull(c); } else { @@ -4921,7 +4915,7 @@ void addNodeToNodeReply(client *c, clusterNode *node) { length++; } if (server.cluster_preferred_endpoint_type != CLUSTER_ENDPOINT_TYPE_HOSTNAME - && node->hostname) + && sdslen(node->hostname) != 0) { addReplyBulkCString(c, "hostname"); addReplyBulkCString(c, node->hostname); diff --git a/src/cluster.h b/src/cluster.h index 465654205..7e0d7c25a 100644 --- a/src/cluster.h +++ b/src/cluster.h @@ -134,8 +134,8 @@ typedef struct clusterNode { mstime_t repl_offset_time; /* Unix time we received offset for this node */ mstime_t orphaned_time; /* Starting time of orphaned master condition */ long long repl_offset; /* Last known repl offset for this node. */ - char ip[NET_IP_STR_LEN]; /* Latest known IP address of this node */ - char *hostname; /* The known hostname for this node */ + char ip[NET_IP_STR_LEN]; /* Latest known IP address of this node */ + sds hostname; /* The known hostname for this node */ int port; /* Latest known clients port (TLS or plain). */ int pport; /* Latest known clients plaintext port. Only used if the main clients port is for TLS. */ @@ -339,8 +339,6 @@ typedef struct { * changes in clusterMsg be caught at compile time. */ -/* Avoid static_assert on non-C11 compilers. */ -#if __STDC_VERSION__ >= 201112L static_assert(offsetof(clusterMsg, sig) == 0, "unexpected field offset"); static_assert(offsetof(clusterMsg, totlen) == 4, "unexpected field offset"); static_assert(offsetof(clusterMsg, ver) == 8, "unexpected field offset"); @@ -362,7 +360,6 @@ static_assert(offsetof(clusterMsg, flags) == 2250, "unexpected field offset"); static_assert(offsetof(clusterMsg, state) == 2252, "unexpected field offset"); static_assert(offsetof(clusterMsg, mflags) == 2253, "unexpected field offset"); static_assert(offsetof(clusterMsg, data) == 2256, "unexpected field offset"); -#endif #define CLUSTERMSG_MIN_LEN (sizeof(clusterMsg)-sizeof(union clusterMsgData)) diff --git a/src/server.h b/src/server.h index 853e9f5fa..ed514a32b 100644 --- a/src/server.h +++ b/src/server.h @@ -57,6 +57,10 @@ #include #endif +#ifndef static_assert +#define static_assert(expr, lit) extern char __static_assert_failure[(expr) ? 1:-1] +#endif + typedef long long mstime_t; /* millisecond time type. */ typedef long long ustime_t; /* microsecond time type. */