From b5faec44b68eee55bf06bed21943c6e5be2f2496 Mon Sep 17 00:00:00 2001 From: Jon Clayden Date: Mon, 21 Oct 2024 11:45:03 +0100 Subject: [PATCH 1/3] Include tinydir.h earlier; avoid R-unfriendly calls --- console/nii_dicom.cpp | 6 +++--- console/nii_dicom_batch.cpp | 10 ++++++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/console/nii_dicom.cpp b/console/nii_dicom.cpp index 78d4a311..7f30b55e 100644 --- a/console/nii_dicom.cpp +++ b/console/nii_dicom.cpp @@ -8008,10 +8008,10 @@ struct TDICOMdata readDICOMx(char *fname, struct TDCMprefs *prefs, struct TDTI4D mxIdx = i; } if (isVerbose > 1) - printf("slice %d is %gmm from isocenter\n", i, dx); + printMessage("slice %d is %gmm from isocenter\n", i, dx); } if (isVerbose > 1) - printf("slice %d is furthest (%gmm) from isocenter\n", mxIdx, mxDx); + printMessage("slice %d is furthest (%gmm) from isocenter\n", mxIdx, mxDx); // second pass: measure each slices scalar distance from furthest slice // ensure their are no repeated slice positions fidx *objects = (fidx *)malloc(sizeof(struct fidx) * numDimensionIndexValues); @@ -8019,7 +8019,7 @@ struct TDICOMdata readDICOMx(char *fname, struct TDCMprefs *prefs, struct TDTI4D for (int i = 0; i < numDimensionIndexValues; i++) { float dx = sqrt(pow(patientPosition1[i] - patientPosition1[mxIdx], 2) + pow(patientPosition2[i] - patientPosition2[mxIdx], 2) + pow(patientPosition3[i] - patientPosition3[mxIdx], 2)); if (isVerbose > 1) - printf("slice %d is %gmm from furthest slice\n", i, dx); + printMessage("slice %d is %gmm from furthest slice\n", i, dx); objects[i].value = dx; // sliceMM[i]; objects[i].index = i; diff --git a/console/nii_dicom_batch.cpp b/console/nii_dicom_batch.cpp index 0ce33de7..70b5dc26 100644 --- a/console/nii_dicom_batch.cpp +++ b/console/nii_dicom_batch.cpp @@ -26,9 +26,9 @@ #else #undef MiniZ #endif +#include "tinydir.h" #include "nifti1_io_core.h" #include "print.h" -#include "tinydir.h" #ifndef USING_R #include "nifti1.h" #endif @@ -3678,7 +3678,9 @@ int nii_createFilename(struct TDICOMdata dcm, char *niiFilename, struct TDCMopts strcat(bidsSubject, "1"); else strcat(bidsSubject, opts.bidsSubject); +#ifndef USING_R printf("%s<<<:::\n", bidsSubject); +#endif char bidsSession[kOptsStr] = "ses-"; if (strlen(opts.bidsSession) <= 0) strcat(bidsSession, "1"); @@ -7098,7 +7100,7 @@ void setBidsSiemens(struct TDICOMdata *d, int nConvert, int isVerbose, const cha strcat(suffixBIDS, modalityBIDS); } if ((isVerbose > 0) || (strlen(dataTypeBIDS) < 1)) - printf("::autoBids:Siemens CSAseqFname:'%s' pulseSeq:'%s' seqName:'%s'\n", + printMessage("::autoBids:Siemens CSAseqFname:'%s' pulseSeq:'%s' seqName:'%s'\n", seqDetails, d->pulseSequenceName, d->sequenceName); if (isDerived) strcpy(dataTypeBIDS, "derived"); @@ -8845,8 +8847,12 @@ int saveDcm2Nii(int nConvert, struct TDCMsort dcmSort[], struct TDICOMdata dcmLi // issue867 TODO sizeof(TDTI4D) = 10mb, this could be reduced if elements reduced from kMaxDTI4D TDTI4D *dti4Ds = (TDTI4D *)malloc(sizeof(TDTI4D)); if (dti4Ds == NULL) { +#ifdef USING_R + Rf_error("Failed to allocate memory for dti4Ds"); +#else perror("Failed to allocate memory for dti4Ds"); exit(EXIT_FAILURE); +#endif } // Copy the content from the original dti4D into dti4Ds *dti4Ds = *dti4D; From 6a3a4b49f4867bc7ee8d3b9194757a7e8ed1e94e Mon Sep 17 00:00:00 2001 From: Jon Clayden Date: Mon, 21 Oct 2024 11:45:57 +0100 Subject: [PATCH 2/3] Remove R special case now that stack usage is less --- console/nii_dicom.cpp | 41 ----------------------------------------- 1 file changed, 41 deletions(-) diff --git a/console/nii_dicom.cpp b/console/nii_dicom.cpp index 7f30b55e..48609a13 100644 --- a/console/nii_dicom.cpp +++ b/console/nii_dicom.cpp @@ -4211,31 +4211,6 @@ int fcmp(const void *a, const void *b) { return 0; } -#ifdef USING_R - -// True iff dcm1 sorts *before* dcm2 -bool compareTDCMdim(const TDCMdim &dcm1, const TDCMdim &dcm2) { - for (int i = MAX_NUMBER_OF_DIMENSIONS - 1; i >= 0; i--) { - if (dcm1.dimIdx[i] < dcm2.dimIdx[i]) - return true; - else if (dcm1.dimIdx[i] > dcm2.dimIdx[i]) - return false; - } - return false; -} // compareTDCMdim() - -bool compareTDCMdimRev(const TDCMdim &dcm1, const TDCMdim &dcm2) { - for (int i = 0; i < MAX_NUMBER_OF_DIMENSIONS; i++) { - if (dcm1.dimIdx[i] < dcm2.dimIdx[i]) - return true; - else if (dcm1.dimIdx[i] > dcm2.dimIdx[i]) - return false; - } - return false; -} // compareTDCMdimRev() - -#else - int compareTDCMdim(void const *item1, void const *item2) { struct TDCMdim const *dcm1 = (const struct TDCMdim *)item1; struct TDCMdim const *dcm2 = (const struct TDCMdim *)item2; @@ -4261,8 +4236,6 @@ int compareTDCMdimRev(void const *item1, void const *item2) { return 0; } // compareTDCMdimRev() -#endif // USING_R - struct TDICOMdata readDICOMx(char *fname, struct TDCMprefs *prefs, struct TDTI4D *dti4D) { // struct TDICOMdata readDICOMv(char * fname, int isVerbose, int compressFlag, struct TDTI4D *dti4D) { int isVerbose = prefs->isVerbose; @@ -4845,13 +4818,8 @@ struct TDICOMdata readDICOMx(char *fname, struct TDCMprefs *prefs, struct TDTI4D // philDTI[i].V[0] = -1; // array for storing DimensionIndexValues int numDimensionIndexValues = 0; -#ifdef USING_R - // Allocating a large array on the stack, as below, vexes valgrind and may cause overflow - std::vector dcmDim(kMaxSlice2D); -#else //don't use stack! TDCMdim dcmDim[kMaxSlice2D]; TDCMdim *dcmDim = (TDCMdim *)malloc(kMaxSlice2D * sizeof(TDCMdim)); -#endif for (int i = 0; i < kMaxSlice2D; i++) { dcmDim[i].diskPos = i; for (int j = 0; j < MAX_NUMBER_OF_DIMENSIONS; j++) @@ -8104,17 +8072,10 @@ struct TDICOMdata readDICOMx(char *fname, struct TDCMprefs *prefs, struct TDTI4D if ((isKludgeIssue533) && (numDimensionIndexValues > 1)) printWarning("Guessing temporal order for Philips enhanced DICOM ASL (issue 532).\n"); // sort dimensions -#ifdef USING_R - if (stackPositionItem < maxVariableItem) - std::sort(dcmDim.begin(), dcmDim.begin() + numberOfFrames, compareTDCMdim); - else - std::sort(dcmDim.begin(), dcmDim.begin() + numberOfFrames, compareTDCMdimRev); -#else if (stackPositionItem < maxVariableItem) qsort(dcmDim, numberOfFrames, sizeof(struct TDCMdim), compareTDCMdim); else qsort(dcmDim, numberOfFrames, sizeof(struct TDCMdim), compareTDCMdimRev); -#endif // for (int i = 0; i < numberOfFrames; i++) // printf("i %d diskPos= %d dimIdx= %d %d %d %d TE= %g\n", i, dcmDim[i].diskPos, dcmDim[i].dimIdx[0], dcmDim[i].dimIdx[1], dcmDim[i].dimIdx[2], dcmDim[i].dimIdx[3], dti4D->TE[i]); for (int i = 0; i < numberOfFrames; i++) { @@ -8470,9 +8431,7 @@ struct TDICOMdata readDICOMx(char *fname, struct TDCMprefs *prefs, struct TDTI4D // printf("%g\t%g\t%s\n", d.intenIntercept, d.intenScale, fname); if ((d.isLocalizer) && (strstr(d.seriesDescription, "b1map"))) // issue751 b1map uses same base as scout d.isLocalizer = false; - #ifndef USING_R free(dcmDim); - #endif return d; } // readDICOMx() From cf9f98166f1e992a5486b0bb1fa9491032f497d9 Mon Sep 17 00:00:00 2001 From: Jon Clayden Date: Mon, 21 Oct 2024 11:50:21 +0100 Subject: [PATCH 3/3] Small fixes: out-of-bounds or NULL accesses, memory leaks, uninitialised decision variables --- console/nii_dicom.cpp | 11 +++++++++-- console/nii_dicom_batch.cpp | 26 +++++++++++++++++++++----- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/console/nii_dicom.cpp b/console/nii_dicom.cpp index 48609a13..7aacae08 100644 --- a/console/nii_dicom.cpp +++ b/console/nii_dicom.cpp @@ -3963,7 +3963,7 @@ void clear_volume(struct TVolumeDiffusion *ptvd) { ptvd->_dtiV[0] = -1; for (int i = 1; i < 4; ++i) ptvd->_dtiV[i] = 2; - for (int i = 1; i < 6; ++i) + for (int i = 0; i < 6; ++i) ptvd->_symBMatrix[i] = NAN; // numDti = 0; } // clear_volume() @@ -4254,6 +4254,13 @@ struct TDICOMdata readDICOMx(char *fname, struct TDCMprefs *prefs, struct TDTI4D dti4D->frameReferenceTime[0] = -1; // dti4D->fragmentOffset[0] = -1; dti4D->intenScale[0] = 0.0; +#ifdef USING_R + // Ensure dti4D fields are initialised, as in nii_readParRec() + for (int i = 0; i < kMaxDTI4D; i++) { + dti4D->S[i].V[0] = -1.0; + dti4D->TE[i] = -1.0; + } +#endif d.deID_CS_n = 0; struct TVolumeDiffusion volDiffusion = initTVolumeDiffusion(&d, dti4D); struct stat s; @@ -7432,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); d.CSA.tablePos[3] = v[3] - tableDeltaGE; d.CSA.tablePos[0] = 1.0; diff --git a/console/nii_dicom_batch.cpp b/console/nii_dicom_batch.cpp index 70b5dc26..04baddc0 100644 --- a/console/nii_dicom_batch.cpp +++ b/console/nii_dicom_batch.cpp @@ -1055,13 +1055,17 @@ int geProtocolBlock(const char *filename, int geOffset, int geLength, int isVerb #ifdef myDisableMiniZ ret = inflate(&s, Z_SYNC_FLUSH); if (ret != Z_STREAM_END) { + free(pCmp); free(pUnCmp); + inflateEnd(&s); return EXIT_FAILURE; } #else ret = mz_inflate(&s, MZ_SYNC_FLUSH); if (ret != MZ_STREAM_END) { + free(pCmp); free(pUnCmp); + inflateEnd(&s); return EXIT_FAILURE; } #endif @@ -1108,7 +1112,9 @@ int geProtocolBlock(const char *filename, int geOffset, int geLength, int isVerb printMessage(" ViewOrder %d SliceOrder %d\n", *viewOrder, *sliceOrder); printMessage("%s\n", pUnCmp); } + free(pCmp); free(pUnCmp); + inflateEnd(&s); return EXIT_SUCCESS; } #endif // myReadGeProtocolBlock() @@ -1629,7 +1635,7 @@ tse3d: T2*/ int iterations = 0; // note, some vendors write 'OSEM3D-OP-PSFi10s16' others 'OP-OSEM4i21s' // order matters `OP-OSEM4i21s` should have i=4 NOT i=21 - bool sEnd = d.reconstructionMethod[strlen(d.reconstructionMethod) - 1] == 's'; + bool sEnd = strlen(d.reconstructionMethod) == 0 ? false : (d.reconstructionMethod[strlen(d.reconstructionMethod) - 1] == 's'); for (int i = 1; i < 33; i++) { char stri[12]; if (sEnd) @@ -5558,6 +5564,10 @@ int nii_saveNII(char *niiFilename, struct nifti_1_header hdr, unsigned char *im, return EXIT_FAILURE; image->data = (void *)im; images->append(image, name); + // A copy of "image" has been made, so free all aspects of it except the data (which is owned by the caller) + free(image->fname); + free(image->iname); + nifti_free_extensions(image); free(image); return EXIT_SUCCESS; } @@ -6430,6 +6440,7 @@ int nii_saveCrop(char *niiFilename, struct nifti_1_header hdr, unsigned char *im strcat(niiFilenameCrop, "_Crop"); const int returnCode = nii_saveNII3D(niiFilenameCrop, hdrX, imX, opts, d); free(imX); + free(sliceSums); return returnCode; } // nii_saveCrop() @@ -6769,16 +6780,20 @@ void readSoftwareVersionsGE(char softwareVersionsGE[], int verbose, char geVersi sepStart += 1; } } - len = 11; // RX27.0_R02_ + // Default: start from the beginning + if (ismatched == false) { + sepStart = softwareVersionsGE; + } + len = 12; // RX27.0_R02_, plus nul terminator char *versionString = (char *)malloc(sizeof(char) * len); versionString[len - 1] = 0; - memcpy(versionString, sepStart, len); + memcpy(versionString, sepStart, len-1); char c1, c2, c3, c4; // RX27.0_R02_ or MR29.1_EA_2 - sscanf(versionString, "%c%c%d.%d_%c%c%d", &c1, &c2, geMajorVersionInt, geMinorVersionInt, &c3, &c4, geReleaseVersionInt); + int fields = sscanf(versionString, "%c%c%d.%d_%c%c%d", &c1, &c2, geMajorVersionInt, geMinorVersionInt, &c3, &c4, geReleaseVersionInt); memcpy(geVersionPrefix, &c1, 1); memcpy(geVersionPrefix + 1, &c2, 1); - if ((c3 == 'E') && (c4 == 'A')) { + if ((fields >= 6) && (c3 == 'E') && (c4 == 'A')) { *geReleaseVersionInt = 0; } free(versionString); @@ -9509,6 +9524,7 @@ int searchDirRenameDICOM(char *path, int maxDepth, int depth, struct TDCMopts *o } } } + tinydir_close(&dir); return count; }