ENH: Zero-initialize the matrix locals clang cannot prove are filled

[gdevenyi]

Eight of the eleven -Wconditional-uninitialized warnings are false
positives of one shape: a mat33, mat44 or nifti_dmat44 local filled by a
nested loop over all of its elements, which clang cannot prove covers the
whole object.

There is no defect, but the warning earns its place -- the ninth instance
of it was the real bug in nifti_mat44_to_orientation() fixed earlier in
this series -- so the matrices get an explicit zero initialiser rather
than the check being switched off. The cost is zeroing 36 to 128 bytes
in matrix helpers that are not on any hot path.

With this the tree is free of -Wconditional-uninitialized.

---

Interface impact: none. On the union of all these changes, configured with USE_FSL_CODE=ON and USE_CIFTI_CODE=ON: all 448 exported symbols across libniftiio, libnifti2, libznz, libfslio, libnifticdf and libcifti are identical to master under nm -D --defined-only, and all ten installed headers are identical under gcc -E -P. Under gcc -dM -E one macro definition differs, intentionally and only in text: #61 makes FSL_RADIOLOGICAL read (-1) so it is safe inside an expression. Its value is still -1, checked by compiling against each installed fslio.h and printing it.

Verification. This branch: builds with gcc 16.1.1, ctest unchanged from master (2 of 345 fail on master itself in this environment; #31 and #29 each fix one). The union of all the PRs: 0 errors under both gcc 16.1.1 and clang 22.1.8, ctest 345/345 under each, and the whole suite under valgrind memcheck with --trace-children=yes gives 484 traced processes with no invalid access, no uninitialised value and no leak in any nifti binary.

Coordination. Every line of every branch was compared, whitespace-normalised, against the diffs of the open PRs (#11, #21, #22, #23, #24). Where one of those already changes a line, the line was left alone, and the few deliberate overlaps are named in the text above. What survives is 17 compiler warnings, all of them on those lines: 9 -Wsign-conversion (5 in fslio.c for #22, 2 in nifti2_io.c and 2 in nifti_tester001.c for #24) and 8 -Wcalloc-transposed-args in nifti_findhdrname and nifti_findimgname, which #11 rewrites. No formatting changes appear anywhere, to stay clear of #10 and #12.

One of a set of independent, single-purpose PRs. Each bases on master and can be merged on its own, in any order.

The full set of PRs (35)

The union of all of them is on the fork as all-changes, if you want to build and test the lot at once.

CI and build

• COMP: Show test output when the starter workflow fails #28 — COMP: Show test output when the starter workflow fails
• COMP: Give install_linking the source directory instead of guessing it #29 — COMP: Give install_linking the source directory instead of guessing it
• COMP: Guard the CMP0169 policy setting so older CMake still configures #30 — COMP: Guard the CMP0169 policy setting so older CMake still configures
• BUG: Compare NIfTI test output by content, not by compressed bytes #31 — BUG: Compare NIfTI test output by content, not by compressed bytes
• COMP: Make the 'Build and Test' analysis workflow run #57 — COMP: Make the 'Build and Test' analysis workflow run

Configuration and documentation

• ENH: Rework the clang-tidy configuration for C11 #32 — ENH: Rework the clang-tidy configuration for C11
• COMP: Add a shared compiler warning set #33 — COMP: Add a shared compiler warning set
• COMP: Enable _FORTIFY_SOURCE in optimized builds #34 — COMP: Enable _FORTIFY_SOURCE in optimised builds
• COMP: Build as ISO C11 with no compiler extensions #38 — COMP: Build as ISO C11 with no compiler extensions
• DOC: Describe the CMake build and how to run the memory checks #58 — DOC: Describe the CMake build and how to run the memory checks
• COMP: Add CI jobs for clang-tidy and for building with -Werror #59 — COMP: Add CI jobs for clang-tidy and for building with -Werror

Defects

• BUG: Fix cdfbin's argument range check, which could never fire #39 — BUG: Fix cdfbin's argument range check, which could never fire
• BUG: Fix an out-of-bounds indirect call in axio_show_mim_summary #40 — BUG: Fix an out-of-bounds indirect call in axio_show_mim_summary
• BUG: Bound the formatted write in znzprintf, and end the va_list on failure #41 — BUG: Bound the formatted write in znzprintf, and end the va_list on failure
• BUG: Report XML read errors instead of treating them as end of input #42 — BUG: Report XML read errors instead of treating them as end of input
• BUG: Stop returning -1 from functions that return size_t #43 — BUG: Stop returning -1 from functions that return size_t
• BUG: Fix the ambiguous-filename path in nifti_findhdrname #54 — BUG: Stop the library calling exit(), and fix the defects beside it
• BUG: Check the allocations whose result is used immediately #55 — BUG: Check the allocations whose result is used immediately
• BUG: Fix cifti_tool's CIFTI extension search, which never advanced #56 — BUG: Fix cifti_tool's CIFTI extension search, which never advanced
• BUG: Free the extension data when nifti_add_extension fails #62 — BUG: Free the extension data when nifti_add_extension fails
• BUG: Free the header on nifti_tool's duplicate-file failure paths #60 — BUG: Free the header on nifti_tool's duplicate-file failure paths
• BUG: Use memcpy instead of casting to over-aligned pointer types #44 — BUG: Use memcpy instead of casting to over-aligned pointer types
• BUG: Stop casting away const in fslio and cifti #45 — BUG: Stop casting away const in fslio and cifti
• BUG: Bound the aux_file copy by sizeof rather than a repeated 24 #46 — BUG: Bound the aux_file copy by sizeof rather than a repeated 24
• ENH: Make the 64-to-32 bit truncations explicit #52 — BUG: Fix 64-to-32 bit truncations, including a real overflow in fslio
• BUG: Fix signed/unsigned comparisons in the nifti tools #51 — BUG: Fix signed/unsigned comparisons in the nifti tools

Warning and check classes

• STYLE: Add the missing newline at end of nifti1_tool.h #35 — STYLE: Add the missing newline at end of nifti1_tool.h
• ENH: Give the cifti tools' gopt internal linkage #36 — ENH: Give the cifti tools' gopt internal linkage
• COMP: Declare the exported functions that no header declares #37 — COMP: Declare the exported functions that no header declares
• BUG: Give i and j a defined value in nifti_mat44_to_orientation #47 — BUG: Give i and j a defined value in nifti_mat44_to_orientation
• ENH: Zero-initialize the matrix locals clang cannot prove are filled #48 — ENH: Zero-initialise the matrix locals clang cannot prove are filled
• BUG: Give the statement macros a do/while(0) body and drop the stray semicolons #49 — BUG: Give the statement macros a do/while(0) body
• ENH: Pass calloc its arguments in the documented order #50 — ENH: Pass calloc its arguments in the documented order
• BUG: Parenthesize macro arguments and replacement lists #61 — BUG: Parenthesise macro arguments and replacement lists
• ENH: Make the implicit sign conversions explicit #53 — ENH: Make the implicit sign conversions explicit

[hjmjohnson]

Rebased onto master (b4876bf), cleaned the commit message, and corrected one factual claim in it.

No regression test applies: all eight locals are fully written before any read, so zeroing them changes nothing observable. Details below.

Note the PR title says "initialise"; the commit subject is now ENH: Zero-initialize the matrix locals clang cannot prove are filled (US English). Worth retitling the PR to match.

Message cleanup and one correction

Removed the Co-Authored-By: trailer naming an AI tool and the Claude-Session: URL. "initialise"/"initialiser" to "initialize"/"initializer".

The old body ended with "With this the tree is free of -Wconditional-uninitialized." That is not true against current master. Measured on the rebased branch:

$ clang -fsyntax-only -Wconditional-uninitialized ... nifti2/nifti2_io.c
# master:     5 warnings
# this branch: 1 warning

nifti2/nifti2_io.c:2990:24: warning: variable 'j' may be uninitialized when used here
 2990 |    *icod = i ; *jcod = j ; *kcod = k ; }

That remaining one is the real defect in nifti_mat44_to_orientation(), which is not on master yet and is not this PR's to fix. The body now says so explicitly instead of claiming a clean tree.

One fix to the diff

nifti_dmat33 and nifti_dmat44 hold double, but the initializer used the float literal 0.0f. Changed those two to 0.0; the four genuine mat33/mat44 cases keep 0.0f.

Why there is no red-green test

Each of the eight locals is written in full before it is read:

• mat44_to_mat33, nifti_mat33_mul (x3), nifti_tester001: nested loops covering every element of the 3x3 / 4x4.
• nifti_dmat44_mul, nifti_mat44_mul: C.m[i][j] = 0.0; then accumulate, for all 16.

The warning is clang failing to prove whole-object coverage, not a real read of an unset element. So a test that observes any matrix result is identical with and without the change; there is no red to prove. Class is BUILD — the observable effect is five compiler warnings becoming one.

Build and test

Release, NIFTI_BUILD_APPLICATIONS=ON USE_NIFTI2_CODE=ON USE_CIFTI_CODE=ON USE_FSL_CODE=ON: 100% tests passed, 0 tests failed out of 362.

Whitespace churn check: git diff master HEAD and git diff -w master HEAD both report 7 added lines.

[seanm]

Hmmm, not sure if I'm a fan of this one, especially since all the warnings as supposedly false positives. Setting the values to zero prevents tools like MSan and valgrind from detecting use of uninitialized data.

These days we have -ftrivial-auto-var-init=zero which people that are worried about this can use,.

[gdevenyi]

Setting the values to zero prevents tools like MSan and valgrind from detecting use of uninitialized data.

Good idea, I'd rather catch it with an address sanitization tool. This exists because the "warnings" pass of work I did happened before the sanitization pass.