ENH: Pass calloc its arguments in the documented order - #50
hjmjohnson merged 1 commit into
Conversation
41516c1 to
1554a18
Compare
1554a18 to
1d6a6ad
Compare
1d6a6ad to
fd38626
Compare
|
Rebased onto No regression test applies, and nothing observable exists to test. Message cleanupRemoved the Replaced the bare Also corrected a count: the body said "Eight further instances are left alone". There are ten on current five per io file, in Build and test
Whitespace churn check: |
fd38626 to
530211e
Compare
530211e to
edcc0ea
Compare
Four calls were written calloc(sizeof(char), n) rather than calloc(n, sizeof(char)), which gcc 14 and later diagnose as -Wcalloc-transposed-args. There is no behavior change: calloc multiplies its two arguments and sizeof(char) is 1 by definition, so the same number of zeroed bytes is returned either way and the overflow check is the same product. The warning is worth clearing anyway, because the diagnostic exists to catch the case where the element size is not 1, at which point the transposition produces a buffer of the wrong size. Leaving benign instances in the tree trains the reader to ignore the warning that will one day be real. Ten further instances are left alone deliberately. They are in nifti_findhdrname(), nifti_findimgname(), nifti_makehdrname() and nifti_makeimgname() in both io files, all of which an open pull request rewrites while keeping the transposed order. They will need fixing on top of that work; changing them here would only conflict with it.
edcc0ea to
0a33e97
Compare
affba1e
into
InsightSoftwareConsortium:master
3 participants
Four calls were written calloc(sizeof(char), n) rather than
calloc(n, sizeof(char)), which gcc 14 and later diagnose as
-Wcalloc-transposed-args.
There is no behaviour change: calloc multiplies its two arguments and
sizeof(char) is 1 by definition. The warning is worth clearing anyway,
because the diagnostic exists to catch the case where the element size is
not 1, at which point the transposition produces a buffer of the wrong
size. Leaving benign instances in the tree trains the reader to ignore
the warning that will one day be real.
Eight further instances are left alone deliberately: they are inside
nifti_findhdrname(), nifti_findimgname(), nifti_makehdrname() and
nifti_makeimgname(), which upstream PR #24 rewrites. That PR keeps the
transposed order, so they will need fixing on top of it; changing them
here would only create a conflict.
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