BUG: Stop casting away const in fslio and cifti - #45
Conversation
124b775 to
2558482
Compare
2558482 to
5ae7bd3
Compare
5ae7bd3 to
a6f0957
Compare
|
Rebased onto No regression test applies: every hunk is type-qualifier-only and generates identical code. Details below. Message cleanupRemoved the Three fixes to the diff
Why there is no red-green testEvery hunk changes only type qualifiers on locals, a static function's return type, and the removal of casts. No expression's value, no control flow, and no emitted instruction changes:
A ctest cannot distinguish the two trees. The class is BUILD/COSMETIC: nine Build and test
Whitespace churn check: |
a6f0957 to
ab8da65
Compare
Nine -Wcast-qual warnings. Casting away const is how a read-only
contract turns into a write to memory the caller thought was safe, so
each was looked at rather than silenced.
fslio.c, FslGetFileType2()
mutablefslio = (FSLIO *)fslio; /* dodgy and will generate warnings */
mutablefslio->niftiptr->nifti_type = ...;
The comment is right that it looks dodgy, but the cast was never
needed. `const FSLIO * fslio` makes the *member* niftiptr const --
its type is `nifti_image * const` -- while what it points at stays
fully mutable. The assignment is legal as written, so the cast and
the local both go.
fslio.c FslWriteVolumes(), cifti axio_num_tokens(), text_to_i64(),
text_to_f64()
Pointers cast to char * and then only read: walked, indexed, or
handed to strtoll()/strtod(), which take const char *. Declared
const, casts removed.
cifti strip_whitespace()
Returned `(char *)str` on its early-exit paths, handing the caller a
writable pointer to the const string it passed in. The function is
static, so its return type is nobody else's business; it now returns
const char *.
ab8da65 to
23b6691
Compare
0a9aa9c
into
InsightSoftwareConsortium:master
Nine -Wcast-qual warnings. Casting away const is how a read-only
contract turns into a write to memory the caller thought was safe, so
each was looked at rather than silenced.
fslio.c, FslGetFileType2()
mutablefslio = (FSLIO )fslio; / dodgy and will generate warnings */
mutablefslio->niftiptr->nifti_type = ...;
fslio.c FslWriteVolumes(), cifti axio_num_tokens(), text_to_i64(),
text_to_f64()
Pointers cast to char * and then only read: walked, indexed, or
handed to strtoll()/strtod(), which take const char *. Declared
const, casts removed.
cifti strip_whitespace()
Returned
(char *)stron its early-exit paths, handing the caller awritable pointer to the const string it passed in. The function is
static, so its return type is nobody else's business; it now returns
const char *.
Interface impact: none. On the union of all these changes, configured with
USE_FSL_CODE=ONandUSE_CIFTI_CODE=ON: all 448 exported symbols acrosslibniftiio,libnifti2,libznz,libfslio,libnifticdfandlibciftiare identical tomasterundernm -D --defined-only, and all ten installed headers are identical undergcc -E -P. Undergcc -dM -Eone macro definition differs, intentionally and only in text: #61 makesFSL_RADIOLOGICALread(-1)so it is safe inside an expression. Its value is still-1, checked by compiling against each installedfslio.hand printing it.Verification. This branch: builds with gcc 16.1.1,
ctestunchanged frommaster(2 of 345 fail onmasteritself 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,ctest345/345 under each, and the whole suite under valgrind memcheck with--trace-children=yesgives 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 infslio.cfor #22, 2 innifti2_io.cand 2 innifti_tester001.cfor #24) and 8-Wcalloc-transposed-argsinnifti_findhdrnameandnifti_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
masterand 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