refactor(memory): add safe_free, safe_str_free, safe_buf_free, safe_grow to platform.h; cleanup#210
refactor(memory): add safe_free, safe_str_free, safe_buf_free, safe_grow to platform.h; cleanup#210map588 wants to merge 3 commits intoDeusData:mainfrom
Conversation
…row to platform.h Centralize heap memory management into four safe wrappers alongside existing safe_realloc: - safe_free(ptr): frees and NULLs any pointer to prevent double-free - safe_str_free(&str): frees const char* with NULL-out (replaces free((void*)str)) - safe_buf_free(buf, &count): frees array and zeros its count - safe_grow(arr, n, cap, factor): one-line capacity-doubling realloc Applied across cypher.c, store.c, mcp.c, and pass_githistory.c, eliminating ~60 lines of repetitive free/grow boilerplate.
There was a problem hiding this comment.
Pull request overview
This PR centralizes heap memory management into platform.h helpers and applies them across the store, cypher parser, MCP grep collection, and git-history parsing to reduce repetitive free/grow boilerplate and improve consistency.
Changes:
- Add
safe_free,safe_str_free,safe_buf_free, andsafe_growhelpers alongsidesafe_reallocinsrc/foundation/platform.h. - Replace many direct
free(...)and manual capacity-growth blocks with the new wrappers across multiple modules. - Standardize “free + NULL-out” behavior for heap-owned strings and arrays in several cleanup paths.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/foundation/platform.h | Introduces new “safe” memory-management wrappers/macros used across the codebase. |
| src/store/store.c | Replaces many manual free/realloc growth patterns with safe_* helpers across store queries and free functions. |
| src/cypher/cypher.c | Updates parser/lexer/result cleanup and dynamic array growth to use the new wrappers. |
| src/mcp/mcp.c | Simplifies grep-match dynamic array growth with safe_grow. |
| src/pipeline/pass_githistory.c | Simplifies commit-list growth with safe_grow in both libgit2 and popen implementations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /* Safe free: frees and NULLs a pointer to prevent double-free / use-after-free. | ||
| * Accepts void** so it works with any pointer type via the macro. */ | ||
| static inline void safe_free_impl(void **pp) { | ||
| if (pp && *pp) { | ||
| free(*pp); | ||
| *pp = NULL; | ||
| } | ||
| } | ||
| #define safe_free(ptr) safe_free_impl((void **)(void *)&(ptr)) | ||
|
|
||
| /* Safe string free: frees a const char* and NULLs it. | ||
| * Casts away const so callers don't need the (void*) dance. */ | ||
| static inline void safe_str_free(const char **sp) { | ||
| if (sp && *sp) { | ||
| free((void *)*sp); | ||
| *sp = NULL; | ||
| } | ||
| } | ||
|
|
||
| /* Safe buffer free: frees a heap array and zeros its element count. | ||
| * Use for dynamic arrays paired with a size_t count. */ | ||
| static inline void safe_buf_free_impl(void **buf, size_t *count) { | ||
| if (buf && *buf) { | ||
| free(*buf); | ||
| *buf = NULL; | ||
| } | ||
| if (count) { | ||
| *count = 0; | ||
| } | ||
| } | ||
| #define safe_buf_free(buf, countp) safe_buf_free_impl((void **)(void *)&(buf), (countp)) | ||
|
|
There was a problem hiding this comment.
safe_free/safe_buf_free implement pointer-nullification by casting &(ptr) to void **. In standard C this is not type-safe: writing through a void ** that actually points to (e.g.) a char * object can violate strict-aliasing rules and is undefined behavior under optimization. Suggest implementing safe_free/safe_buf_free as statement macros that operate directly on the original lvalue (free then assign NULL), or using a C11 _Generic wrapper to preserve the correct pointer-to-pointer type without void ** casting.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
| /* Safe grow: doubles capacity and reallocs when count reaches cap. | ||
| * Usage: safe_grow(arr, count, cap, growth_factor) | ||
| * Evaluates to the new arr (NULL on OOM — old memory freed by safe_realloc). */ | ||
| #define safe_grow(arr, n, cap, factor) do { \ | ||
| if ((size_t)(n) >= (size_t)(cap)) { \ | ||
| (cap) *= (factor); \ | ||
| (arr) = safe_realloc((arr), (size_t)(cap) * sizeof(*(arr))); \ | ||
| } \ | ||
| } while (0) |
There was a problem hiding this comment.
The safe_grow comment says it "doubles capacity" and "evaluates to the new arr", but the macro is a do { ... } while (0) statement and grows by multiplying cap by the caller-provided factor. Please update the comment to match the actual semantics to avoid misuse/confusion (especially since callers might pass factors other than 2).
Centralize heap memory management into four safe wrappers alongside existing safe_realloc:
Applied across cypher.c, store.c, mcp.c, and pass_githistory.c, eliminating ~60 lines of repetitive free/grow boilerplate.