From cf66ed219aec63038f62f96f5fbdc4e37868bb0d Mon Sep 17 00:00:00 2001 From: frog Date: Fri, 18 Jun 2004 00:11:44 +0000 Subject: [PATCH] * Valgrind note: after Mathieu Malaterre teached me how to read the valgrind FAQ ;-] (see http://valgrind.kde.org/faq.html), I learned that: Using gcc, you can force the STL to use malloc and to free memory as soon as possible by globally disabling memory caching. With 3.2.2 and later, you should export the environment variable GLIBCPP_FORCE_NEW before running your program. By setting GLIBCPP_FORCE_NEW, STL related memory leak messages of gdcm simply vanish (it is still not clear to me, whether this means that STL std::string leaks or if valgrind believes it leaks...). * Fixing of SegFault of Test/makeDicomDir (as shown by ctest or by running bin/gdcmTests makeDicomDir): - src/gdcmDicomDir.cxx: dynamic casting used + clean up. - Test/makeDicomDir.cxx now properly traps empty lists and returns with 1. NOW, makeDicomDir cleanly fails (in ctest terminology) instead of SegFaulting (I drowned in DicomDir related code when trying to understand why the list is empty...). * src/gdcmDocument.h: first BSD license header try. * Doc/License.txt added. --- Frog --- ChangeLog | 28 +++++++++-- Doc/License.txt | 40 ++++++++++++++++ src/gdcmDicomDir.cxx | 108 ++++++++++++++++++++++++++----------------- src/gdcmDocument.cxx | 7 +-- src/gdcmDocument.h | 18 ++++++++ 5 files changed, 152 insertions(+), 49 deletions(-) create mode 100644 Doc/License.txt diff --git a/ChangeLog b/ChangeLog index 4e298344..01960353 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,25 @@ +2004-06-18 Eric Boix + * Valgrind note: after Mathieu Malaterre teached me how to read + the valgrind FAQ ;-] (see http://valgrind.kde.org/faq.html), I + learned that: + Using gcc, you can force the STL to use malloc and to free memory as + soon as possible by globally disabling memory caching. + With 3.2.2 and later, you should export the environment variable + GLIBCPP_FORCE_NEW before running your program. + By setting GLIBCPP_FORCE_NEW, STL related memory leak messages of gdcm + simply vanish (it is still not clear to me, whether this means that + STL std::string leaks or if valgrind believes it leaks...). + * Fixing of SegFault of Test/makeDicomDir (as shown by ctest or by + running bin/gdcmTests makeDicomDir): + - src/gdcmDicomDir.cxx: dynamic casting used + clean up. + - Test/makeDicomDir.cxx now properly traps empty lists and returns + with 1. + NOW, makeDicomDir cleanly fails (in ctest terminology) instead of + SegFaulting (I drowned in DicomDir related code when trying to + understand why the list is empty...). + * src/gdcmDocument.h: first BSD license header try. + * Doc/License.txt added. + 2004-06-15 Eric Boix * src/gdcmDocument.[h|cxx]: - Clean up of the Transfer related predicates. They are now all based @@ -199,12 +221,12 @@ 2. Tests as mentionned smarter 3. Some clean up 4. Add a new method in gdcmDict that return the PubDict by name - this is interesting for 3rd party lib like ITK, - where we could set the institution name / patient name... + this is interesting for 3rd party lib like ITK, + where we could set the institution name / patient name... * ENH: 1. Now the test suite is working for real 2. All binaries are now output in the gdcm-bin directory - (this was not true before) + (this was not true before) 2004-04-28 Jean-Pierre Roux * ENH add the provisional gdcmHeader::SQDepthLevel to allow diff --git a/Doc/License.txt b/Doc/License.txt new file mode 100644 index 00000000..7c7630b1 --- /dev/null +++ b/Doc/License.txt @@ -0,0 +1,40 @@ +Gdcm is copyrighted as an open-source, Berkely-style license. It allows +unrestricted use, including use in commercial products. The complete +copyright is shown below. + +/*========================================================================= + +Copyright (c) 1999-2003 CREATIS +(CREATIS = Centre de Recherche et d'Applications en Traitement de l'Image) +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + + * Redistributions of source code must retain the above copyright notice, + this list of conditions and the following disclaimer. + + * Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + + * The name of CREATIS, nor the names of any contributors (CNRS, INSERM, UCB, + Universite Lyon I), may be used to endorse or promote products derived + from this software without specific prior written permission. + + * Modified source versions must be plainly marked as such, and must not be + misrepresented as being the original software. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDER AND CONTRIBUTORS ``AS IS'' +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHORS OR CONTRIBUTORS BE LIABLE FOR +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +=========================================================================*/ + diff --git a/src/gdcmDicomDir.cxx b/src/gdcmDicomDir.cxx index 5c508292..bbd9374a 100644 --- a/src/gdcmDicomDir.cxx +++ b/src/gdcmDicomDir.cxx @@ -20,7 +20,6 @@ #include "gdcmDebug.h" #include "gdcmGlobal.h" #include "gdcmHeader.h" - #include "gdcmSeqEntry.h" #include "gdcmSQItem.h" #include "gdcmValEntry.h" @@ -69,12 +68,11 @@ gdcmDicomDir::gdcmDicomDir(const char *FileName, bool parseDir, metaElems=NULL; -// gdcmDocument already executed -// if user passed a root directory, sure we didn't get anything + // gdcmDocument already executed + // if user passed a root directory, sure we didn't get anything - if( GetEntry().begin()==GetEntry().end() ) + if( GetEntry().begin() == GetEntry().end() ) { - // if parseDir == false, it should be tagged as an error dbg.Verbose(0, "gdcmDicomDir::gdcmDicomDir : entry HT empty"); if(strlen(FileName)==1 && FileName[0]=='.') { // user passed '.' as Name @@ -90,14 +88,19 @@ gdcmDicomDir::gdcmDicomDir(const char *FileName, bool parseDir, dbg.Verbose(0, "gdcmDicomDir::gdcmDicomDir : Parse directory" " and create the DicomDir"); ParseDirectory(); + } else { + /// \todo if parseDir == false, it should be tagged as an error } } - else { - gdcmDocEntry *e = GetDocEntryByNumber(0x0004, 0x1220); // Directory record sequence + else + { + // Directory record sequence + gdcmDocEntry *e = GetDocEntryByNumber(0x0004, 0x1220); if (e==NULL) { - dbg.Verbose(0, "gdcmDicomDir::gdcmDicomDir : NO Directory record sequence (0x0004,0x1220)" - ); - // FIXME : what to do when the parsed file IS NOT a DICOMDIR file ? + dbg.Verbose(0, "gdcmDicomDir::gdcmDicomDir : NO Directory record" + " sequence (0x0004,0x1220)"); + /// \todo FIXME : what to do when the parsed file IS NOT a + /// DICOMDIR file ? } CreateDicomDir(); } @@ -122,7 +125,7 @@ gdcmDicomDir::gdcmDicomDir(bool exception_on_error): endArg= NULL; progress=0.0; abort=false; - std::string pathBidon = ""; // Sorry, NULL not allowed ... + std::string pathBidon = "Bidon"; // Sorry, NULL not allowed ... SetElement(pathBidon, GDCM_DICOMDIR_META, NULL); // Set the META elements AddDicomDirMeta(); } @@ -493,7 +496,7 @@ gdcmDicomDirPatient * gdcmDicomDir::NewPatient(void) { } else { - entry->SetLength(entry->GetValue().length()); + entry->SetLength(entry->GetValue().length()); } s->AddDocEntry(entry); } @@ -503,13 +506,12 @@ gdcmDicomDirPatient * gdcmDicomDir::NewPatient(void) { return p; } - /** - * \ingroup gdcmDicomDir * \brief adds to the HTable * the gdcmEntries (Dicom Elements) corresponding to the given type * @param path full path file name (only used when type = GDCM_DICOMDIR_IMAGE - * @param type gdcmObject type to create (GDCM_DICOMDIR_PATIENT, GDCM_DICOMDIR_STUDY, GDCM_DICOMDIR_SERIE ...) + * @param type gdcmObject type to create (GDCM_DICOMDIR_PATIENT, + * GDCM_DICOMDIR_STUDY, GDCM_DICOMDIR_SERIE ...) * @param header gdcmHeader of the current file */ void gdcmDicomDir::SetElement(std::string &path,gdcmDicomDirType type, @@ -519,7 +521,7 @@ void gdcmDicomDir::SetElement(std::string &path,gdcmDicomDirType type, std::list::iterator it; guint16 tmpGr, tmpEl; gdcmDictEntry *dictEntry; - gdcmDocEntry *entry; + gdcmValEntry *entry; std::string val; switch(type) @@ -548,7 +550,7 @@ void gdcmDicomDir::SetElement(std::string &path,gdcmDicomDirType type, tmpGr=it->group; tmpEl=it->elem; dictEntry=GetPubDict()->GetDictEntryByNumber(tmpGr,tmpEl); - entry=new gdcmDocEntry(dictEntry); + entry=new gdcmValEntry(dictEntry); entry->SetOffset(0); // just to avoid further missprinting if(header) @@ -559,20 +561,22 @@ void gdcmDicomDir::SetElement(std::string &path,gdcmDicomDirType type, if(val==GDCM_UNFOUND) { if((tmpGr==0x0004) &&(tmpEl==0x1130) ) // File-set ID - { - // force to the *end* File Name - val=GetName(path); + { + // force to the *end* File Name + val=GetName(path); } else if( (tmpGr==0x0004) && (tmpEl==0x1500) ) // Only used for image { if(header->GetFileName().substr(0,path.length())!=path) { - dbg.Verbose(0, "gdcmDicomDir::SetElement : the base path of file name is incorrect"); + dbg.Verbose(0, "gdcmDicomDir::SetElement : the base path" + " of file name is incorrect"); val=header->GetFileName(); } - else { + else + { val=&(header->GetFileName().c_str()[path.length()]); - } + } } else { @@ -582,18 +586,18 @@ void gdcmDicomDir::SetElement(std::string &path,gdcmDicomDirType type, else { if (header->GetEntryLengthByNumber(tmpGr,tmpEl)== 0) - val=it->value; + val=it->value; } - ((gdcmValEntry *)entry)->SetValue(val); + entry->SetValue(val); if(dictEntry) { if(dictEntry->GetGroup()==0xfffe) - { - entry->SetLength(((gdcmValEntry *)entry)->GetValue().length()); - } - else if( (dictEntry->GetVR()=="UL") || (dictEntry->GetVR()=="SL") ) + { + entry->SetLength(entry->GetValue().length()); + } + else if( (dictEntry->GetVR()=="UL") || (dictEntry->GetVR()=="SL") ) { entry->SetLength(4); } @@ -607,15 +611,15 @@ void gdcmDicomDir::SetElement(std::string &path,gdcmDicomDirType type, } else { - entry->SetLength(((gdcmValEntry *)entry)->GetValue().length()); + entry->SetLength(entry->GetValue().length()); } } //AddDocEntry(entry); // both in H Table and in chained list tagHT[entry->GetKey()] = entry; // FIXME : use a SEQUENCE ! } } + /** - * \ingroup gdcmDicomDir * \brief CallStartMethod */ void gdcmDicomDir::CallStartMethod(void) @@ -663,31 +667,47 @@ void gdcmDicomDir::CreateDicomDir() gdcmDicomDirType type=gdcmDicomDir::GDCM_DICOMDIR_META; - gdcmDocEntry *e = GetDocEntryByNumber(0x0004, 0x1220); // Directory record sequence - if (e==NULL) { - dbg.Verbose(0, "gdcmDicomDir::gdcmDicomDir : NO Directory record sequence (0x0004,0x1220)" - ); - // FIXME : what to do when the parsed file IS NOT a DICOMDIR file ? + // Directory record sequence + gdcmDocEntry *e = GetDocEntryByNumber(0x0004, 0x1220); + if (e==NULL) + { + dbg.Verbose(0, "gdcmDicomDir::gdcmDicomDir : NO Directory record" + " sequence (0x0004,0x1220)"); + /// \todo FIXME: what to do when the parsed file IS NOT a DICOMDIR file ? return; } - gdcmDicomDirMeta *m = new gdcmDicomDirMeta(&tagHT); - - gdcmSeqEntry *s = (gdcmSeqEntry *)e; // FIXME : It is allowed ??? + gdcmSeqEntry* s = dynamic_cast(e); + if (!s) + { + dbg.Verbose(0, "gdcmDicomDir::CreateDicomDir: no SeqEntry present"); + return; + } + ListSQItem listItems = s->GetSQItems(); + gdcmDicomDirMeta *m = new gdcmDicomDirMeta(&tagHT); gdcmDocEntry * d; + std::string v; for(ListSQItem::iterator i=listItems.begin(); i !=listItems.end();++i) { d=(*i)->GetDocEntryByNumber(0x0004, 0x1430); // Directory Record Type - std::string v=((gdcmValEntry *)d)->GetValue(); - + if (gdcmValEntry* ValEntry = dynamic_cast< gdcmValEntry* >(d) ) + { + v = ValEntry->GetValue(); + } + else + { + dbg.Verbose(0, "gdcmDicomDir::CreateDicomDir: not a ValEntry."); + continue; + } + if(v=="PATIENT ") { AddDicomDirPatientToEnd(*i); //AddObjectToEnd(type,*i); type=gdcmDicomDir::GDCM_DICOMDIR_PATIENT; - } + } else if(v=="STUDY ") { @@ -711,7 +731,9 @@ void gdcmDicomDir::CreateDicomDir() } else - continue ; // It was 'non PATIENT', 'non STUDY', 'non SERIE', 'non IMAGE' SQItem + // It was not a 'PATIENT', nor a 'STUDY', nor a 'SERIE', + // neither an 'IMAGE' SQItem. Skip to next item. + continue; } } /** diff --git a/src/gdcmDocument.cxx b/src/gdcmDocument.cxx index 715a7f60..eb73f948 100644 --- a/src/gdcmDocument.cxx +++ b/src/gdcmDocument.cxx @@ -95,6 +95,8 @@ gdcmDocument::gdcmDocument(const char *inFilename, if ( !OpenFile(exception_on_error)) return; + dbg.Verbose(0, "gdcmDocument::gdcmDocument: starting parsing of file: ", + filename.c_str()); rewind(fp); //if (!CheckSwap()) // return false; // to go on compiling @@ -227,9 +229,8 @@ bool gdcmDocument::IsReadable(void) { return(false); } if(!tagHT.empty()<=0) { - std::cout << "gdcmDocument::IsReadable: wrong tagHT size : " - << tagHT.size() - <