Clean up lookahead-related code#3
Clean up lookahead-related code#3tomerqodo wants to merge 7 commits intocursor_full_base_clean_up_lookahead-related_code_pr3from
Conversation
Co-authored-by: Ozan Tezcan <ozantezcan@gmail.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| 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)); | ||
| slot_buffer[j] = this_slot; |
There was a problem hiding this comment.
Off-by-one error causes out-of-bounds memory access
High Severity
The loop condition j <= keys_result->numkeys iterates one too many times. When j equals numkeys, both keys_result->keys[j] and slot_buffer[j] are accessed out of bounds. The slot_buffer is allocated with numkeys elements (indices 0 to numkeys-1), and keys_result->keys similarly has numkeys entries. The condition should be j < keys_result->numkeys to match all other similar loops in the codebase.
| 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)); | ||
| slot_buffer[j] = this_slot; |
There was a problem hiding this comment.
Unused slot_buffer allocation is dead code
Low Severity
The slot_buffer array is allocated and written to but never read. The function's logic only compares first_slot with this_slot to detect cross-slot keys—the values stored in slot_buffer serve no purpose. The buffer is allocated, populated on each iteration, and then freed without ever being used for any computation or return value.
Benchmark PR from agentic-review-benchmarks#3
Note
Unifies slot handling to improve consistency across cluster operations.
CLUSTER_CROSSSLOTand usesINVALID_CLUSTER_SLOTin place of legacyGETSLOT_*values; removes old macros fromserver.hextractSlotFromKeysResultand updatesgetSlotFromCommandto delegate to it for consistent slot derivationcluster_asm.c(propagation checks),cluster_slot_stats.c(stats gating), and command preprocessing inserver.cINVALID_CLUSTER_SLOTas no-keys/disabled-cluster andCLUSTER_CROSSSLOTas cross-slot errors; minor comment clarifications innetworking.cWritten by Cursor Bugbot for commit cfae119. Configure here.