BUG: Fix cifti_tool's CIFTI extension search, which never advanced - #126
Open
hjmjohnson wants to merge 7 commits into
Open
hjmjohnson wants to merge 7 commits into
hjmjohnson wants to merge 7 commits into
Conversation
A branch filter naming a branch that does not exist leaves the workflow configured but never run, which looks the same as one that runs and passes: no red check appears because no check appears at all. build.yml sat dead behind a filter naming main on a repository whose branch is master, and the only thing that found it was reading the file. The check judges a filter only when every entry is a literal name, so a release-* pattern or a branch that does not exist yet is left alone, and it reports a name that resolves to nothing rather than one that merely differs from the default. (cherry picked from commit db038e9)
cppcheck 2.19 nullPointerOutOfMemory: the uppercase copy of the name was written into an unchecked malloc result. Return -1, the same value the function already uses for an unrecognized name. (cherry picked from commit c2d181e)
cppcheck 2.19 unreachableCode: FSLIOERR ends in exit(EXIT_FAILURE), so the return statements following it in FslReadAllVolumes and FslReadHeader can never run. (cherry picked from commit f5f3766)
Fixes some clang-tidy bugprone-macro-parentheses warnings. Would be an issue where an argument that is an expression would evaluated different with different precedence. (cherry picked from commit 7387fd5)
Fixes many bugprone-implicit-widening-of-multiplication-result warnings. Here the multiplications were happening in small types (int, usually 32 bit) then stored in large types (size_t, usually 64 bit). The multiplication could have overflowed. Now the multiplication is done with large types and thus less likely to overflow. (cherry picked from commit 6d06faa)
disp_cifti_extension() searched for the CIFTI extension with
ext = nim->ext_list;
for( ind = 0; ind < nim->num_ext; ind++ )
if( ext->ecode == NIFTI_ECODE_CIFTI ) break;
ext is never advanced, so this tests the first extension num_ext times.
cifti_tool could only ever find a CIFTI extension that happened to be
first in the list; with any other extension ahead of it the tool
reported 'no CIFTI extension' for a file that has one. It now indexes
ext_list[ind] and leaves ext NULL when there is no match, which also
avoids the read past the end of the list that a bare ext++ would have
introduced.
The same function opened its output stream before the 'no CIFTI
extension' check and returned without closing it; the early return now
closes the stream like the normal path does.
(cherry picked from commit d93153c)
cext_second_extension.nii carries a comment extension ahead of the CIFTI one. The test requires the extension's payload in the output and forbids the 'no CIFTI extension' message, and the existing unterminated-cext test still pins the first-position case, so neither a search that always matches nor one that never does would pass. (cherry picked from commit 2405d09)
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
3 participants
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.
Re-submission of #56, reverted from
masteron 2026-09-24. Content isunchanged from the original.
Position 4 of 11 in the deep stack. Base:
stack/fix-axml-skip-depth.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
stack/test-regression-coveragemasterstack/pr-fix-alloc-null-checksstack/test-regression-coveragestack/fix-axml-skip-depthstack/pr-fix-alloc-null-checksstack/pr-fix-analyzer-leaks<- this PRstack/fix-axml-skip-depthstack/pr-fix-sign-conversionstack/pr-fix-analyzer-leaksstack/fix-fslio-64bit-arithmeticstack/pr-fix-sign-conversionstack/fix-cifti-null-streamstack/fix-fslio-64bit-arithmeticstack/pr-fix-calloc-transposed-argsstack/fix-cifti-null-streamstack/pr-fix-shorten-64-to-32stack/pr-fix-calloc-transposed-argsstack/fix-image-read-complex-checkstack/pr-fix-shorten-64-to-32stack/pr-fix-xml-read-errorsstack/fix-image-read-complex-checkThe order is the order these changes sat on
masterbefore the revert, soit builds and tests at every step.
Commits introduced by this PR
Ordering for all the re-submitted work is tracked in #84.