add malloc failure test for cgif_raw API#93
Conversation
| if(pContext == NULL) { | ||
| return CGIF_EALLOC; | ||
| } | ||
| memset(pContext, 0, sizeof(LZWGenState)); |
There was a problem hiding this comment.
Found by the new ealloc_raw test with ASAN:
When an early malloc fails, the goto cleanup path frees all pContext fields, including ones never assigned. Zero-initialize pContext so unassigned pointers are NULL and safe to free.
There was a problem hiding this comment.
Pull request overview
Adds a targeted regression test that injects malloc failures across the cgif_raw encode path to ensure allocation failures are handled cleanly (no crashes / invalid frees), and tightens internal initialization to make failure cleanup safe.
Changes:
- Add a new Meson test target (
ealloc_raw) that compilescgif_raw.cdirectly somalloccan be intercepted. - Introduce
tests/ealloc_raw.c, which iteratively fails eachmalloccall duringcgif_raw_newgif+cgif_raw_addframe+cgif_raw_close. - Zero-initialize
LZWGenStateafter allocation to ensure cleanup paths can safelyfree()partially-initialized members.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
tests/meson.build |
Registers the new ealloc_raw malloc-failure injection test executable. |
tests/ealloc_raw.c |
Implements iterative malloc failure injection by compiling cgif_raw.c into the test TU with a malloc macro override. |
src/cgif_raw.c |
Initializes LZWGenState to zero to make allocation-failure cleanup safe. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MCLoebl
left a comment
There was a problem hiding this comment.
Looks good. Minor stuff: is the initialization in l.11,12 of ealloc_raw.c needed? I suggest replacing n by the more intuitive name fail_after in l.69, 78, 87 as well.
Iteratively fail each malloc call in the cgif_raw code path (newgif + addframe + close) to verify all allocation failures are handled correctly without crashes.