From a0091d68a2eaa1a0c128f962030bb6c45cc0c366 Mon Sep 17 00:00:00 2001 From: malaterre Date: Tue, 16 Nov 2004 02:54:34 +0000 Subject: [PATCH] ENH: Slightly bigger patch: 1. Getting toward full integration of JMR patch for writting dicom from scratch 2. Update Test to test part of this patch: CreateUniqueUID 3. File was not close properly in gdcmDict 4. Use of typedef is to be prefered when possible (gdcmDict.cxx) 5. Use of const ref instead of copy (speed issue) 6. Remove temporary (duplicate) string in TranslateToKey 7. Mark extremely dangerous code as such (gdcmDocument.cxx and AddEntry fallback case) 8. Do not repeat virtual in subclasses 9. Implemented in gdcm::Util two new function: GetIPAddress, and CreateUniqueUID --- ChangeLog | 12 +++++++ Testing/TestUtil.cxx | 2 ++ src/gdcmDict.cxx | 24 +++++-------- src/gdcmDict.h | 11 +++--- src/gdcmDictEntry.cxx | 8 ++--- src/gdcmDocument.cxx | 20 +++++++---- src/gdcmDocument.h | 6 ++-- src/gdcmElementSet.cxx | 20 ++++++----- src/gdcmFile.cxx | 5 ++- src/gdcmSeqEntry.h | 12 +++---- src/gdcmUtil.cxx | 82 ++++++++++++++++++++++++++++++++++++++++-- src/gdcmUtil.h | 8 +++-- 12 files changed, 152 insertions(+), 58 deletions(-) diff --git a/ChangeLog b/ChangeLog index 39809c83..e4a4b8b7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +2004-11-15 Mathieu Malaterre + * ENH: Slightly bigger patch: + 1. Getting toward full integration of JMR patch for writting dicom from scratch + 2. Update Test to test part of this patch: CreateUniqueUID + 3. File was not close properly in gdcmDict + 4. Use of typedef is to be prefered when possible (gdcmDict.cxx) + 5. Use of const ref instead of copy (speed issue) + 6. Remove temporary (duplicate) string in TranslateToKey + 7. Mark extremely dangerous code as such (gdcmDocument.cxx and AddEntry fallback case) + 8. Do not repeat virtual in subclasses + 9. Implemented in gdcm::Util two new function: GetIPAddress, and CreateUniqueUID + 2004-11-15 Mathieu Malaterre * Apply first patch toward better string comparison when dealing with broken DICOM files. Essentially the string could be padded with a space instead diff --git a/Testing/TestUtil.cxx b/Testing/TestUtil.cxx index fb9cb97c..8804c720 100644 --- a/Testing/TestUtil.cxx +++ b/Testing/TestUtil.cxx @@ -5,6 +5,8 @@ int TestUtil(int , char * []) { + std::cout << gdcm::Util::CreateUniqueUID("") << std::endl; + const char ref[] = "MONOCHROME1"; std::string a = "MONOCHROME1"; a += '\0'; diff --git a/src/gdcmDict.cxx b/src/gdcmDict.cxx index 051d44de..19b93639 100644 --- a/src/gdcmDict.cxx +++ b/src/gdcmDict.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmDict.cxx,v $ Language: C++ - Date: $Date: 2004/11/03 18:08:56 $ - Version: $Revision: 1.51 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.52 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -63,10 +63,10 @@ Dict::Dict(std::string const & filename) const DictEntry newEntry(group, element, vr, fourth, name); AddNewEntry(newEntry); } - from.close(); Filename = filename; } + from.close(); } /** @@ -159,12 +159,8 @@ bool Dict::AddNewEntry(DictEntry const & newEntry) } else { - KeyHt.insert( - std::map::value_type - (newEntry.GetKey(), newEntry)); - NameHt.insert( - std::map::value_type - (newEntry.GetName(), newEntry )); + KeyHt.insert( TagKeyHT::value_type(newEntry.GetKey(), newEntry)); + NameHt.insert( TagNameHT::value_type(newEntry.GetName(), newEntry )); return true; } } @@ -179,12 +175,8 @@ bool Dict::ReplaceEntry(DictEntry const & newEntry) { if ( RemoveEntry(newEntry.GetKey()) ) { - KeyHt.insert( - std::map::value_type - (newEntry.GetKey(), newEntry)); - NameHt.insert( - std::map::value_type - (newEntry.GetName(), newEntry )); + KeyHt.insert( TagKeyHT::value_type(newEntry.GetKey(), newEntry)); + NameHt.insert( TagNameHT::value_type(newEntry.GetName(), newEntry )); return true; } return false; @@ -202,7 +194,7 @@ bool Dict::RemoveEntry (TagKey const & key) TagKeyHT::const_iterator it = KeyHt.find(key); if(it != KeyHt.end()) { - const DictEntry & entryToDelete = it->second; + const DictEntry& entryToDelete = it->second; NameHt.erase(entryToDelete.GetName()); KeyHt.erase(key); diff --git a/src/gdcmDict.h b/src/gdcmDict.h index 2b90d10b..dd4910dc 100644 --- a/src/gdcmDict.h +++ b/src/gdcmDict.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmDict.h,v $ Language: C++ - Date: $Date: 2004/10/27 22:31:12 $ - Version: $Revision: 1.22 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.23 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -30,10 +30,11 @@ namespace gdcm { //----------------------------------------------------------------------------- -typedef std::map TagKeyHT; +typedef std::map TagKeyHT; typedef std::map TagNameHT; -typedef std::list EntryNamesList; -typedef std::map > EntryNamesByCatMap; +typedef std::list EntryNamesList; +typedef std::map > EntryNamesByCatMap; //----------------------------------------------------------------------------- /* * \defgroup Dict diff --git a/src/gdcmDictEntry.cxx b/src/gdcmDictEntry.cxx index 77590de3..bc0dddfd 100644 --- a/src/gdcmDictEntry.cxx +++ b/src/gdcmDictEntry.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmDictEntry.cxx,v $ Language: C++ - Date: $Date: 2004/10/18 02:35:35 $ - Version: $Revision: 1.28 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.29 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -61,9 +61,7 @@ DictEntry::DictEntry(uint16_t group, uint16_t element, */ TagKey DictEntry::TranslateToKey(uint16_t group, uint16_t element) { - TagKey key = Util::Format("%04x|%04x", group , element); - - return key; + return Util::Format("%04x|%04x", group, element); } //----------------------------------------------------------------------------- diff --git a/src/gdcmDocument.cxx b/src/gdcmDocument.cxx index 879a3348..821b7027 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/15 16:12:30 $ - Version: $Revision: 1.123 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.124 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -1346,7 +1346,7 @@ void Document::ParseDES(DocEntrySet *set, long offset, { /////////////////////// ValEntry ValEntry* newValEntry = - new ValEntry( newDocEntry->GetDictEntry() ); + new ValEntry( newDocEntry->GetDictEntry() ); //LEAK newValEntry->Copy( newDocEntry ); // When "set" is a Document, then we are at the top of the @@ -1365,7 +1365,11 @@ void Document::ParseDES(DocEntrySet *set, long offset, + newValEntry->GetKey() ); } - set->AddEntry( newValEntry ); + if( !set->AddEntry( newValEntry ) ) + { + // If here expect big troubles + delete newValEntry; //otherwise mem leak + } LoadDocEntry( newValEntry ); if (newValEntry->IsItemDelimitor()) { @@ -1406,7 +1410,11 @@ void Document::ParseDES(DocEntrySet *set, long offset, + newBinEntry->GetKey() ); } - set->AddEntry( newBinEntry ); + if( !set->AddEntry( newBinEntry ) ) + { + //Expect big troubles if here + delete newBinEntry; + } LoadDocEntry( newBinEntry ); } @@ -1596,7 +1604,7 @@ void Document::LoadDocEntry(DocEntry* entry) s << " x(" << std::hex << entry->GetLength() << ")"; binEntryPtr->SetValue(s.str()); } - // Be carefull : a BinEntry IS_A ValEntry ... + // Be carefull : a BinEntry IS_A ValEntry ... else if (ValEntry* valEntryPtr = dynamic_cast< ValEntry* >(entry) ) { // s << "gdcm::NotLoaded. (ValEntry)"; diff --git a/src/gdcmDocument.h b/src/gdcmDocument.h index 26cca8c8..8f9b84e9 100644 --- a/src/gdcmDocument.h +++ b/src/gdcmDocument.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmDocument.h,v $ Language: C++ - Date: $Date: 2004/11/15 16:12:30 $ - Version: $Revision: 1.58 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.59 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -261,7 +261,7 @@ private: void BuildFlatHashTableRecurse( TagDocEntryHT& builtHT, DocEntrySet* set ); - void HandleBrokenEndian(uint16_t group, uint16_t elem); + void HandleBrokenEndian(uint16_t group, uint16_t elem); public: // Accessors: /// Accessor to \ref PrintLevel diff --git a/src/gdcmElementSet.cxx b/src/gdcmElementSet.cxx index 47de1a18..22f4ed98 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/05 21:23:46 $ - Version: $Revision: 1.26 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.27 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -90,7 +90,9 @@ void ElementSet::Print(std::ostream& os) */ void ElementSet::Write(std::ofstream* fp, FileType filetype) { - for (TagDocEntryHT::const_iterator i = TagHT.begin(); i != TagHT.end(); ++i) + for (TagDocEntryHT::const_iterator i = TagHT.begin(); + i != TagHT.end(); + ++i) { i->second->Write(fp, filetype); } @@ -107,9 +109,9 @@ void ElementSet::Write(std::ofstream* fp, FileType filetype) * \brief add a new Dicom Element pointer to the H Table * @param newEntry entry to add */ -bool ElementSet::AddEntry( DocEntry* newEntry) +bool ElementSet::AddEntry(DocEntry* newEntry) { - TagKey key = newEntry->GetKey(); + const TagKey& key = newEntry->GetKey(); if( TagHT.count(key) == 1 ) { @@ -119,7 +121,7 @@ bool ElementSet::AddEntry( DocEntry* newEntry) } else { - TagHT[newEntry->GetKey()] = newEntry; + TagHT.insert(TagDocEntryHT::value_type(newEntry->GetKey(), newEntry)); return true; } } @@ -128,9 +130,9 @@ bool ElementSet::AddEntry( DocEntry* newEntry) * \brief Clear the hash table from given entry BUT keep the entry. * @param entryToRemove Entry to remove. */ -bool ElementSet::RemoveEntryNoDestroy( DocEntry* entryToRemove) +bool ElementSet::RemoveEntryNoDestroy(DocEntry* entryToRemove) { - TagKey key = entryToRemove->GetKey(); + const TagKey& key = entryToRemove->GetKey(); if( TagHT.count(key) == 1 ) { TagHT.erase(key); @@ -148,7 +150,7 @@ bool ElementSet::RemoveEntryNoDestroy( DocEntry* entryToRemove) */ bool ElementSet::RemoveEntry( DocEntry* entryToRemove) { - TagKey key = entryToRemove->GetKey(); + const TagKey& key = entryToRemove->GetKey(); if( TagHT.count(key) == 1 ) { TagHT.erase(key); diff --git a/src/gdcmFile.cxx b/src/gdcmFile.cxx index 2faa6553..b5de832e 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/15 15:29:04 $ - Version: $Revision: 1.154 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.155 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -110,7 +110,6 @@ File::~File() { delete PixelConverter; } - } /** diff --git a/src/gdcmSeqEntry.h b/src/gdcmSeqEntry.h index e23657f9..5d0ca21b 100644 --- a/src/gdcmSeqEntry.h +++ b/src/gdcmSeqEntry.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmSeqEntry.h,v $ Language: C++ - Date: $Date: 2004/10/25 03:03:45 $ - Version: $Revision: 1.22 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.23 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -33,17 +33,17 @@ class GDCM_EXPORT SeqEntry : public DocEntry { public: SeqEntry( DictEntry* ); - SeqEntry(DocEntry* d, int depth); + SeqEntry( DocEntry* d, int depth ); ~SeqEntry(); - virtual void Print(std::ostream &os = std::cout); - virtual void Write(std::ofstream *fp, FileType); + void Print(std::ostream &os = std::cout); + void Write(std::ofstream *fp, FileType filetype); /// returns the SQITEM chained List for this SeQuence. ListSQItem const & GetSQItems() const { return Items; } /// Sets the delimitor mode - void SetDelimitorMode(bool dm) { DelimitorMode = dm;} + void SetDelimitorMode(bool dm) { DelimitorMode = dm; } /// Sets the Sequence Delimitation Item void SetSequenceDelimitationItem(DocEntry * e) { SeqTerm = e;} diff --git a/src/gdcmUtil.cxx b/src/gdcmUtil.cxx index 30adaed2..b551067f 100644 --- a/src/gdcmUtil.cxx +++ b/src/gdcmUtil.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmUtil.cxx,v $ Language: C++ - Date: $Date: 2004/11/16 02:04:00 $ - Version: $Revision: 1.63 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.64 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -27,6 +27,9 @@ #include //only included in implementation file #include //only included in implementation file +#include // for gethostname +#include // for gethostbyname + namespace gdcm { @@ -267,7 +270,7 @@ std::string Util::DicomString(const char* s) bool Util::DicomStringEqual(const std::string& s1, const char *s2) { // s2 is the string from the DICOM reference: 'MONOCHROME1' - std::string s1_even = s1; //Never directly change input parameter + std::string s1_even = s1; //Never change input parameter std::string s2_even = DicomString( s2 ); if( s1_even[s1_even.size()-1] == ' ') { @@ -276,6 +279,79 @@ bool Util::DicomStringEqual(const std::string& s1, const char *s2) return s1_even == s2_even; } +/** + * \ingroup Util + * \brief Return the IP adress of the machine writting the DICOM image + */ +std::string Util::GetIPAddress() +{ + // This is a rip from http://www.codeguru.com/Cpp/I-N/internet/network/article.php/c3445/ +#ifndef HOST_NAME_MAX + // SUSv2 guarantees that `Host names are limited to 255 bytes'. + // POSIX 1003.1-2001 guarantees that `Host names (not including the + // terminating NUL) are limited to HOST_NAME_MAX bytes'. +# define HOST_NAME_MAX 255 + // In this case we should maybe check the string was not truncated. + // But I don't known how to check that... +#endif //HOST_NAME_MAX + + std::string str; + char szHostName[HOST_NAME_MAX+1]; + int r = gethostname(szHostName, HOST_NAME_MAX); + + if( r == 0 ) + { + // Get host adresses + struct hostent * pHost = gethostbyname(szHostName); + + for( int i = 0; pHost!= NULL && pHost->h_addr_list[i]!= NULL; i++ ) + { + for( int j = 0; jh_length; j++ ) + { + if( j > 0 ) str += "."; + + str += Util::Format("%u", + (unsigned int)((unsigned char*)pHost->h_addr_list[i])[j]); + } + // str now contains one local IP address + } + } + // If an error occur r == -1 + // Most of the time it will return 127.0.0.1... + return str; +} + +/** + * \ingroup Util + * \brief Creates a new UID. As stipulate in the DICOM ref + * each time a DICOM image is create it should have + * a unique identifier (URI) + */ +std::string Util::CreateUniqueUID(const std::string& root) +{ + // The code works as follow: + // echo "gdcm" | od -b + // 0000000 147 144 143 155 012 + // Therefore we return + // radical + 147.144.143.155 + IP + time() + std::string radical = root; + if( !root.size() ) //anything better ? + { + radical = "0.0."; // Is this really usefull ? + } + // else + // A root was specified use it to forge our new UID: + radical += "147.144.143.155"; // gdcm + radical += "."; + radical += Util::GetIPAddress(); + radical += "."; + radical += Util::GetCurrentDate(); + radical += "."; + radical += Util::GetCurrentTime(); + + return radical; +} + template std::ostream& binary_write(std::ostream& os, const T& val) { diff --git a/src/gdcmUtil.h b/src/gdcmUtil.h index 99cb2555..bf897015 100644 --- a/src/gdcmUtil.h +++ b/src/gdcmUtil.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmUtil.h,v $ Language: C++ - Date: $Date: 2004/11/16 02:04:00 $ - Version: $Revision: 1.43 $ + Date: $Date: 2004/11/16 02:54:35 $ + Version: $Revision: 1.44 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -54,6 +54,10 @@ public: static std::string DicomString(const char* s, size_t l); static std::string DicomString(const char* s); static bool DicomStringEqual(const std::string& s1, const char *s2); + static std::string CreateUniqueUID(const std::string& root); + +private: + static std::string GetIPAddress(); //Do not expose this method }; template -- 2.45.1