mport: fix upgrade.c ohash callback signatures after ohash update#398
Conversation
The ohash_info struct in lib/libc/ohash now expects: calloc: void *(size_t, size_t, void *) free: void *(void *, void *) Update ecalloc and efree in contrib/mport/libmport/upgrade.c to match the new signatures. Switch from malloc+memset to calloc for correctness. AI-Assisted-by: Claude Sonnet 4.6 Signed-off-by: Lucas Holt <luke@foolishgames.com>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAligns mport's upgrade.c ohash allocator callbacks with the updated libohash API by changing the ecalloc/efree prototypes and implementations to match the new calloc/free signatures and using calloc directly for zeroed allocation. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Code Review
This pull request refactors the memory allocation helper functions in upgrade.c. Specifically, it updates ecalloc to use calloc instead of malloc and memset, and simplifies efree by removing an unused size parameter. I have no further feedback to provide as there are no review comments to evaluate.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Claude finished @laffer1's task —— View job Code Review Complete ✅Reviewed the ohash callback signature fixes in Todo List:
✅ Review ResultsOverall Assessment: LGTM - The changes are correct and necessary fixes for the ohash library update. Memory Management ✅
API Compatibility ✅
Style(9) Compliance ✅
Security ✅
Code Quality ✅
No issues found - this is a well-executed fix for the ohash API compatibility. |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Both
ecallocandefreeignore theirdataargument; if that’s intentional, consider marking it__unused(or casting to(void)data) to make this explicit and avoid warnings. - In
ecalloc, you could improve the error message by including the requested sizes (e.g.,err(1, "calloc %zu x %zu", s1, s2);) to aid debugging when allocation fails.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Both `ecalloc` and `efree` ignore their `data` argument; if that’s intentional, consider marking it `__unused` (or casting to `(void)data`) to make this explicit and avoid warnings.
- In `ecalloc`, you could improve the error message by including the requested sizes (e.g., `err(1, "calloc %zu x %zu", s1, s2);`) to aid debugging when allocation fails.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary
ecallocupdated from(size_t, void *)to(size_t, size_t, void *)to match the newohash_info.callocsignature; implementation switched frommalloc+memsettocallocefreeupdated from(void *, size_t, void *)to(void *, void *)to match the newohash_info.freesignatureFixes build errors introduced after
lib/libc/ohashwas updated:Test plan
cd contrib/mport/libmport && make— confirmupgrade.ocompiles without errorsmake buildworld— confirm no regression in full build🤖 Generated with Claude Code
Summary by Sourcery
Bug Fixes: