fix(examples): allocate geo_z instead of geo_y in 3D readGeometry - #375
Open
andrewwhitecdw wants to merge 2 commits into
Open
fix(examples): allocate geo_z instead of geo_y in 3D readGeometry#375andrewwhitecdw wants to merge 2 commits into
andrewwhitecdw wants to merge 2 commits into
Conversation
## Summary
In the `readGeometry()` helper of the C API examples, the 3D branch
allocated `*geo_y` a second time instead of allocating `*geo_z`. The
subsequent `fscanf` then wrote through the (NULL) `*geo_z` pointer,
causing a segmentation fault whenever 3D geometry input was used.
## Root cause
```c
*geo_x = (double *)malloc(n * sizeof(double));
*geo_y = (double *)malloc(n * sizeof(double));
if (dimension == 3)
{
*geo_y = (double *)malloc(n * sizeof(double)); // leaks geo_y,
// geo_z never allocated
for (int i = 0; i < n; i ++)
if (3 != fscanf(fin, "%lf %lf %lf\n",
*geo_x + i, *geo_y + i, *geo_z + i)) // write to NULL
```
Callers initialize `gz = NULL`, pass `&gz`, and later `free(gz)` — so
`*geo_z` is expected to be allocated here.
## Fix
Allocate `*geo_z` in the 3D branch (one-word change in each of
`examples/amgx_capi.c` and `examples/amgx_capi_multi.c`).
## Testing
Full AMGX build/tests were not run (GPU/CUDA-heavy dependency).
Instead, `readGeometry()` was extracted verbatim into a standalone C
harness and compiled with `gcc -Wall -Wextra` (clean):
- Fixed version: 3D and 2D geometry files parse correctly, `geo_z`
values verified -> PASS (exit 0).
- Original (pre-fix) version: segfaults on the 3D input
(exit 139, SIGSEGV) — confirming the bug and the fix.
## Why existing tests missed it
The example programs are not exercised with 3D geometry input files by
any automated test; the 2D path (the common case) works fine because
`geo_z` is unused there.
Flags the bug where the 3D branch of readGeometry() allocated *geo_y a second time instead of *geo_z, so fscanf wrote through the NULL *geo_z pointer and segfaulted on any 3D geometry input file. The test extracts readGeometry() verbatim from examples/amgx_capi.c and examples/amgx_capi_multi.c, compiles it with gcc into a small harness, and runs it on a 3D geometry file. Red-green verification: - sh tests/test_geo_z_alloc.sh -> PASS (exit 0) with the fix. - git show HEAD~1:examples/amgx_capi.c > examples/amgx_capi.c (and same for amgx_capi_multi.c), re-run -> FAIL (exit 139, SIGSEGV) on both.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In the
readGeometry()helper of the C API examples, the 3D branch allocated*geo_ya second time instead of allocating*geo_z. The subsequentfscanfthen wrote through the (NULL)*geo_zpointer, causing a segmentation fault whenever 3D geometry input was used.Root cause
Callers initialize
gz = NULL, pass&gz, and laterfree(gz)— so*geo_zis expected to be allocated here.Fix
Allocate
*geo_zin the 3D branch (one-word change in each ofexamples/amgx_capi.candexamples/amgx_capi_multi.c).Testing
Full AMGX build/tests were not run (GPU/CUDA-heavy dependency). Instead,
readGeometry()was extracted verbatim into a standalone C harness and compiled withgcc -Wall -Wextra(clean):geo_zvalues verified -> PASS (exit 0).Why existing tests missed it
The example programs are not exercised with 3D geometry input files by any automated test; the 2D path (the common case) works fine because
geo_zis unused there.