ronantakizawa/codereview-bench huggingface dataset:
Repository: valkey-io/valkey
File: src/server.c (C)

Code before:
    UNUSED(d);
    zfree(val);
}

void dictListDestructor(dict *d, void *val) {
    UNUSED(d);
    listRelease((list *)val);
}

void dictDictDestructor(dict *d, void *val) {
    UNUSED(d);
    dictRelease((dict *)val);
}

int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) {
    int l1, l2;
    UNUSED(d);

    l1 = sdslen((sds)key1);
    l2 = sdslen((sds)key2);
    if (l1 != l2) return 0;
    return memcmp(key1, key2, l1) == 0;
}

size_t dictSdsKeyLen(const void *key) { 
    return sdsAllocSize((sds)key);
}

size_t dictSdsKeyToBytes(unsigned char *buf, const void *key, unsigned char *header_size) {
    sds keysds = (sds)key;
    size_t n_bytes = sdsAllocSize(keysds);
    memcpy(buf, sdsAllocPtr(keysds), n_bytes);
    *header_size = sdsHdrSize(keysds[-1]);
    return n_bytes;
}

/* A case insensitive version used for the command lookup table and other
 * places where case insensitive non binary-safe comparison is needed. */
int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2) {
    UNUSED(d);
    return strcasecmp(key1, key2) == 0;
}

void dictObjectDestructor(dict *d, void *val) {
    UNUSED(d);
    if (val == NULL) return; /* Lazy freeing will set value to NULL. */
    decrRefCount(val);
}

void dictSdsDestructor(dict *d, void *val) {
    UNUSED(d);

Review comment (suggestion):
`sdsAllocSize` can return more than we need. `sdslen((sds)key)) + sdsHrdSize((sds)key) + 1` gives the tightest range.

Code after:
    zfree(val);
}

void dictListDestructor(dict *d, void *val) {
    UNUSED(d);
    listRelease((list *)val);
}

void dictDictDestructor(dict *d, void *val) {
    UNUSED(d);
    dictRelease((dict *)val);
}

int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) {
    int l1, l2;
    UNUSED(d);

    l1 = sdslen((sds)key1);
    l2 = sdslen((sds)key2);
    if (l1 != l2) return 0;
    return memcmp(key1, key2, l1) == 0;
}

size_t dictSdsEmbedKey(unsigned char *buf, size_t buf_len, const void *key, uint8_t *key_offset) {
    return sdscopytobuffer(buf, buf_len, (sds)key, key_offset);
}

/* A case insensitive version used for the command lookup table and other
 * places where case insensitive non binary-safe comparison is needed. */
int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2) {
    UNUSED(d);
    return strcasecmp(key1, key2) == 0;
}

void dictObjectDestructor(dict *d, void *val) {
    UNUSED(d);
    if (val == NULL) return; /* Lazy freeing will set value to NULL. */
    decrRefCount(val);
}

void dictSdsDestructor(dict *d, void *val) {
    UNUSED(d);
    sdsfree(val);
}

void *dictSdsDup(dict *d, const void *key) {
    UNUSED(d);
    return sdsdup((const sds)key);
}

int dictObjKeyCompare(dict *d, const void *key1, const void *key2) {
Repository: valkey-io/valkey
File: src/server.c (C)

Code before:

void dictListDestructor(dict *d, void *val) {
    UNUSED(d);
    listRelease((list *)val);
}

void dictDictDestructor(dict *d, void *val) {
    UNUSED(d);
    dictRelease((dict *)val);
}

int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) {
    int l1, l2;
    UNUSED(d);

    l1 = sdslen((sds)key1);
    l2 = sdslen((sds)key2);
    if (l1 != l2) return 0;
    return memcmp(key1, key2, l1) == 0;
}

size_t dictSdsKeyLen(const void *key) { 
    return sdsAllocSize((sds)key);
}

size_t dictSdsKeyToBytes(unsigned char *buf, const void *key, unsigned char *header_size) {
    sds keysds = (sds)key;
    size_t n_bytes = sdsAllocSize(keysds);
    memcpy(buf, sdsAllocPtr(keysds), n_bytes);
    *header_size = sdsHdrSize(keysds[-1]);
    return n_bytes;
}

/* A case insensitive version used for the command lookup table and other
 * places where case insensitive non binary-safe comparison is needed. */
int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2) {
    UNUSED(d);
    return strcasecmp(key1, key2) == 0;
}

void dictObjectDestructor(dict *d, void *val) {
    UNUSED(d);
    if (val == NULL) return; /* Lazy freeing will set value to NULL. */
    decrRefCount(val);
}

void dictSdsDestructor(dict *d, void *val) {
    UNUSED(d);
    sdsfree(val);
}


Review comment (suggestion):
[Readability suggestion] This function essentially replicate an sds string to a caller provided buffer. The only difference from `sdsdup` is that the caller allocates the memory. I think this logic belongs to sds.c.

Code after:
void dictListDestructor(dict *d, void *val) {
    UNUSED(d);
    listRelease((list *)val);
}

void dictDictDestructor(dict *d, void *val) {
    UNUSED(d);
    dictRelease((dict *)val);
}

int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) {
    int l1, l2;
    UNUSED(d);

    l1 = sdslen((sds)key1);
    l2 = sdslen((sds)key2);
    if (l1 != l2) return 0;
    return memcmp(key1, key2, l1) == 0;
}

size_t dictSdsEmbedKey(unsigned char *buf, size_t buf_len, const void *key, uint8_t *key_offset) {
    return sdscopytobuffer(buf, buf_len, (sds)key, key_offset);
}

/* A case insensitive version used for the command lookup table and other
 * places where case insensitive non binary-safe comparison is needed. */
int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2) {
    UNUSED(d);
    return strcasecmp(key1, key2) == 0;
}

void dictObjectDestructor(dict *d, void *val) {
    UNUSED(d);
    if (val == NULL) return; /* Lazy freeing will set value to NULL. */
    decrRefCount(val);
}

void dictSdsDestructor(dict *d, void *val) {
    UNUSED(d);
    sdsfree(val);
}

void *dictSdsDup(dict *d, const void *key) {
    UNUSED(d);
    return sdsdup((const sds)key);
}

int dictObjKeyCompare(dict *d, const void *key1, const void *key2) {
    const robj *o1 = key1, *o2 = key2;
    return dictSdsKeyCompare(d, o1->ptr, o2->ptr);
}
Repository: valkey-io/valkey
File: src/server.c (C)

Code before:
    UNUSED(d);
    listRelease((list *)val);
}

void dictDictDestructor(dict *d, void *val) {
    UNUSED(d);
    dictRelease((dict *)val);
}

int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) {
    int l1, l2;
    UNUSED(d);

    l1 = sdslen((sds)key1);
    l2 = sdslen((sds)key2);
    if (l1 != l2) return 0;
    return memcmp(key1, key2, l1) == 0;
}

size_t dictSdsKeyLen(const void *key) { 
    return sdsAllocSize((sds)key);
}

size_t dictSdsKeyToBytes(unsigned char *buf, const void *key, unsigned char *header_size) {
    sds keysds = (sds)key;
    size_t n_bytes = sdsAllocSize(keysds);
    memcpy(buf, sdsAllocPtr(keysds), n_bytes);
    *header_size = sdsHdrSize(keysds[-1]);
    return n_bytes;
}

/* A case insensitive version used for the command lookup table and other
 * places where case insensitive non binary-safe comparison is needed. */
int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2) {
    UNUSED(d);
    return strcasecmp(key1, key2) == 0;
}

void dictObjectDestructor(dict *d, void *val) {
    UNUSED(d);
    if (val == NULL) return; /* Lazy freeing will set value to NULL. */
    decrRefCount(val);
}

void dictSdsDestructor(dict *d, void *val) {
    UNUSED(d);
    sdsfree(val);
}

void *dictSdsDup(dict *d, const void *key) {
    UNUSED(d);

Review comment (bug):
[Readability suggestion] This size computation logic needs to match `dictSdsKeyToBytes` to avoid buffer overrun bugs.

```suggestion
    size_t n_bytes = dictSdsKeyLen(keysds);
```

 Or better yet, let the caller provide the buffer length and we can assert on it being larger than `dictSdsKeyLen(key)`
 
 ```c
 size_t dictSdsKeyCopy(unsigned char *buf, size_t buf_len, const void *key, unsigned char *key_offset)
 ```

Code after:
    listRelease((list *)val);
}

void dictDictDestructor(dict *d, void *val) {
    UNUSED(d);
    dictRelease((dict *)val);
}

int dictSdsKeyCompare(dict *d, const void *key1, const void *key2) {
    int l1, l2;
    UNUSED(d);

    l1 = sdslen((sds)key1);
    l2 = sdslen((sds)key2);
    if (l1 != l2) return 0;
    return memcmp(key1, key2, l1) == 0;
}

size_t dictSdsEmbedKey(unsigned char *buf, size_t buf_len, const void *key, uint8_t *key_offset) {
    return sdscopytobuffer(buf, buf_len, (sds)key, key_offset);
}

/* A case insensitive version used for the command lookup table and other
 * places where case insensitive non binary-safe comparison is needed. */
int dictSdsKeyCaseCompare(dict *d, const void *key1, const void *key2) {
    UNUSED(d);
    return strcasecmp(key1, key2) == 0;
}

void dictObjectDestructor(dict *d, void *val) {
    UNUSED(d);
    if (val == NULL) return; /* Lazy freeing will set value to NULL. */
    decrRefCount(val);
}

void dictSdsDestructor(dict *d, void *val) {
    UNUSED(d);
    sdsfree(val);
}

void *dictSdsDup(dict *d, const void *key) {
    UNUSED(d);
    return sdsdup((const sds)key);
}

int dictObjKeyCompare(dict *d, const void *key1, const void *key2) {
    const robj *o1 = key1, *o2 = key2;
    return dictSdsKeyCompare(d, o1->ptr, o2->ptr);
}

uint64_t dictObjHash(const void *key) {
Repository: valkey-io/valkey
File: src/zmalloc.c (C)

Code before:
}

/* Like zfree(), but doesn't call zmalloc_size(). */
void zfree_with_size(void *ptr, size_t size) {
    if (ptr == NULL) return;

#ifndef HAVE_MALLOC_SIZE
    ptr = (char *)ptr - PREFIX_SIZE;
    size += PREFIX_SIZE;
#endif

    zfree_internal(ptr, size);
}

char *zstrdup(const char *s) {
    size_t l = strlen(s) + 1;
    char *p = zmalloc(l);

    memcpy(p, s, l);
    return p;
}

size_t zmalloc_used_memory(void) {
    size_t um = 0;
    for (int i = 0; i < total_active_threads; i++) {
        um += used_memory_thread[i];
    }
    if (unlikely(total_active_threads) > MAX_THREADS_NUM) {
        um += atomic_load_explicit(&used_memory_for_additional_threads, memory_order_relaxed);
    }
    return um;
}

void zmalloc_set_oom_handler(void (*oom_handler)(size_t)) {
    zmalloc_oom_handler = oom_handler;
}

/* Use 'MADV_DONTNEED' to release memory to operating system quickly.
 * We do that in a fork child process to avoid CoW when the parent modifies
 * these shared pages. */
void zmadvise_dontneed(void *ptr) {
#if defined(USE_JEMALLOC) && defined(__linux__)
    static size_t page_size = 0;
    if (page_size == 0) page_size = sysconf(_SC_PAGESIZE);
    size_t page_size_mask = page_size - 1;

    size_t real_size = zmalloc_size(ptr);
    if (real_size < page_size) return;

    /* We need to align the pointer upwards according to page size, because
     * the memory address is increased upwards and we only can free memory

Review comment (suggestion):
Now total_active_threads can be greater than MAX_THREADS_NUM so...

```suggestion
    for (int i = 0; i < min(total_active_threads, MAX_THREADS_NUM); i++) {
        um += used_memory_thread[i];
```

Code after:
    zfree_internal(ptr, size);
}

/* Like zfree(), but doesn't call zmalloc_size(). */
void zfree_with_size(void *ptr, size_t size) {
    if (ptr == NULL) return;

#ifndef HAVE_MALLOC_SIZE
    ptr = (char *)ptr - PREFIX_SIZE;
    size += PREFIX_SIZE;
#endif

    zfree_internal(ptr, size);
}

char *zstrdup(const char *s) {
    size_t l = strlen(s) + 1;
    char *p = zmalloc(l);

    memcpy(p, s, l);
    return p;
}

size_t zmalloc_used_memory(void) {
    size_t um = 0;
    int threads_num = total_active_threads;
    if (unlikely(total_active_threads > MAX_THREADS_NUM)) {
        um += atomic_load_explicit(&used_memory_for_additional_threads, memory_order_relaxed);
        threads_num = MAX_THREADS_NUM;
    }
    for (int i = 0; i < threads_num; i++) {
        um += used_memory_thread[i];
    }
    return um;
}

void zmalloc_set_oom_handler(void (*oom_handler)(size_t)) {
    zmalloc_oom_handler = oom_handler;
}

/* Use 'MADV_DONTNEED' to release memory to operating system quickly.
 * We do that in a fork child process to avoid CoW when the parent modifies
 * these shared pages. */
void zmadvise_dontneed(void *ptr) {
#if defined(USE_JEMALLOC) && defined(__linux__)
    static size_t page_size = 0;
    if (page_size == 0) page_size = sysconf(_SC_PAGESIZE);
    size_t page_size_mask = page_size - 1;

    size_t real_size = zmalloc_size(ptr);
    if (real_size < page_size) return;
Repository: valkey-io/valkey
File: src/zmalloc.c (C)

Code before:
/* Like zfree(), but doesn't call zmalloc_size(). */
void zfree_with_size(void *ptr, size_t size) {
    if (ptr == NULL) return;

#ifndef HAVE_MALLOC_SIZE
    ptr = (char *)ptr - PREFIX_SIZE;
    size += PREFIX_SIZE;
#endif

    zfree_internal(ptr, size);
}

char *zstrdup(const char *s) {
    size_t l = strlen(s) + 1;
    char *p = zmalloc(l);

    memcpy(p, s, l);
    return p;
}

size_t zmalloc_used_memory(void) {
    size_t um = 0;
    for (int i = 0; i < total_active_threads; i++) {
        um += used_memory_thread[i];
    }
    if (unlikely(total_active_threads) > MAX_THREADS_NUM) {
        um += atomic_load_explicit(&used_memory_for_additional_threads, memory_order_relaxed);
    }
    return um;
}

void zmalloc_set_oom_handler(void (*oom_handler)(size_t)) {
    zmalloc_oom_handler = oom_handler;
}

/* Use 'MADV_DONTNEED' to release memory to operating system quickly.
 * We do that in a fork child process to avoid CoW when the parent modifies
 * these shared pages. */
void zmadvise_dontneed(void *ptr) {
#if defined(USE_JEMALLOC) && defined(__linux__)
    static size_t page_size = 0;
    if (page_size == 0) page_size = sysconf(_SC_PAGESIZE);
    size_t page_size_mask = page_size - 1;

    size_t real_size = zmalloc_size(ptr);
    if (real_size < page_size) return;

    /* We need to align the pointer upwards according to page size, because
     * the memory address is increased upwards and we only can free memory
     * based on page. */
    char *aligned_ptr = (char *)(((size_t)ptr + page_size_mask) & ~page_size_mask);

Review comment (suggestion):
```suggestion
    if (unlikely(total_active_threads > MAX_THREADS_NUM)) {
```

Code after:

/* Like zfree(), but doesn't call zmalloc_size(). */
void zfree_with_size(void *ptr, size_t size) {
    if (ptr == NULL) return;

#ifndef HAVE_MALLOC_SIZE
    ptr = (char *)ptr - PREFIX_SIZE;
    size += PREFIX_SIZE;
#endif

    zfree_internal(ptr, size);
}

char *zstrdup(const char *s) {
    size_t l = strlen(s) + 1;
    char *p = zmalloc(l);

    memcpy(p, s, l);
    return p;
}

size_t zmalloc_used_memory(void) {
    size_t um = 0;
    int threads_num = total_active_threads;
    if (unlikely(total_active_threads > MAX_THREADS_NUM)) {
        um += atomic_load_explicit(&used_memory_for_additional_threads, memory_order_relaxed);
        threads_num = MAX_THREADS_NUM;
    }
    for (int i = 0; i < threads_num; i++) {
        um += used_memory_thread[i];
    }
    return um;
}

void zmalloc_set_oom_handler(void (*oom_handler)(size_t)) {
    zmalloc_oom_handler = oom_handler;
}

/* Use 'MADV_DONTNEED' to release memory to operating system quickly.
 * We do that in a fork child process to avoid CoW when the parent modifies
 * these shared pages. */
void zmadvise_dontneed(void *ptr) {
#if defined(USE_JEMALLOC) && defined(__linux__)
    static size_t page_size = 0;
    if (page_size == 0) page_size = sysconf(_SC_PAGESIZE);
    size_t page_size_mask = page_size - 1;

    size_t real_size = zmalloc_size(ptr);
    if (real_size < page_size) return;

    /* We need to align the pointer upwards according to page size, because
Repository: valkey-io/valkey
File: src/config.h (C/C++)

Code before:
#include <kernel/OS.h>
#define valkey_set_thread_title(name) rename_thread(find_thread(0), name)
#else
#if (defined __APPLE__ && defined(MAC_OS_X_VERSION_MAX_ALLOWED) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070)
int pthread_setname_np(const char *name);
#include <pthread.h>
#define valkey_set_thread_title(name) pthread_setname_np(name)
#else
#define valkey_set_thread_title(name)
#endif
#endif
#endif

/* Check if we can use setcpuaffinity(). */
#if (defined __linux || defined __NetBSD__ || defined __FreeBSD__ || defined __DragonFly__)
#define USE_SETCPUAFFINITY
void setcpuaffinity(const char *cpulist);
#endif

/* Test for posix_fadvise() */
#if defined(__linux__) || defined(__FreeBSD__)
#define HAVE_FADVISE
#endif

#define IO_THREADS_MAX_NUM 128
#define MAX_THREADS_NUM (IO_THREADS_MAX_NUM + 3 + 1)

#ifndef CACHE_LINE_SIZE
#if defined(__aarch64__) && defined(__APPLE__)
#define CACHE_LINE_SIZE 128
#else
#define CACHE_LINE_SIZE 64
#endif
#endif

#endif


Review comment (performance):
(Minor) CACHE_LINE_SIZE is good in this file, but IO_THREADS_MAX_NUM is a little misplaced here, IMHO.

Now that zmalloc has a protection against too many threads, can we just define a limit within zmalloc.c that is independent of IO threads? That's my preference, but I will not insist if others disagree.

Code after:
#include <kernel/OS.h>
#define valkey_set_thread_title(name) rename_thread(find_thread(0), name)
#else
#if (defined __APPLE__ && defined(MAC_OS_X_VERSION_MAX_ALLOWED) && MAC_OS_X_VERSION_MAX_ALLOWED >= 1070)
int pthread_setname_np(const char *name);
#include <pthread.h>
#define valkey_set_thread_title(name) pthread_setname_np(name)
#else
#define valkey_set_thread_title(name)
#endif
#endif
#endif

/* Check if we can use setcpuaffinity(). */
#if (defined __linux || defined __NetBSD__ || defined __FreeBSD__ || defined __DragonFly__)
#define USE_SETCPUAFFINITY
void setcpuaffinity(const char *cpulist);
#endif

/* Test for posix_fadvise() */
#if defined(__linux__) || defined(__FreeBSD__)
#define HAVE_FADVISE
#endif

#define IO_THREADS_MAX_NUM 128

#ifndef CACHE_LINE_SIZE
#if defined(__aarch64__) && defined(__APPLE__)
#define CACHE_LINE_SIZE 128
#else
#define CACHE_LINE_SIZE 64
#endif
#endif

#endif
Repository: valkey-io/valkey
File: src/cluster_slot_stats.h (C/C++)

Code before:
/*
 * Copyright Valkey Contributors.
 * All rights reserved.
 * SPDX-License-Identifier: BSD 3-Clause
 */

#include "server.h"
#include "cluster.h"

void clusterSlotStatReset(int slot);
void clusterSlotStatsReset(void);
void clusterSlotStatsAddCpuDuration(client *c, ustime_t duration);


Review comment (suggestion):
We don't normally add copyright information in header files.
```suggestion
```

Code after:
#include "server.h"
#include "cluster.h"
#include "script.h"
#include "cluster_legacy.h"

void clusterSlotStatReset(int slot);
void clusterSlotStatResetAll(void);
void clusterSlotStatsAddCpuDuration(client *c, ustime_t duration);
void clusterSlotStatsInvalidateSlotIfApplicable(scriptRunCtx *ctx);
Repository: valkey-io/valkey
File: src/db.c (C)

Code before:
    for (int i = 2; i < argc; i++) {
        int remargs = argc - i - 1; /* Remaining args other than current. */
        char *arg = argv[i]->ptr;
        if (!strcasecmp(arg, "get") && remargs >= 2) {
            i += 2;
        } else if ((!strcasecmp(arg, "set") || !strcasecmp(arg, "incrby")) && remargs >= 3) {
            readonly = 0;
            i += 3;
            break;
        } else if (!strcasecmp(arg, "overflow") && remargs >= 1) {
            i += 1;
        } else {
            readonly = 0; /* Syntax error. safer to assume non-RO. */
            break;
        }
    }

    if (readonly) {
        keys[0].flags = CMD_KEY_RO | CMD_KEY_ACCESS;
    } else {
        keys[0].flags = CMD_KEY_RW | CMD_KEY_ACCESS | CMD_KEY_UPDATE;
    }
    return 1;
}

bool dbsHaveNoKeys(void) {
    for (int i = 0; i < server.dbnum; i++) {
        if (server.db[i] && kvstoreSize(server.db[i]->keys) != 0) {
            return false;
        }
    }
    return true;
}


Review comment (refactor):
It's odd that this function is in db.c while dbHasNoKeys and createDatabase is in server.c.

I like to think of server.c as the unit handling the global server state and db.c handling the serverDb abstract datatype. This is not true though. The abstractions are mixed and the globals are accessed from all over the place.

Anyway, we can try to avoid making this mess even more messy by keeping these related functions together, or do you have another idea?

Code after:

    for (int i = 2; i < argc; i++) {
        int remargs = argc - i - 1; /* Remaining args other than current. */
        char *arg = argv[i]->ptr;
        if (!strcasecmp(arg, "get") && remargs >= 2) {
            i += 2;
        } else if ((!strcasecmp(arg, "set") || !strcasecmp(arg, "incrby")) && remargs >= 3) {
            readonly = 0;
            i += 3;
            break;
        } else if (!strcasecmp(arg, "overflow") && remargs >= 1) {
            i += 1;
        } else {
            readonly = 0; /* Syntax error. safer to assume non-RO. */
            break;
        }
    }

    if (readonly) {
        keys[0].flags = CMD_KEY_RO | CMD_KEY_ACCESS;
    } else {
        keys[0].flags = CMD_KEY_RW | CMD_KEY_ACCESS | CMD_KEY_UPDATE;
    }
    return 1;
}
Repository: valkey-io/valkey
File: src/cluster.h (C/C++)

Code before:
long long clusterNodeReplOffset(clusterNode *node);
clusterNode *clusterLookupNode(const char *name, int length);
int detectAndUpdateCachedNodeHealth(void);
client *createCachedResponseClient(int resp);
void deleteCachedResponseClient(client *recording_client);
void clearCachedClusterSlotsResponse(void);
unsigned int countKeysInSlot(unsigned int hashslot);
int getSlotOrReply(client *c, robj *o);

/* functions with shared implementations */
int clusterNodeIsMyself(clusterNode *n);
clusterNode *getNodeByQuery(client *c, struct serverCommand *cmd, robj **argv, int argc, int *hashslot, int *ask);
int clusterRedirectBlockedClientIfNeeded(client *c);
void clusterRedirectClient(client *c, clusterNode *n, int hashslot, int error_code);
void migrateCloseTimedoutSockets(void);
unsigned int keyHashSlot(char *key, int keylen);
int patternHashSlot(char *pattern, int length);
int isValidAuxString(char *s, unsigned int length);
void migrateCommand(client *c);
void clusterCommand(client *c);
ConnectionType *connTypeOfCluster(void);
int isNodeAvailable(clusterNode *node);
long long getNodeReplicationOffset(clusterNode *node);
sds aggregateClientOutputBuffer(client *c);
void resetClusterStats(void);
int verifyPortNumber(client *c, long long *port, long long *cport);
#endif /* __CLUSTER_H */


Review comment (suggestion):
This function does more than just verification, it also parses and assigns port values. Consider renaming it to something like `parseAndValidatePorts`  for better clarity.

Code after:
clusterNode *clusterLookupNode(const char *name, int length);
int detectAndUpdateCachedNodeHealth(void);
client *createCachedResponseClient(int resp);
void deleteCachedResponseClient(client *recording_client);
void clearCachedClusterSlotsResponse(void);
unsigned int countKeysInSlot(unsigned int hashslot);
int getSlotOrReply(client *c, robj *o);

/* functions with shared implementations */
int clusterNodeIsMyself(clusterNode *n);
clusterNode *getNodeByQuery(client *c, struct serverCommand *cmd, robj **argv, int argc, int *hashslot, int *ask);
int clusterRedirectBlockedClientIfNeeded(client *c);
void clusterRedirectClient(client *c, clusterNode *n, int hashslot, int error_code);
void migrateCloseTimedoutSockets(void);
unsigned int keyHashSlot(char *key, int keylen);
int patternHashSlot(char *pattern, int length);
int isValidAuxString(char *s, unsigned int length);
void migrateCommand(client *c);
void clusterCommand(client *c);
ConnectionType *connTypeOfCluster(void);
int isNodeAvailable(clusterNode *node);
long long getNodeReplicationOffset(clusterNode *node);
sds aggregateClientOutputBuffer(client *c);
void resetClusterStats(void);
#endif /* __CLUSTER_H */