BUG: Fix the REJECT_COMPLEX path in nifti_image_read - #74
hjmjohnson merged 2 commits into
Conversation
0518a5e to
80aad59
Compare
Under FSLSTYLE the block that reports "Multiple possible filenames" has
three defects, and they cannot be separated.
free(basename); <- freed here
char *gzname = calloc(...); <- unchecked
strcpy(gzname, hdrname); <- used at once
...
fprintf(stderr,"... %s\n", basename); <- read after free
exit(134); <- kills the caller
basename is read by the message, so it cannot be freed at the top.
gzname goes straight into strcpy(), so it has to be checked. And once
the function returns NULL instead of calling exit(), every path out has
to release what it holds, which is what ties the three together: moving
the free of basename decides what the error paths must free, and there
are no error paths to speak of until exit() is gone.
FSLSTYLE is off by default but is a supported configuration:
-DFSLSTYLE:BOOL=ON turns it on, and nifti2/Makefile defines it always.
Built and tested with -DFSLSTYLE=ON.
The nifti_image_read() half of the original change is now #74.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Under FSLSTYLE the block that reports "Multiple possible filenames" has
three defects, and they cannot be separated.
free(basename); <- freed here
char *gzname = calloc(...); <- unchecked
strcpy(gzname, hdrname); <- used at once
...
fprintf(stderr,"... %s\n", basename); <- read after free
exit(134); <- kills the caller
basename is read by the message, so it cannot be freed at the top.
gzname goes straight into strcpy(), so it has to be checked. And once
the function returns NULL instead of calling exit(), every path out has
to release what it holds, which is what ties the three together: moving
the free of basename decides what the error paths must free, and there
are no error paths to speak of until exit() is gone.
FSLSTYLE is off by default but is a supported configuration:
-DFSLSTYLE:BOOL=ON turns it on, and nifti2/Makefile defines it always.
Built and tested with -DFSLSTYLE=ON.
The nifti_image_read() half of the original change is now #74.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
80aad59 to
e93a6e0
Compare
e93a6e0 to
7d30810
Compare
|
Half of this PR is now on
What What the branch is nowTwo commits on
The ancestor commits the branch carried are already upstream under new SHAs and dropped out during the rebase. Message cleanupRemoved the Test and red proof
With the fix reverted to The test process is killed at status 13 before it reaches either assertion, which is the defect stated directly. With the fix: Build and test
Four of the added lines are re-indentation: the block's closing |
nifti_image_read() ended the calling process with exit(13) when it met a complex datatype, leaking the open file and hfile on the way out. A library reports the condition to its caller: the block now frees the image, closes the file, releases hfile and returns NULL, which is how every other rejection in the function behaves. The message also loses its "64", which was wrong for the 128 and 256 cases. REJECT_COMPLEX is off by default but is supported: -DFSLSTYLE:BOOL=ON turns it on, and nifti2/Makefile defines it always.
Writes a DT_COMPLEX64 image and a DT_FLOAT32 image, then asserts nifti_image_read() returns NULL for the first and a usable image for the second, so a blanket refusal cannot pass. Registered only when FSLSTYLE_REJECT_COMPLEX is on. Against exit(13) the test process dies with status 13 before reaching either assertion.
7d30810 to
5c219c0
Compare
461cd33
into
InsightSoftwareConsortium:master
2 participants
The block has two defects, and they interlock.
It sits before the
if( nim == NULL )test but readsnim->datatype, so a header that fails to convert is dereferenced as NULL rather than reported. And it callsexit(13), which ends the caller's process from inside a library and leaks the open file andhfileon the way out.Moving it after the NULL check is what makes returning possible, so both are fixed together: the block now runs on a
nimthat exists, frees the image, closes the file, releaseshfileand returns NULL.The message also loses its "64", which was wrong for the 128 and 256 cases.
Split out of #54, which keeps the
nifti_findhdrname()half. The two touch different functions and do not conflict.REJECT_COMPLEXis off by default but is a supported configuration:-DFSLSTYLE:BOOL=ONturns it on, andnifti2/Makefiledefines it always. Built and tested with-DFSLSTYLE=ON.🤖 Generated with Claude Code