Skip to content

ENH: Give the cifti tools' gopt internal linkage - #36

Merged
hjmjohnson merged 1 commit into
InsightSoftwareConsortium:masterfrom
gdevenyi:pr/fix-missing-variable-declarations
Sep 22, 2026
Merged

hjmjohnson merged 1 commit into
InsightSoftwareConsortium:masterfrom
gdevenyi:pr/fix-missing-variable-declarations

Conversation

@gdevenyi

@gdevenyi gdevenyi commented Aug 15, 2026 •

Copy link
Copy Markdown

afni_xml_tool.c and cifti_tool.c each define a file-scope opts_t gopt
with external linkage, so the two share a symbol name and neither
declares it in a header. Each is used only inside its own translation
unit, always by address.

Both files build executables rather than libraries, so static removes
nothing from any shared library's exported symbols.

Clears both -Wmissing-variable-declarations warnings.


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

Configuration and documentation

Defects

Warning and check classes

This was referenced Aug 15, 2026
@hjmjohnson

Copy link
Copy Markdown
Member

Rebased onto current master (b4876bf) and pushed as eca6db7. The 28 commits that were below this one in the branch are all on master now under rewritten SHAs, so the PR is down to its own single commit and the diff is the two static keywords. No test added: static on a file-scope object with no other translation unit referencing it is a linkage change with no observable runtime behavior.

Exported-symbol baseline: no update needed, and I checked. gopt is defined in cifti/afni_xml_tool.c and cifti/cifti_tool.c, both of which build executables. cmake/collect_exported_symbols.sh walks only the built shared libraries, and cmake/exported_symbols_linux.txt contains no gopt line. The baseline job stays green with the baseline unchanged.

What changed in this update
  • Rebased onto master; applied cleanly, no conflicts.
  • Commit message: dropped the Co-Authored-By: naming an AI tool and the Claude-Session: URL. Nothing else in the message needed changing.
  • Source diff is unchanged from before the rebase.
Build and test

Release, NIFTI_BUILD_APPLICATIONS=ON USE_NIFTI2_CODE=ON USE_CIFTI_CODE=ON USE_FSL_CODE=ON, Ninja, macOS/AppleClang: build clean, 100% tests passed, 0 tests failed out of 362.

afni_xml_tool.c and cifti_tool.c each define a file-scope `opts_t gopt`
with external linkage, so the two share a symbol name and neither
declares it in a header.  Each is used only inside its own translation
unit, always by address.

Both files build executables rather than libraries, so `static` removes
nothing from any shared library's exported symbols.

Clears both -Wmissing-variable-declarations warnings.
@hjmjohnson
hjmjohnson force-pushed the pr/fix-missing-variable-declarations branch from eca6db7 to 3364257 Compare September 22, 2026 15:53
@hjmjohnson
hjmjohnson merged commit a2d3b5c into InsightSoftwareConsortium:master Sep 22, 2026
25 checks passed
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