BUG: Report XML read errors instead of treating them as end of input - #42
hjmjohnson merged 2 commits into
Conversation
a257e7e to
febf35e
Compare
febf35e to
2dfd021
Compare
2dfd021 to
f336f41
Compare
|
Rebased onto Message and comment cleanupRemoved the AI Tests and red proofTwo tests in
The assertion is on the diagnostic, not the exit status: the tool exits 1 both before and after the fix, so exit status proves nothing here. With The wrong diagnostic is exactly the symptom — expat was handed a zero-length final buffer and blamed the document for the I/O error. With the fix: Not coveredThe |
9428095 to
bbf051b
Compare
axml_read_file()'s read loop:
blen = fread(buf, 1, bsize, fp);
...
done = blen < (unsigned)bsize;
A short read is the loop's only stopping condition, so an I/O failure is
indistinguishable from reaching the end of the file: the parse stops
early and the caller is handed whatever was parsed so far, with no
indication that the rest of the document was never read. ferror() is now
checked and the failure reported.
The loop also called fread() once more after a file ending exactly on a
buffer boundary; testing feof() as part of the stopping condition removes
that read, which the clang static analyzer flags as a read at EOF.
A directory opens but cannot be read, so afni_xml_tool on cifti/testdata reaches the ferror() branch; the test pins the diagnostic rather than the exit status, which was already non-zero before the fix. A second test parses a real document and forbids that diagnostic, so a change that reported a read error unconditionally would not pass.
bbf051b to
9939264
Compare
283c5d4
into
InsightSoftwareConsortium:master
axml_read_file()'s read loop:
A short read is the loop's only stopping condition, so an I/O failure is
indistinguishable from reaching the end of the file: the parse stops
early and the caller is handed whatever was parsed so far, with no
indication that the rest of the document was never read. ferror() is now
checked and the failure reported.
The loop also called fread() once more after a file ending exactly on a
buffer boundary; testing feof() as part of the stopping condition removes
that read, which the clang static analyzer flags as a read at EOF.
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