📄 src/cluster.c:1112

The loop condition j <= keys_result->numkeys iterates one time too many. When numkeys is N, this accesses keys_result->keys[N] which is out-of-bounds (valid indices are 0..N-1), and writes to slot_buffer[N] which is also out-of-bounds (buffer was allocated for exactly N elements).

This causes:

1. Heap buffer overflow writing to slot_buffer[numkeys] (1 element past allocation)
2. Out-of-bounds read of keys_result->keys[numkeys].pos, leading to undefined behavior — could crash, corrupt data, or read garbage producing incorrect slot calculations

This is a guaranteed bug for any command with 1+ keys.

    for (int j = 0; j < keys_result->numkeys; j++) {

📄 src/cluster.c:1110

The slot_buffer array is allocated via malloc, populated in the loop, and then freed — but its contents are never read or used for any purpose. The cross-slot check is done entirely through the first_slot variable.

This introduces:

1. An unnecessary malloc/free pair on every command execution — a hot path in Redis
2. A potential memory leak if malloc returns NULL (no NULL check) — though on Linux with overcommit this is unlikely, it's still undefined behavior to write to a NULL pointer
3. Additional complexity with no functional benefit

The entire slot_buffer allocation and writes should be removed.

    int first_slot = INVALID_CLUSTER_SLOT;

    for (int j = 0; j < keys_result->numkeys; j++) {
        robj *this_key = argv[keys_result->keys[j].pos];
        int this_slot = (int)keyHashSlot((char*)this_key->ptr, sdslen(this_key->ptr));

        if (first_slot == INVALID_CLUSTER_SLOT)
            first_slot = this_slot;
        else if (first_slot != this_slot) {
            return CLUSTER_CROSSSLOT;
        }
    }
    return first_slot;

📄 src/server.c:4141

After the if (num_keys < 0) { return; } branch, the else if (num_keys >= 0) condition is guaranteed to be true — it will never be false. This makes the code slightly confusing to read, as it suggests there could be a third case. A plain else (or removing the condition entirely and de-indenting the block) would be clearer.

The old code used else if (num_keys > 0) which was meaningful because it excluded the num_keys == 0 case. The change to >= 0 makes it equivalent to a simple else.

    } else {
        /* Handle cross-slot keys: mark error and reset slot. */
        if (pcmd->slot == CLUSTER_CROSSSLOT) {
            pcmd->read_error = CLIENT_READ_CROSS_SLOT;
            pcmd->slot = INVALID_CLUSTER_SLOT;
        }
    }