orbisai0security commented on PR #73: URL: https://github.com/apache/apr/pull/73#issuecomment-4474863132
Thanks for the thorough review. You were right on all counts — the original patch was sloppy and overclaimed. I've force-pushed a reworked version that: 1. **Removes the APR_BUFFER_MAX checks entirely** — they are unreachable dead code. The `size` bitfield is 63 bits on 64-bit (31 on 32-bit), so its max representable value equals `APR_BUFFER_MAX`. The condition `> APR_BUFFER_MAX` is always false. 2. **Keeps only the NULL alloc check** in `apr_buffer_cpy()` — this prevents `memcpy(NULL, src, size)` which is undefined behaviour when the user-supplied allocator fails. 3. **Updates the `apr_buffer_cpy()` documentation** to explicitly state that NULL may be returned on allocation failure, consistent with `apr_buffer_arraydup()`'s documented APR_ENOMEM. 4. **Adds a test** exercising the allocation failure path. The PR title and description have been rewritten to accurately describe the single focused change. Apologies for the earlier noise. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
