tidy-up: drop stray casts for allocated pointers#21865
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tidies up C allocations across curl’s codebase and documentation by removing unnecessary casts from malloc/calloc/realloc-family calls, and it starts adding allocation-failure handling in a few example/debug-style code paths.
Changes:
- Remove stray pointer casts on allocation/reallocation calls across lib/, src/, tests/, projects/, and docs/.
- Add initial allocation-failure guards in libuv/libevent example and tool debug code paths (but some call sites still need NULL handling).
- Minor formatting/line-wrapping adjustments around realloc calls.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/libtest/lib2302.c | Removes an unnecessary cast from a curlx_calloc() assignment in a websocket libtest. |
| src/tool_operate.c | Refactors libuv context allocation; adds a NULL guard inside the allocator (call site still needs to handle NULL). |
| src/tool_formparse.c | Removes an unnecessary cast from a curlx_calloc() allocation. |
| src/tool_cb_wrt.c | Removes an unnecessary cast from a curlx_realloc() call in the Windows console path. |
| src/mkhelp.pl | Removes an unnecessary cast in generated zlib allocator glue code. |
| projects/OS400/curlmain.c | Removes an unnecessary cast from malloc() in OS/400 argument conversion main. |
| projects/OS400/curlcl.c | Removes an unnecessary cast from malloc() in the OS/400 CL wrapper (already checks allocation). |
| projects/OS400/ccsidcurl.c | Removes unnecessary casts from malloc()/calloc() in OS/400 CCSID getinfo handling. |
| lib/vtls/schannel.c | Removes unnecessary casts from curlx_malloc() allocations. |
| lib/vtls/schannel_verify.c | Removes unnecessary casts from curlx_malloc() allocations. |
| lib/vquic/cf-ngtcp2.c | Removes an unnecessary cast and rewraps a curlx_realloc() call. |
| lib/mime.c | Removes unnecessary casts and slightly refactors declarations around curlx_malloc(). |
| lib/content_encoding.c | Removes an unnecessary cast in the zlib zalloc callback implementation. |
| docs/FAQ.md | Removes an unnecessary cast in an example realloc() call (reveals an unsafe realloc pattern needing adjustment). |
| docs/examples/multi-uv.c | Refactors allocation + adds NULL guard in libuv example context creation (caller still needs to handle NULL). |
| docs/examples/multi-event.c | Refactors allocation + adds NULL guard in libevent example context creation (caller still needs to handle NULL). |
Comments suppressed due to low confidence (1)
docs/FAQ.md:1024
- This FAQ example assigns realloc() directly to mem->memory. If realloc() fails, the original pointer is lost (leak) and subsequent cleanup cannot free it. Use a temporary pointer and only update mem->memory after success; on failure, return 0 to signal a write error.
mem->memory = realloc(mem->memory, mem->size + realsize + 1);
if(mem->memory) {
memcpy(&(mem->memory[mem->size]), ptr, realsize);
mem->size += realsize;
mem->memory[mem->size] = 0;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| struct contextuv *c = curlx_malloc(sizeof(*c)); | ||
|
|
||
| c->sockfd = sockfd; | ||
| c->uv = uv; | ||
| if(c) { | ||
| c->sockfd = sockfd; | ||
| c->uv = uv; |
| struct curl_context *context = malloc(sizeof(*context)); | ||
|
|
||
| context = (struct curl_context *)malloc(sizeof(*context)); | ||
| if(context) { | ||
| context->sockfd = sockfd; | ||
| context->uv = uv; | ||
|
|
||
| context->sockfd = sockfd; | ||
| context->uv = uv; | ||
|
|
||
| uv_poll_init_socket(uv->loop, &context->poll_handle, sockfd); | ||
| context->poll_handle.data = context; | ||
| uv_poll_init_socket(uv->loop, &context->poll_handle, sockfd); | ||
| context->poll_handle.data = context; | ||
| } |
| struct curl_context *context = malloc(sizeof(*context)); | ||
|
|
||
| context->sockfd = sockfd; | ||
|
|
||
| context->event = event_new(base, sockfd, 0, curl_perform, context); | ||
| if(context) { | ||
| context->sockfd = sockfd; | ||
| context->event = event_new(base, sockfd, 0, curl_perform, context); | ||
| } |
There was a problem hiding this comment.
True, but it's debug/example code and I don't feel going the extra mile here.
Also how to fail in this case? Leaving this for a future fix if someone is interested.
There was a problem hiding this comment.
Or perhaps this patch should not attempt to touch this at all, since it makes
these OOM/NULL-deref issues less visible.
There was a problem hiding this comment.
Rolling back NULL checks.
| /* Allocate memory for the ASCII arguments and vector. */ | ||
| argv = (char **)malloc((argc + 1) * sizeof(*argv) + bytecount); | ||
| argv = malloc((argc + 1) * sizeof(*argv) + bytecount); | ||
|
|
||
| /* Build the vector and convert argument encoding. */ | ||
| outbuf = (char *)(argv + argc + 1); |
There was a problem hiding this comment.
True, but not attempting to fix this in untestable code.
Also: - check for failed allocations in example and debug code.
Initial version of this patch tried to add missing NULL checks
to three allocations touched (in example and UV debug code).
I backtracked, because later the NULL pointers were derefed
anyway.