Fixed some warnings from the new cppcheck 2.19

[seanm]

• duplicateBreak,fsliolib/fslio.c:812,style,Consecutive return, break, continue, goto or throw statements are unnecessary.
• duplicateBreak,fsliolib/fslio.c:2095,style,Consecutive return, break, continue, goto or throw statements are unnecessary.
• nullPointerOutOfMemory,nifticdf/nifticdf.c:11053,warning,If memory allocation fails, then there is a possible null pointer dereference: upt

[gdevenyi]

Closed PR Comment

#26 includes the two fslio.c hunks from this PR — the unreachable return (NULL) after FSLIOERR in FslReadAllVolumes() and FslReadHeader(). They are correct: FSLIOERR calls exit(), so those returns cannot run.

Worth noting the assumption explicitly, since #26 also removes exit() from niftilib and nifti2: these two deletions are only safe because FSLIOERR stays fatal. #26 deliberately leaves it that way — de-fatalising it changes the contract at roughly fifty call sites and belongs in its own change — but if anyone does that later, these two returns have to come back.

The third hunk, the malloc NULL check in nifti_intent_code(), is moot in #26: it removes nifticdf entirely. If that removal does not land, this hunk is still needed.

#26 also generalises the class — nine allocations whose result was dereferenced within a line or two without a check, in FslInit(), FslReadHeader(), FslGetHdrImgNames(), FslWriteVolumes(), nifti_read_header(), the four PIGZ znzFile allocations, and three strdup calls in afni_xml.c.

Same offer as on #22: merge first and I will rebase, or close as covered.

[hjmjohnson]

Rebased onto current master (b4876bf) and split into two commits so the real fix and the dead-code removal carry different prefixes. The diff is unchanged.

The fslio.c half is only correct because FSLIOERR calls exit(). That is worth a decision, not just a merge — see below.

Rebase

master's last 44 commits were rewritten on 2026-09-22 (messages only, trees byte-identical), so this branch's base no longer existed and GitHub reported it CONFLICTING. Rebased with git rebase --onto master cf340f4^; applied without conflict, and diff <(git diff master HEAD) <(git diff cf340f4^ cf340f4) is empty — the content delta is byte-identical.

Commit messages

The single commit Fixed some warnings from the new cppcheck 2.19 became two, each naming the cppcheck check it answers, with the project's prefix and wrapped to 78/72. Author and author date preserved on both. Nothing was stripped; the original carried no tool trailer or transient URL.

• BUG: Check the allocation in nifti_intent_code before copying into it (nullPointerOutOfMemory)
• STYLE: Drop the unreachable returns after FSLIOERR (unreachableCode)

No regression test — and why

nifti_intent_code malloc check — BEHAVIORAL, but NOT-DEMONSTRABLE as a ctest. The only way to reach the new branch is to make a 1-to-40-byte malloc fail, which needs allocator interposition (DYLD_INSERT_LIBRARIES / LD_PRELOAD / a __wrap_malloc link) — platform-specific, and not something the suite has a seam for today. A test that merely calls nifti_intent_code("garbage") returns -1 with and without the fix, so it would be worthless.

FSLIOERR returns — COSMETIC. The statements are unreachable by construction; there is no runtime difference to observe.

The FSLIOERR question a maintainer should settle

#define FSLIOERR(x) { fprintf(stderr,"Error:: %s\n",(x)); fflush(stderr); exit(EXIT_FAILURE); }

FslReadAllVolumes and FslReadHeader are library entry points. Today they terminate the calling process on a bad file rather than returning NULL, and the return(NULL) lines this PR deletes are the visible trace of an intent that the macro never honored. Removing them silences cppcheck and is correct against the code as written, but it also removes the marker showing where the non-fatal path was meant to be, and it makes FSLIOERR harder to change to a non-exiting form later — every caller would then need its error return written back in.

Two coherent positions:

1. exit() in fsliolib is accepted as-is; merge this as the dead-code removal it is.
2. The library should not terminate its host; then the fix is to change FSLIOERR and keep these returns, and this half of the PR should be dropped rather than merged.

I have not chosen for you — the nifticdf commit stands on its own either way and can be merged independently.

Build and test

macOS/arm64, Ninja, Release, -DNIFTI_BUILD_APPLICATIONS=ON -DUSE_NIFTI2_CODE=ON -DUSE_CIFTI_CODE=ON -DUSE_FSL_CODE=ON: builds clean, 100% tests passed, 0 tests failed out of 362.

[hjmjohnson]

The objection previously raised against this PR is withdrawn. FSLIOERR keeping its exit() has since been accepted as a deliberate, permanent carve-out for fsliolib, so the two return(NULL) lines this deletes are genuinely dead code rather than the trace of a future non-fatal path.

Rebased onto current master. Two commits, independent of each other.

Why the earlier objection no longer applies

The concern was that these returns are unreachable only because the macro never returns, and that deleting them would make a later change to FSLIOERR harder — the deleted lines being the visible marker of where the non-fatal path would need to go.

That change is now settled as one that will not happen. The measurement behind the decision: 71 FSLIOERR call sites in fslio.c, of which 63 are if (...) FSLIOERR(...); with no braces, and by enclosing return type 29 are in void functions with no error channel at all, 18 return size_t where neither 0 nor -1 is usable, and 11 return double with no error convention. Only 13 of 71 have a usable error value. fsliolib also has no tests, and USE_FSL_CODE is OFF by default.

So the returns are dead, and deleting dead code is the right call.

Commit 1: the nifticdf allocation check

   unam = (char *)malloc(strlen(name)+1);
+  if (!unam)
+     return -1 ;
   strcpy(unam,name);

Correct and minimal. -1 is already this function's error value — the line above returns it for a NULL or empty name — so the check reports failure through the channel the function already has, rather than inventing one.

Independent of the fsliolib commit; either can be taken alone.

Commit 2: the unreachable returns

Both sites are braced, so this is a plain deletion with no effect on control flow:

   if (fslio->niftiptr == NULL) {
         FSLIOERR("FslReadAllVolumes: error reading NIfTI image");
-        return(NULL);
   }

   FslSetFileType(fslio,fslio->niftiptr->nifti_type);

One thing worth seeing rather than glossing: after the deletion, the line following the block dereferences the pointer the block just found to be NULL. That is correct only because FSLIOERR exits, and the deleted return was the last visual cue to that. It is the accepted design now, but a reader arriving cold will not see it from this code alone.

Test coverage

No test is added, and none applies:

• The allocation check is reachable only on allocation failure, which this suite has no seam to force. NOT-DEMONSTRABLE.
• The deletions remove unreachable statements and change no behavior. COSMETIC.

Manufacturing a test for either would produce one that passes identically with and without the change.

Verification

Configuration	Result
Release, cifti + FSL + FSLSTYLE=ON	367/367, no new warnings

FSLSTYLE=ON is the configuration that compiles fsliolib at all, so it is the one this needed. The single warning in that build is the pre-existing ld: ignoring duplicate libraries notice, which is present on master too.