From: regrain Date: Tue, 16 Nov 2004 16:20:22 +0000 (+0000) Subject: * FIX : now, the DocEntries are all deleted in the gdcmElementSet. X-Git-Tag: Version1.0.bp~592 X-Git-Url: https://git.creatis.insa-lyon.fr/pubgit/?a=commitdiff_plain;h=bac2910466f502dec4c1842527aa81a8f83f04b8;p=gdcm.git * FIX : now, the DocEntries are all deleted in the gdcmElementSet. Two problems appear when doing it : - with the gdcmFile : when the GetImageData method is called, the pixels are stored in the gdcmPixelConvert, but a gdcmBinEntry link to these datas (pixels). And each structure destruct the datas when it's destructed. So we have two destructions for the same datas. To solve it, a flag is added in the gdcmBinEntry to indicate if the BinEntry owns the datas or not. If it doesn't own datas, then they will not destroyed by the gdcmBinEntry. - with the gdcmDicomDir : the sequences (gdcmSQItem) contain DocEntry elements. The DicomDir* (DicomDirPatient, etc.) inherit from SQItem. Thus destruct the DicomDir* elements and the TagHT of the ElementSet create a double destruction of the same DocEntry's. So, to solve it, the TagHT is simply cleared and the DicomDir* elements are destroyed. * TODO : add an entry concerning memory leaks in the DicomDir -- BeNours --- diff --git a/ChangeLog b/ChangeLog index ce610cb8..85e4a7e9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,22 @@ 2004-11-15 Benoit Regrain - * FIX : src/gdcmDocument.cxx Remove obvious code in the destructor + * FIX : now, the DocEntries are all deleted in the gdcmElementSet. + Two problems appear when doing it : + - with the gdcmFile : when the GetImageData method is called, the pixels + are stored in the gdcmPixelConvert, but a gdcmBinEntry link to these + datas (pixels). And each structure destruct the datas when it's + destructed. So we have two destructions for the same datas. To solve it, + a flag is added in the gdcmBinEntry to indicate if the BinEntry owns the + datas or not. If it doesn't own datas, then they will not destroyed by + the gdcmBinEntry. + - with the gdcmDicomDir : the sequences (gdcmSQItem) contain DocEntry + elements. The DicomDir* (DicomDirPatient, etc.) inherit from SQItem. + Thus destruct the DicomDir* elements and the TagHT of the ElementSet + create a double destruction of the same DocEntry's. So, to solve it, + the TagHT is simply cleared and the DicomDir* elements are destroyed. + * TODO : add an entry concerning memory leaks in the DicomDir + +2004-11-15 Benoit Regrain + * FIX : src/gdcmDocument.cxx Remove obvious code in the destructor * FIX : src/gdcmPixelConvert : Set to NULL the deleted structures in the squeeze method diff --git a/TODO b/TODO index 271ae89f..8b1b2f75 100644 --- a/TODO +++ b/TODO @@ -72,6 +72,17 @@ Bah, comme Win32 pose encore pb: et si on prenait: radical + 147.144.143.155 + IP + time() +----------------------------------------------------------------------------- +Description: gdcmDicomDir and SQItem creation +Date: 2004 Nov 16 +Attributed: +Details: DicomDir creates some SQItem (by new). After that, it creates + the corresponding DicomDirPatient, etc. using the content of the SQItem + (the content is composed with some DocEntry's that can't be destroyed). + So, if the SQItem is deleted, then it's content is deleted to. But the + DicomDirPatient, etc. use the content of the SQItem. Then, the SQItem can't + be deleted, and when have memory leaks + ----------------------------------------------------------------------------- Description: [BUG] Better handling of unfound Dicom dictionary. When gdcm doesn't find the Dicom dictionary (because it's @@ -336,3 +347,4 @@ Comments: a.write(output); } ----------------------------------------------------------------------------- + diff --git a/src/gdcmBinEntry.cxx b/src/gdcmBinEntry.cxx index 22dbae3c..d0301c5e 100644 --- a/src/gdcmBinEntry.cxx +++ b/src/gdcmBinEntry.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmBinEntry.cxx,v $ Language: C++ - Date: $Date: 2004/11/15 16:12:30 $ - Version: $Revision: 1.36 $ + Date: $Date: 2004/11/16 16:20:23 $ + Version: $Revision: 1.37 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -32,6 +32,7 @@ namespace gdcm BinEntry::BinEntry(DictEntry* e) : ValEntry(e) { BinArea = 0; + SelfArea = true; } /** @@ -49,6 +50,7 @@ BinEntry::BinEntry(DocEntry* e) : ValEntry(e->GetDictEntry()) //SQDepthLevel = e->GetDepthLevel(); BinArea = 0; // let's be carefull ! + SelfArea = true; } /** @@ -56,7 +58,7 @@ BinEntry::BinEntry(DocEntry* e) : ValEntry(e->GetDictEntry()) */ BinEntry::~BinEntry() { - if (BinArea) + if (BinArea && SelfArea) { delete[] BinArea; BinArea = 0; // let's be carefull ! @@ -125,11 +127,13 @@ void BinEntry::Write(std::ofstream* fp, FileType filetype) /// \brief Sets the value (non string) of the current Dicom Header Entry -void BinEntry::SetBinArea( uint8_t* area ) +void BinEntry::SetBinArea( uint8_t* area, bool self ) { - if (BinArea) + if (BinArea && SelfArea) delete[] BinArea; + BinArea = area; + SelfArea=self; } //----------------------------------------------------------------------------- diff --git a/src/gdcmBinEntry.h b/src/gdcmBinEntry.h index 285d9418..f79e3e34 100644 --- a/src/gdcmBinEntry.h +++ b/src/gdcmBinEntry.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmBinEntry.h,v $ Language: C++ - Date: $Date: 2004/10/22 03:05:40 $ - Version: $Revision: 1.24 $ + Date: $Date: 2004/11/16 16:20:23 $ + Version: $Revision: 1.25 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -47,12 +47,13 @@ public: /// \brief Returns the area value of the current Dicom Header Entry /// when it's not string-translatable (e.g : a LUT table) uint8_t* GetBinArea() { return BinArea; } - void SetBinArea( uint8_t* area ); - + void SetBinArea( uint8_t* area, bool self = true ); + private: /// \brief unsecure memory area to hold 'non string' values /// (ie : Lookup Tables, overlays, icons) uint8_t* BinArea; + bool SelfArea; }; } // end namespace gdcm //----------------------------------------------------------------------------- diff --git a/src/gdcmDicomDir.cxx b/src/gdcmDicomDir.cxx index 7b5857ae..28160d5c 100644 --- a/src/gdcmDicomDir.cxx +++ b/src/gdcmDicomDir.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmDicomDir.cxx,v $ Language: C++ - Date: $Date: 2004/11/16 10:25:52 $ - Version: $Revision: 1.80 $ + Date: $Date: 2004/11/16 16:20:23 $ + Version: $Revision: 1.81 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -121,7 +121,8 @@ DicomDir::DicomDir(std::string const & fileName, bool parseDir ): /// \todo FIXME : what do we do when the parsed file IS NOT a /// DICOMDIR file ? } - CreateDicomDir(); + else + CreateDicomDir(); } } @@ -133,6 +134,8 @@ DicomDir::~DicomDir() SetStartMethod(NULL); SetProgressMethod(NULL); SetEndMethod(NULL); + + TagHT.clear(); for(ListDicomDirPatient::iterator cc = Patients.begin(); cc!= Patients.end(); ++cc) @@ -401,9 +404,6 @@ void DicomDir::CreateDicomDirChainedList(std::string const & path) VectDocument list; Header *header; - TagHT.clear(); - Patients.clear(); - for( DirList::iterator it = fileList.begin(); it != fileList.end(); ++it ) @@ -532,7 +532,7 @@ DicomDirPatient * DicomDir::NewPatient() DicomDirPatient *p = new DicomDirPatient(s, &TagHT); Patients.push_front( p ); - return p; + return p; } /** @@ -553,6 +553,7 @@ void DicomDir::SetElement(std::string const & path, DicomDirType type, ValEntry *entry; std::string val; SQItem *si = new SQItem(0); // all the items will be at level 1 + switch( type ) { case GDCM_DICOMDIR_IMAGE: @@ -923,6 +924,9 @@ void DicomDir::AddDicomDirSerieToEnd(SQItem *s) */ void DicomDir::SetElements(std::string const & path, VectDocument const &list) { + TagHT.clear(); + Patients.clear(); + std::string patPrevName = "", patPrevID = ""; std::string studPrevInstanceUID = "", studPrevID = ""; std::string serPrevInstanceUID = "", serPrevID = ""; diff --git a/src/gdcmDocument.cxx b/src/gdcmDocument.cxx index 226dc896..ea6ee6e7 100644 --- a/src/gdcmDocument.cxx +++ b/src/gdcmDocument.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmDocument.cxx,v $ Language: C++ - Date: $Date: 2004/11/16 14:48:19 $ - Version: $Revision: 1.128 $ + Date: $Date: 2004/11/16 16:20:23 $ + Version: $Revision: 1.129 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -951,10 +951,10 @@ bool Document::SetEntryByNumber(uint8_t*content, int lgth, //content = content + '\0'; // fing a trick to enlarge a binary field? } */ - BinEntry* a = (BinEntry *)TagHT[key]; - a->SetBinArea(content); - a->SetLength(lgth); - a->SetValue(GDCM_BINLOADED); + BinEntry* entry = (BinEntry *)TagHT[key]; + entry->SetBinArea(content); + entry->SetLength(lgth); + entry->SetValue(GDCM_BINLOADED); return true; } @@ -1092,7 +1092,7 @@ void Document::LoadEntryBinArea(BinEntry* element) return; } - element->SetBinArea((uint8_t*)a); + element->SetBinArea(a); } /** diff --git a/src/gdcmElementSet.cxx b/src/gdcmElementSet.cxx index cfd3aad0..b485261b 100644 --- a/src/gdcmElementSet.cxx +++ b/src/gdcmElementSet.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmElementSet.cxx,v $ Language: C++ - Date: $Date: 2004/11/16 13:20:34 $ - Version: $Revision: 1.29 $ + Date: $Date: 2004/11/16 16:20:23 $ + Version: $Revision: 1.30 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -44,15 +44,11 @@ ElementSet::ElementSet(int depthLevel) */ ElementSet::~ElementSet() { - for(TagDocEntryHT::iterator cc = TagHT.begin();cc != TagHT.end(); ++cc) + for(TagDocEntryHT::iterator cc = TagHT.begin();cc != TagHT.end(); ++cc) { - DocEntry* entryToDelete = cc->second; - if ( entryToDelete ) + if ( cc->second ) { - // FIXME - // Because the gdcmFile links the datas of gdcmPixelConvert with the - // data in a binArea, these datas are deleted 2 times... very bad... - //delete entryToDelete; + delete cc->second; } } TagHT.clear(); diff --git a/src/gdcmFile.cxx b/src/gdcmFile.cxx index 2cf2c255..837df465 100644 --- a/src/gdcmFile.cxx +++ b/src/gdcmFile.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmFile.cxx,v $ Language: C++ - Date: $Date: 2004/11/16 05:03:35 $ - Version: $Revision: 1.156 $ + Date: $Date: 2004/11/16 16:20:23 $ + Version: $Revision: 1.157 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -19,6 +19,7 @@ #include "gdcmFile.h" #include "gdcmDebug.h" #include "gdcmUtil.h" +#include "gdcmBinEntry.h" #include namespace gdcm @@ -87,7 +88,7 @@ void File::Initialise() ImageDataSize = ImageDataSizeRaw; } - PixelConverter = new PixelConvert; //LEAK ! + PixelConverter = new PixelConvert; PixelConverter->GrabInformationsFromHeader( HeaderInternal ); } SaveInitialValues(); @@ -136,7 +137,7 @@ void File::SaveInitialValues() InitialRedLUTData = 0; InitialGreenLUTData = 0; InitialBlueLUTData = 0; - + if ( HeaderInternal->IsReadable() ) { // the following values *may* be modified @@ -211,28 +212,29 @@ void File::RestoreInitialValues() */ void File::DeleteInitialValues() { - // InitialLutDescriptors and InitialLutData // will have to be deleted if the don't belong any longer // to the Header H table when the header is deleted... - if ( InitialRedLUTDescr ) +// FIXME +// We don't know if the InitialLutData are still in the header or not ! +/* if ( InitialRedLUTDescr ) delete InitialRedLUTDescr; if ( InitialGreenLUTDescr ) delete InitialGreenLUTDescr; - if ( InitialBlueLUTDescr ) - delete InitialBlueLUTDescr; + if ( InitialBlueLUTDescr ) + delete InitialBlueLUTDescr; - if ( InitialRedLUTData ) + if ( InitialRedLUTData ) delete InitialRedLUTData; - if ( InitialGreenLUTData != NULL) + if ( InitialGreenLUTData ) delete InitialGreenLUTData; - if ( InitialBlueLUTData != NULL) - delete InitialBlueLUTData; + if ( InitialBlueLUTData ) + delete InitialBlueLUTData;*/ } //----------------------------------------------------------------------------- @@ -377,12 +379,7 @@ uint8_t* File::GetImageData() } // We say the value *is* loaded. - GetHeader()->SetEntryByNumber( GDCM_BINLOADED, - GetHeader()->GetGrPixel(), GetHeader()->GetNumPixel()); - - // Will be 7fe0, 0010 in standard case - GetHeader()->SetEntryBinAreaByNumber( pixelData, - GetHeader()->GetGrPixel(), GetHeader()->GetNumPixel()); + SetPixelData(pixelData); // END PIXELCONVERT CLEANME return pixelData; @@ -429,9 +426,9 @@ size_t File::GetImageDataIntoVector (void* destination, size_t maxSize) "than caller's expected MaxSize"); return 0; } - memmove( destination, - (void*)PixelConverter->GetRGB(), - PixelConverter->GetRGBSize() ); + memcpy( destination, + (void*)PixelConverter->GetRGB(), + PixelConverter->GetRGBSize() ); return PixelConverter->GetRGBSize(); } @@ -442,9 +439,9 @@ size_t File::GetImageDataIntoVector (void* destination, size_t maxSize) "than caller's expected MaxSize"); return 0; } - memmove( destination, - (void*)PixelConverter->GetDecompressed(), - PixelConverter->GetDecompressedSize() ); + memcpy( destination, + (void*)PixelConverter->GetDecompressed(), + PixelConverter->GetDecompressedSize() ); return PixelConverter->GetDecompressedSize(); } @@ -492,12 +489,13 @@ uint8_t* File::GetImageDataRaw () } // We say the value *is* loaded. - GetHeader()->SetEntryByNumber( GDCM_BINLOADED, +/* GetHeader()->SetEntryByNumber( GDCM_BINLOADED, GetHeader()->GetGrPixel(), GetHeader()->GetNumPixel()); // will be 7fe0, 0010 in standard cases GetHeader()->SetEntryBinAreaByNumber( decompressed, - GetHeader()->GetGrPixel(), GetHeader()->GetNumPixel()); + GetHeader()->GetGrPixel(), GetHeader()->GetNumPixel());*/ + SetPixelData(decompressed); PixelRead = 1; // PixelRaw // END PIXELCONVERT CLEANME @@ -719,5 +717,23 @@ uint8_t* File::GetLutRGBA() return PixelConverter->GetLutRGBA(); } +//----------------------------------------------------------------------------- +// Private +void File::SetPixelData(uint8_t* data) +{ + GetHeader()->SetEntryByNumber( GDCM_BINLOADED, + GetHeader()->GetGrPixel(), GetHeader()->GetNumPixel()); + + // Will be 7fe0, 0010 in standard case + DocEntry* currentEntry = GetHeader()->GetDocEntryByNumber(GetHeader()->GetGrPixel(), GetHeader()->GetNumPixel()); + if ( currentEntry ) + { + if ( BinEntry* binEntry = dynamic_cast(currentEntry) ) + // Flag is to false because datas are kept in the gdcmPixelConvert + binEntry->SetBinArea( data, false ); + } +} + +//----------------------------------------------------------------------------- } // end namespace gdcm diff --git a/src/gdcmFile.h b/src/gdcmFile.h index 8491fb52..7c08ed37 100644 --- a/src/gdcmFile.h +++ b/src/gdcmFile.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmFile.h,v $ Language: C++ - Date: $Date: 2004/10/28 03:10:58 $ - Version: $Revision: 1.68 $ + Date: $Date: 2004/11/16 16:20:23 $ + Version: $Revision: 1.69 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -87,6 +87,8 @@ private: int ComputeDecompressedPixelDataSizeFromHeader(); private: + void SetPixelData(uint8_t* data); + // members variables: /// Header to use to load the file