Updates needed for replacing hiredis in the valkey repo

.clang-format

@@ -13,6 +13,8 @@ IncludeCategories:
     Priority: -1
   - Regex: '^"'
     Priority: 1
+  - Regex: '^<(sds|dict).h>'
+    Priority: 2
   - Regex: '^<valkey/adapters/'
     Priority: 3
   - Regex: '^<valkey/'

CMakeLists.txt

@@ -38,22 +38,32 @@ else()
   add_definitions(-D_CRT_SECURE_NO_WARNINGS -DWIN32_LEAN_AND_MEAN)
 endif()
 
-SET(valkey_sources
+set(valkey_sources
     src/adlist.c
     src/alloc.c
     src/async.c
     src/cluster.c
     src/command.c
     src/conn.c
     src/crc16.c
-    src/dict.c
     src/net.c
     src/read.c
-    src/sds.c
     src/sockcompat.c
     src/valkey.c
     src/vkutil.c)
 
+# Allow the libvalkey provided sds and dict types to be replaced by
+# compatible implementations (like Valkey's).
+# A replaced type is not included in a built archive or shared library.
+if(NOT DICT_INCLUDE_DIR)
+  set(valkey_sources ${valkey_sources} src/dict.c)
+  set(DICT_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src)
+endif()
+if(NOT SDS_INCLUDE_DIR)
+  set(valkey_sources ${valkey_sources} src/sds.c)
+  set(SDS_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/src)
+endif()
+
 ADD_LIBRARY(valkey ${valkey_sources})
 ADD_LIBRARY(valkey::valkey ALIAS valkey)
 set(valkey_export_name valkey CACHE STRING "Name of the exported target")
@@ -78,6 +88,9 @@ TARGET_INCLUDE_DIRECTORIES(valkey
     PUBLIC
         $<INSTALL_INTERFACE:include>
         $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/valkey>
+    PRIVATE
+        $<BUILD_INTERFACE:${DICT_INCLUDE_DIR}>
+        $<BUILD_INTERFACE:${SDS_INCLUDE_DIR}>
 )
 
 CONFIGURE_FILE(valkey.pc.in valkey.pc @ONLY)
@@ -169,6 +182,8 @@ IF(ENABLE_TLS)
         PRIVATE
             $<INSTALL_INTERFACE:include>
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/valkey>
+            $<BUILD_INTERFACE:${DICT_INCLUDE_DIR}>
+            $<BUILD_INTERFACE:${SDS_INCLUDE_DIR}>
     )
 
     IF (APPLE AND BUILD_SHARED_LIBS)
@@ -238,6 +253,8 @@ if(ENABLE_RDMA)
         PRIVATE
             $<INSTALL_INTERFACE:include>
             $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include/valkey>
+            $<BUILD_INTERFACE:${DICT_INCLUDE_DIR}>
+            $<BUILD_INTERFACE:${SDS_INCLUDE_DIR}>
     )
 
     set_target_properties(valkey_rdma

Makefile

@@ -18,6 +18,18 @@ TEST_BINS = $(patsubst $(TEST_DIR)/%.c,$(TEST_DIR)/%,$(TEST_SRCS))
 SOURCES = $(filter-out $(SRC_DIR)/tls.c $(SRC_DIR)/rdma.c, $(wildcard $(SRC_DIR)/*.c))
 HEADERS = $(filter-out $(INCLUDE_DIR)/tls.h $(INCLUDE_DIR)/rdma.h, $(wildcard $(INCLUDE_DIR)/*.h))
 
+# Allow the libvalkey provided sds and dict types to be replaced by
+# compatible implementations (like Valkey's).
+# A replaced type is not included in a built archive or shared library.
+SDS_INCLUDE_DIR ?= $(SRC_DIR)
+DICT_INCLUDE_DIR ?= $(SRC_DIR)
+ifneq ($(SDS_INCLUDE_DIR),$(SRC_DIR))
+  SOURCES := $(filter-out $(SRC_DIR)/sds.c, $(SOURCES))
+endif
+ifneq ($(DICT_INCLUDE_DIR),$(SRC_DIR))
+  SOURCES := $(filter-out $(SRC_DIR)/dict.c, $(SOURCES))
+endif
+
 OBJS = $(patsubst $(SRC_DIR)/%.c,$(OBJ_DIR)/%.o,$(SOURCES))
 
 LIBNAME=libvalkey
@@ -216,10 +228,10 @@ $(RDMA_STLIBNAME): $(RDMA_OBJS)
 	$(STLIB_MAKE_CMD) $(RDMA_STLIBNAME) $(RDMA_OBJS)
 
 $(OBJ_DIR)/%.o: $(SRC_DIR)/%.c | $(OBJ_DIR)
-	$(CC) -std=c99 -pedantic $(REAL_CFLAGS) -I$(INCLUDE_DIR) -MMD -MP -c $< -o $@
+	$(CC) -std=c99 -pedantic $(REAL_CFLAGS) -I$(INCLUDE_DIR) -I$(SDS_INCLUDE_DIR) -I$(DICT_INCLUDE_DIR) -MMD -MP -c $< -o $@
 
 $(OBJ_DIR)/%.o: $(TEST_DIR)/%.c | $(OBJ_DIR)
-	$(CC) -std=c99 -pedantic $(REAL_CFLAGS) -I$(INCLUDE_DIR) -I$(SRC_DIR) -MMD -MP -c $< -o $@
+	$(CC) -std=c99 -pedantic $(REAL_CFLAGS) -I$(INCLUDE_DIR) -I$(SDS_INCLUDE_DIR) -I$(DICT_INCLUDE_DIR) -I$(SRC_DIR) -MMD -MP -c $< -o $@
 
 $(TEST_DIR)/%: $(OBJ_DIR)/%.o $(STLIBNAME)
 	$(CC) -o $@ $< $(RDMA_STLIB) $(STLIBNAME) $(TLS_STLIB) $(LDFLAGS) $(TEST_LDFLAGS)

include/valkey/cluster.h

@@ -138,14 +138,10 @@ typedef struct valkeyClusterAsyncContext {
 
 } valkeyClusterAsyncContext;
 
-#if UINTPTR_MAX == UINT64_MAX
-#define VALKEY_NODE_ITERATOR_SIZE 56
-#else
-#define VALKEY_NODE_ITERATOR_SIZE 32
-#endif
-typedef struct valkeyClusterNodeIterator {
-    char opaque_data[VALKEY_NODE_ITERATOR_SIZE];
-} valkeyClusterNodeIterator;
+/* --- Opaque types --- */
+
+/* 72 bytes needed when using Valkey's dict. */
+typedef uint64_t valkeyClusterNodeIterator[9];
 
 /* --- Configuration options --- */
 

src/async.c

@@ -42,11 +42,12 @@
 
 #include "async.h"
 #include "async_private.h"
-#include "dict.h"
 #include "net.h"
-#include "sds.h"
 #include "valkey_private.h"
 
+#include <dict.h>
+#include <sds.h>
+
 #include <assert.h>
 #include <ctype.h>
 #include <errno.h>
@@ -60,7 +61,7 @@
 int valkeyAppendCmdLen(valkeyContext *c, const char *cmd, size_t len);
 
 /* Functions managing dictionary of callbacks for pub/sub. */
-static unsigned long int callbackHash(const void *key) {
+static uint64_t callbackHash(const void *key) {
     return dictGenHashFunction((const unsigned char *)key,
                                sdslen((const sds)key));
 }

src/cluster.c

@@ -38,10 +38,11 @@
 #include "adlist.h"
 #include "alloc.h"
 #include "command.h"
-#include "dict.h"
-#include "sds.h"
 #include "vkutil.h"
 
+#include <dict.h>
+#include <sds.h>
+
 #include <assert.h>
 #include <ctype.h>
 #include <errno.h>
@@ -120,7 +121,7 @@ void listClusterNodeDestructor(void *val) { freeValkeyClusterNode(val); }
 
 void listClusterSlotDestructor(void *val) { cluster_slot_destroy(val); }
 
-static unsigned long int dictSdsHash(const void *key) {
+static uint64_t dictSdsHash(const void *key) {
     return dictGenHashFunction((unsigned char *)key, sdslen((char *)key));
 }
 
@@ -3384,8 +3385,8 @@ struct nodeIterator {
     int retries_left;
     dictIterator di;
 };
-/* Make sure VALKEY_NODE_ITERATOR_SIZE is correct. */
-vk_static_assert(sizeof(struct nodeIterator) == VALKEY_NODE_ITERATOR_SIZE);
+/* Make sure the opaque memory blob can contain a nodeIterator. */
+vk_static_assert(sizeof(valkeyClusterNodeIterator) >= sizeof(struct nodeIterator));
 
 /* Initiate an iterator for iterating over current cluster nodes */
 void valkeyClusterInitNodeIterator(valkeyClusterNodeIterator *iter,

src/command.c

@@ -45,9 +45,10 @@
 
 #include "alloc.h"
 #include "command.h"
-#include "sds.h"
 #include "vkutil.h"
 
+#include <sds.h>
+
 #include <stdio.h>
 #include <string.h>
 

src/dict.c

@@ -61,8 +61,8 @@ static int _dictInit(dict *ht, dictType *type);
 
 /* Generic hash function (a popular one from Bernstein).
  * I tested a few and this was the best. */
-unsigned long int dictGenHashFunction(const unsigned char *buf, int len) {
-    unsigned long int hash = 5381;
+uint64_t dictGenHashFunction(const unsigned char *buf, int len) {
+    uint64_t hash = 5381;
 
     while (len--)
         hash = ((hash << 5) + hash) + (*buf++); /* hash * 33 + c */

src/dict.h

@@ -36,13 +36,15 @@
 #ifndef __DICT_H
 #define __DICT_H
 
+#include <stdint.h>
+
 #define DICT_OK 0
 #define DICT_ERR 1
 
 typedef struct dictEntry dictEntry; /* opaque */
 
 typedef struct dictType {
-    unsigned long int (*hashFunction)(const void *key);
+    uint64_t (*hashFunction)(const void *key);
     void *(*keyDup)(const void *key);
     void *(*valDup)(const void *obj);
     int (*keyCompare)(const void *key1, const void *key2);
@@ -87,7 +89,7 @@ typedef struct dictIterator {
 #define dictSize(ht) ((ht)->used)
 
 /* API */
-unsigned long int dictGenHashFunction(const unsigned char *buf, int len);
+uint64_t dictGenHashFunction(const unsigned char *buf, int len);
 dict *dictCreate(dictType *type);
 int dictExpand(dict *ht, unsigned long size);
 int dictAdd(dict *ht, void *key, void *val);

src/net.c

@@ -38,10 +38,11 @@
 #include "net.h"
 
 #include "async.h"
-#include "sds.h"
 #include "sockcompat.h"
 #include "valkey_private.h"
 
+#include <sds.h>
+
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>

src/read.c

@@ -41,7 +41,8 @@
 
 #include "alloc.h"
 #include "read.h"
-#include "sds.h"
+
+#include <sds.h>
 
 #include <assert.h>
 #include <ctype.h>
@@ -799,8 +800,10 @@ int valkeyReaderGetReply(valkeyReader *r, void **reply) {
     /* Discard part of the buffer when we've consumed at least 1k, to avoid
      * doing unnecessary calls to memmove() in sds.c. */
     if (r->pos >= 1024) {
-        if (sdsrange(r->buf, r->pos, -1) < 0)
+        /* No length check in Valkeys sdsrange() */
+        if (sdslen(r->buf) > SSIZE_MAX)
             return VALKEY_ERR;
+        sdsrange(r->buf, r->pos, -1);
         r->pos = 0;
         r->len = sdslen(r->buf);
     }

src/valkey.c

@@ -37,9 +37,10 @@
 #include "valkey.h"
 
 #include "net.h"
-#include "sds.h"
 #include "valkey_private.h"
 
+#include <sds.h>
+
 #include <assert.h>
 #include <ctype.h>
 #include <errno.h>
@@ -1033,8 +1034,10 @@ int valkeyBufferWrite(valkeyContext *c, int *done) {
                 if (c->obuf == NULL)
                     goto oom;
             } else {
-                if (sdsrange(c->obuf, nwritten, -1) < 0)
+                /* No length check in Valkeys sdsrange() */
+                if (sdslen(c->obuf) > SSIZE_MAX)
                     goto oom;
+                sdsrange(c->obuf, nwritten, -1);
             }
         }
     }

src/valkey_private.h

@@ -33,9 +33,10 @@
 
 #include "win32.h"
 
-#include "sds.h"
 #include "valkey.h"
 
+#include <sds.h>
+
 #include <limits.h>
 #include <string.h>
 

tests/client_test.c

@@ -16,10 +16,11 @@
 #endif
 #include "adapters/poll.h"
 #include "async.h"
-#include "sds.h"
 #include "valkey.h"
 #include "valkey_private.h"
 
+#include <sds.h>
+
 #include <assert.h>
 #include <errno.h>
 #include <limits.h>