Clean up lookahead-related code#3
Clean up lookahead-related code#3tomerqodo wants to merge 7 commits intocodex_full_base_clean_up_lookahead-related_code_pr3from
Conversation
Co-authored-by: Ozan Tezcan <ozantezcan@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfae1193e6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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)); |
There was a problem hiding this comment.
The loop iterates with j <= keys_result->numkeys, which means when j == numkeys it reads keys_result->keys[numkeys] and writes slot_buffer[numkeys]. Both are one past the allocated bounds (the arrays are sized for numkeys elements), so any command with at least one key can trigger out-of-bounds memory access and crashes or memory corruption. The condition should be < keys_result->numkeys.
Useful? React with 👍 / 👎.
| /* Allocate temporary buffer for slot tracking */ | ||
| int *slot_buffer = malloc(sizeof(int) * keys_result->numkeys); | ||
|
|
There was a problem hiding this comment.
Repo root AGENTS.md rule #2 requires using zmalloc/zfree instead of malloc/free. This new malloc-allocated slot_buffer (and later free) bypasses Redis memory tracking and violates the repository’s allocator policy, which can lead to incorrect memory accounting and inconsistent behavior with allocator hooks. Replace with zmalloc/zfree (or remove the buffer if unnecessary).
Useful? React with 👍 / 👎.
2 participants
Benchmark PR from agentic-review-benchmarks#3