Changes from divest 1.1 #876
Conversation
| dti4D->S[i].V[0] = -1.0; | ||
| dti4D->TE[i] = -1.0; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
This can be unfenced if it's correct. There is branching elsewhere that depends on an otherwise uninitialised dti4D->S[0].V[0], which valgrind flags up, and this seemed like a reasonable place and reasonable values for initialisation based on the comparable nii_readParRec().
| @@ -7464,7 +7439,7 @@ struct TDICOMdata readDICOMx(char *fname, struct TDCMprefs *prefs, struct TDTI4D | |||
| // LO array of floats stored in LONG STRING! | |||
| // [960.5\400\17.9108\0\-9999\-9999] | |||
| // we want 3rd value, e.g. 17.9: | |||
| float v[5]; | |||
| float v[6]; | |||
| dcmMultiFloat(lLength, (char *)&buffer[lPos], 5, v); | |||
There was a problem hiding this comment.
dcmMultiFloat() modifies elements 1 to n of its argument, so the array needs to have length n+1.
| #include "nifti1_io_core.h" | ||
| #include "print.h" | ||
| #include "tinydir.h" |
There was a problem hiding this comment.
This reordering avoids the DT_UNKNOWN symbol clash.
| memcpy(geVersionPrefix, &c1, 1); | ||
| memcpy(geVersionPrefix + 1, &c2, 1); | ||
| if ((c3 == 'E') && (c4 == 'A')) { | ||
| if ((fields >= 6) && (c3 == 'E') && (c4 == 'A')) { |
There was a problem hiding this comment.
The version string 14.0_M4_062 appears in the QA battery, and doesn't match the sscanf() call, so c3 and c4 end up uninitialised. This is defensive programming to check that enough fields were read.
Changes to the core codebase incorporated into
divest1.1, both for R compatibility and to address various runtime issues identified byclangASan/UBSan andvalgrind. One of the latter fixes is fenced usingUSING_R, as I'm not totally confident that it's the right change in the right place.