Add helper function for padded pointer copy (valkey-io#2388)

## Closes valkey-io#2383

Refactor the logic for writing a fixed length (8 byte) pointer
field with zero padding on 32‑bit targets by:
* Introducing a new helper function writePointerWithPadding
* Refactoring encodeFailureReportKey() and encodeTimeoutKey() to use
this helper instead of in‑lining the padding logic

Signed-off-by: Seungmin Lee <sungming@amazon.com>
Co-authored-by: Seungmin Lee <sungming@amazon.com>

Author: sungming2

src/cluster_legacy.c

@@ -1692,8 +1692,6 @@ clusterNode *createClusterNode(char *nodename, int flags) {
  * timestamps and keep the radix‐tree keys compact.  The resulting timestamp is stored
  * in big‑endian format, followed by the pointer to the clusterNode. */
 static void encodeFailureReportKey(clusterNode *node, mstime_t report_time, unsigned char *buf_out) {
-    const size_t node_ptr_pad_bytes = (sizeof(clusterNode *) == 4) ? 4 : 0; // pad on 32-bit
-
     /* Round up to the next second for fewer key splits and quorum grace */
     mstime_t bucketed_time = (report_time / SEC_IN_MS) * SEC_IN_MS + SEC_IN_MS;
 
@@ -1702,8 +1700,7 @@ static void encodeFailureReportKey(clusterNode *node, mstime_t report_time, unsi
     memcpy(buf_out, &big_endian_time, sizeof(uint64_t));
 
     /* Append the node pointer, plus padding if necessary */
-    memcpy(buf_out + sizeof(uint64_t), &node, sizeof(node));
-    if (node_ptr_pad_bytes) memset(buf_out + sizeof(uint64_t) + sizeof(node), 0, node_ptr_pad_bytes);
+    writePointerWithPadding(buf_out + sizeof(uint64_t), node);
 }
 
 /* Reverses the operation of encodeFailureReportKey, reading back a timestamp

src/timeout.c

@@ -89,19 +89,18 @@ int clientsCronHandleTimeout(client *c, mstime_t now_ms) {
 #define CLIENT_ST_KEYLEN 16 /* 8 bytes mstime + 8 bytes client ID. */
 
 /* Given client ID and timeout, write the resulting radix tree key in buf. */
-void encodeTimeoutKey(unsigned char *buf, uint64_t timeout, client *c) {
+void encodeTimeoutKey(client *c, uint64_t timeout, unsigned char *buf_out) {
     timeout = htonu64(timeout);
-    memcpy(buf, &timeout, sizeof(timeout));
-    memcpy(buf + 8, &c, sizeof(c));
-    if (sizeof(c) == 4) memset(buf + 12, 0, 4); /* Zero padding for 32bit target. */
+    memcpy(buf_out, &timeout, sizeof(timeout));
+    writePointerWithPadding(buf_out + sizeof(timeout), c);
 }
 
 /* Given a key encoded with encodeTimeoutKey(), resolve the fields and write
  * the timeout into *toptr and the client pointer into *cptr. */
-void decodeTimeoutKey(unsigned char *buf, uint64_t *toptr, client **cptr) {
-    memcpy(toptr, buf, sizeof(*toptr));
-    *toptr = ntohu64(*toptr);
-    memcpy(cptr, buf + 8, sizeof(*cptr));
+void decodeTimeoutKey(unsigned char *buf, uint64_t *timeout_ptr, client **client_ptr) {
+    memcpy(timeout_ptr, buf, sizeof(*timeout_ptr));
+    *timeout_ptr = ntohu64(*timeout_ptr);
+    memcpy(client_ptr, buf + sizeof(*timeout_ptr), sizeof(*client_ptr));
 }
 
 /* Add the specified client id / timeout as a key in the radix tree we use
@@ -111,7 +110,7 @@ void addClientToTimeoutTable(client *c) {
     if (c->bstate->timeout == 0) return;
     uint64_t timeout = c->bstate->timeout;
     unsigned char buf[CLIENT_ST_KEYLEN];
-    encodeTimeoutKey(buf, timeout, c);
+    encodeTimeoutKey(c, timeout, buf);
     if (raxTryInsert(server.clients_timeout_table, buf, sizeof(buf), NULL, NULL)) c->flag.in_to_table = 1;
 }
 
@@ -122,7 +121,7 @@ void removeClientFromTimeoutTable(client *c) {
     c->flag.in_to_table = 0;
     uint64_t timeout = c->bstate->timeout;
     unsigned char buf[CLIENT_ST_KEYLEN];
-    encodeTimeoutKey(buf, timeout, c);
+    encodeTimeoutKey(c, timeout, buf);
     raxRemove(server.clients_timeout_table, buf, sizeof(buf), NULL);
 }
 

src/unit/test_files.h

@@ -193,6 +193,7 @@ int test_ld2string(int argc, char **argv, int flags);
 int test_fixedpoint_d2string(int argc, char **argv, int flags);
 int test_version2num(int argc, char **argv, int flags);
 int test_reclaimFilePageCache(int argc, char **argv, int flags);
+int test_writePointerWithPadding(int argc, char **argv, int flags);
 int test_valkey_strtod(int argc, char **argv, int flags);
 int test_vector(int argc, char **argv, int flags);
 int test_ziplistCreateIntList(int argc, char **argv, int flags);
@@ -251,7 +252,7 @@ unitTest __test_quicklist_c[] = {{"test_quicklistCreateList", test_quicklistCrea
 unitTest __test_rax_c[] = {{"test_raxRandomWalk", test_raxRandomWalk}, {"test_raxIteratorUnitTests", test_raxIteratorUnitTests}, {"test_raxTryInsertUnitTests", test_raxTryInsertUnitTests}, {"test_raxRegressionTest1", test_raxRegressionTest1}, {"test_raxRegressionTest2", test_raxRegressionTest2}, {"test_raxRegressionTest3", test_raxRegressionTest3}, {"test_raxRegressionTest4", test_raxRegressionTest4}, {"test_raxRegressionTest5", test_raxRegressionTest5}, {"test_raxRegressionTest6", test_raxRegressionTest6}, {"test_raxBenchmark", test_raxBenchmark}, {"test_raxHugeKey", test_raxHugeKey}, {"test_raxFuzz", test_raxFuzz}, {"test_raxRecompressHugeKey", test_raxRecompressHugeKey}, {NULL, NULL}};
 unitTest __test_sds_c[] = {{"test_sds", test_sds}, {"test_typesAndAllocSize", test_typesAndAllocSize}, {"test_sdsHeaderSizes", test_sdsHeaderSizes}, {"test_sdssplitargs", test_sdssplitargs}, {NULL, NULL}};
 unitTest __test_sha1_c[] = {{"test_sha1", test_sha1}, {NULL, NULL}};
-unitTest __test_util_c[] = {{"test_string2ll", test_string2ll}, {"test_string2l", test_string2l}, {"test_ll2string", test_ll2string}, {"test_ld2string", test_ld2string}, {"test_fixedpoint_d2string", test_fixedpoint_d2string}, {"test_version2num", test_version2num}, {"test_reclaimFilePageCache", test_reclaimFilePageCache}, {NULL, NULL}};
+unitTest __test_util_c[] = {{"test_string2ll", test_string2ll}, {"test_string2l", test_string2l}, {"test_ll2string", test_ll2string}, {"test_ld2string", test_ld2string}, {"test_fixedpoint_d2string", test_fixedpoint_d2string}, {"test_version2num", test_version2num}, {"test_reclaimFilePageCache", test_reclaimFilePageCache}, {"test_writePointerWithPadding", test_writePointerWithPadding}, {NULL, NULL}};
 unitTest __test_valkey_strtod_c[] = {{"test_valkey_strtod", test_valkey_strtod}, {NULL, NULL}};
 unitTest __test_vector_c[] = {{"test_vector", test_vector}, {NULL, NULL}};
 unitTest __test_ziplist_c[] = {{"test_ziplistCreateIntList", test_ziplistCreateIntList}, {"test_ziplistPop", test_ziplistPop}, {"test_ziplistGetElementAtIndex3", test_ziplistGetElementAtIndex3}, {"test_ziplistGetElementOutOfRange", test_ziplistGetElementOutOfRange}, {"test_ziplistGetLastElement", test_ziplistGetLastElement}, {"test_ziplistGetFirstElement", test_ziplistGetFirstElement}, {"test_ziplistGetElementOutOfRangeReverse", test_ziplistGetElementOutOfRangeReverse}, {"test_ziplistIterateThroughFullList", test_ziplistIterateThroughFullList}, {"test_ziplistIterateThroughListFrom1ToEnd", test_ziplistIterateThroughListFrom1ToEnd}, {"test_ziplistIterateThroughListFrom2ToEnd", test_ziplistIterateThroughListFrom2ToEnd}, {"test_ziplistIterateThroughStartOutOfRange", test_ziplistIterateThroughStartOutOfRange}, {"test_ziplistIterateBackToFront", test_ziplistIterateBackToFront}, {"test_ziplistIterateBackToFrontDeletingAllItems", test_ziplistIterateBackToFrontDeletingAllItems}, {"test_ziplistDeleteInclusiveRange0To0", test_ziplistDeleteInclusiveRange0To0}, {"test_ziplistDeleteInclusiveRange0To1", test_ziplistDeleteInclusiveRange0To1}, {"test_ziplistDeleteInclusiveRange1To2", test_ziplistDeleteInclusiveRange1To2}, {"test_ziplistDeleteWithStartIndexOutOfRange", test_ziplistDeleteWithStartIndexOutOfRange}, {"test_ziplistDeleteWithNumOverflow", test_ziplistDeleteWithNumOverflow}, {"test_ziplistDeleteFooWhileIterating", test_ziplistDeleteFooWhileIterating}, {"test_ziplistReplaceWithSameSize", test_ziplistReplaceWithSameSize}, {"test_ziplistReplaceWithDifferentSize", test_ziplistReplaceWithDifferentSize}, {"test_ziplistRegressionTestForOver255ByteStrings", test_ziplistRegressionTestForOver255ByteStrings}, {"test_ziplistRegressionTestDeleteNextToLastEntries", test_ziplistRegressionTestDeleteNextToLastEntries}, {"test_ziplistCreateLongListAndCheckIndices", test_ziplistCreateLongListAndCheckIndices}, {"test_ziplistCompareStringWithZiplistEntries", test_ziplistCompareStringWithZiplistEntries}, {"test_ziplistMergeTest", test_ziplistMergeTest}, {"test_ziplistStressWithRandomPayloadsOfDifferentEncoding", test_ziplistStressWithRandomPayloadsOfDifferentEncoding}, {"test_ziplistCascadeUpdateEdgeCases", test_ziplistCascadeUpdateEdgeCases}, {"test_ziplistInsertEdgeCase", test_ziplistInsertEdgeCase}, {"test_ziplistStressWithVariableSize", test_ziplistStressWithVariableSize}, {"test_BenchmarkziplistFind", test_BenchmarkziplistFind}, {"test_BenchmarkziplistIndex", test_BenchmarkziplistIndex}, {"test_BenchmarkziplistValidateIntegrity", test_BenchmarkziplistValidateIntegrity}, {"test_BenchmarkziplistCompareWithString", test_BenchmarkziplistCompareWithString}, {"test_BenchmarkziplistCompareWithNumber", test_BenchmarkziplistCompareWithNumber}, {"test_ziplistStress__ziplistCascadeUpdate", test_ziplistStress__ziplistCascadeUpdate}, {NULL, NULL}};

src/unit/test_util.c

@@ -330,3 +330,30 @@ int test_reclaimFilePageCache(int argc, char **argv, int flags) {
 #endif
     return 0;
 }
+
+int test_writePointerWithPadding(int argc, char **argv, int flags) {
+    UNUSED(argc);
+    UNUSED(argv);
+    UNUSED(flags);
+
+    unsigned char buf[8];
+    static int dummy;
+    void *ptr = &dummy;
+    size_t ptr_size = sizeof(ptr);
+
+    /* Write the pointer and pad to 8 bytes */
+    writePointerWithPadding(buf, ptr);
+
+    /* The first ptr_size bytes must match the raw pointer bytes */
+    unsigned char expected[sizeof(ptr)];
+    memcpy(expected, &ptr, ptr_size);
+    TEST_ASSERT(memcmp(buf, expected, ptr_size) == 0);
+
+
+    /* The remaining bytes (if any) must be zero */
+    for (size_t i = ptr_size; i < sizeof(buf); i++) {
+        TEST_ASSERT(buf[i] == 0);
+    }
+
+    return 0;
+}

src/util.c

@@ -1570,3 +1570,12 @@ int snprintf_async_signal_safe(char *to, size_t n, const char *fmt, ...) {
     va_end(args);
     return result;
 }
+
+/* Writes a pointer into an 8 bytes field, padding with zeros on 32bit targets
+ * to ensure a consistent fixed width encoding. */
+void writePointerWithPadding(unsigned char *buf, const void *ptr) {
+    size_t ptr_size = sizeof(ptr); /* 4 on 32‑bit, 8 on 64‑bit */
+    memcpy(buf, &ptr, ptr_size);
+    /* if it is 32-bit system, pad the remaining 4 bytes with zero */
+    if (ptr_size == 4) memset(buf + ptr_size, 0, ptr_size);
+}

src/util.h

@@ -103,5 +103,6 @@ void getRandomSeedCString(char *buff, size_t len);
 void setRandomSeedCString(char *seed_str, size_t len);
 void getRandomHexChars(char *p, size_t len);
 void getRandomBytes(unsigned char *p, size_t len);
+void writePointerWithPadding(unsigned char *buf, const void *ptr);
 
 #endif