Skip to content

BUG: Do not write to the stream that failed to open - #129

Open
hjmjohnson wants to merge 2 commits into
stack/fix-fslio-64bit-arithmeticfrom
stack/fix-cifti-null-stream
Open

hjmjohnson wants to merge 2 commits into
stack/fix-fslio-64bit-arithmeticfrom
stack/fix-cifti-null-stream

Conversation

@hjmjohnson

Copy link
Copy Markdown
Member

Re-submission of #72, reverted from master on 2026-09-24. Content is
unchanged from the original.

Position 7 of 11 in the deep stack. Base: stack/fix-fslio-64bit-arithmetic.

Based on the pull request above it in the stack, so the diff shown here is
this change alone. Merge the stack bottom-up.

Stack order

# branch base
1 stack/test-regression-coverage master
2 stack/pr-fix-alloc-null-checks stack/test-regression-coverage
3 stack/fix-axml-skip-depth stack/pr-fix-alloc-null-checks
4 stack/pr-fix-analyzer-leaks stack/fix-axml-skip-depth
5 stack/pr-fix-sign-conversion stack/pr-fix-analyzer-leaks
6 stack/fix-fslio-64bit-arithmetic stack/pr-fix-sign-conversion
7 stack/fix-cifti-null-stream <- this PR stack/fix-fslio-64bit-arithmetic
8 stack/pr-fix-calloc-transposed-args stack/fix-cifti-null-stream
9 stack/pr-fix-shorten-64-to-32 stack/pr-fix-calloc-transposed-args
10 stack/fix-image-read-complex-check stack/pr-fix-shorten-64-to-32
11 stack/pr-fix-xml-read-errors stack/fix-image-read-complex-check

The order is the order these changes sat on master before the revert, so
it builds and tests at every step.

Commits introduced by this PR
  • BUG: Do not write to the stream that failed to open
  • ENH: Cover the unopenable -output path in cifti_tool

Ordering for all the re-submitted work is tracked in #84.

gdevenyi and others added 2 commits September 24, 2026 07:12
open_write_stream() returns NULL when its fopen() fails, and reports
it.  Both callers then used the result without testing it.

  $ cifti_tool -input c.nii -disp_cext -output /nonexistent-dir/x.txt
  ** failed to open '/nonexistent-dir/x.txt' for writing
  Segmentation fault (core dumped)

disp_cifti_extension() passes it to fprintf().  eval_cifti_extension()
hands it to axml_set_wstream(), so the NULL becomes the xml library's
write stream and every later write follows it.  Both now return 1; the
message has already been printed.

This file is behind USE_CIFTI_CODE, which defaults to OFF, so the
standard workflows do not compile it.  Verified with
-DUSE_CIFTI_CODE=ON.

(cherry picked from commit f368af9)
Driven through a cmake -P script rather than add_test, because
PASS_REGULAR_EXPRESSION ignores the exit status and a crash that had
already printed the diagnostic would pass.

The script pins that the tool exits normally and reports the failure,
and that an openable -output is still written with the extension in
it, so refusing every write would not pass.

(cherry picked from commit 5c08eb3)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants