From f0ea8af1179c35ddc83da32df10813899427214d Mon Sep 17 00:00:00 2001 From: malaterre Date: Fri, 5 Nov 2004 20:23:12 +0000 Subject: [PATCH 1/1] ENH: Improve string manipulation. I now inforce the notion of 'DicomString' A DicomString can contain as many \0 as they want and it is *always* of even length. We only support odd length for very rare case. And in the near future this should be removed. --- ChangeLog | 7 ++++++ Dicts/DicomDir.dic | 1 + Testing/CMakeLists.txt | 1 + Testing/TestAllEntryVerify.cxx | 16 +++++++++--- Testing/TestDicomString.cxx | 24 ++++++++++++++++++ src/gdcmDocument.cxx | 45 ++++++++++++++++++++++------------ src/gdcmHeader.cxx | 9 ++++--- src/gdcmUtil.cxx | 39 ++++++++++++++++++++++++++--- src/gdcmUtil.h | 7 ++++-- src/gdcmValEntry.cxx | 12 ++++----- src/gdcmValEntry.h | 6 ++--- 11 files changed, 129 insertions(+), 38 deletions(-) create mode 100644 Testing/TestDicomString.cxx diff --git a/ChangeLog b/ChangeLog index 58061aac..9b07f6a3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2004-11-05 Mathieu Malaterre + * Improve string manipulation. I now inforce the notion of 'DicomString' + A DicomString can contain as many \0 as they want + and it is *always* of even length. + We only support odd length for very rare case. + And in the near future this should be removed. + 2004-11-03 Mathieu Malaterre * /binary_write/ gdcm source. Now even on big endian we are writting little endian. This should -heopfully- fix some tests diff --git a/Dicts/DicomDir.dic b/Dicts/DicomDir.dic index 377e0a1c..d65145fb 100644 --- a/Dicts/DicomDir.dic +++ b/Dicts/DicomDir.dic @@ -81,3 +81,4 @@ imageElem 0028 0011 "0" // Columns imageElem 0028 0030 "1.0\1.0 " // Pixel Spacing imageElem 0050 0004 "0" // Calibration Image imageElem fffe e00d "" // Item delimitation : length to be set to ZERO later + diff --git a/Testing/CMakeLists.txt b/Testing/CMakeLists.txt index 0899b62a..fa3a96bd 100644 --- a/Testing/CMakeLists.txt +++ b/Testing/CMakeLists.txt @@ -12,6 +12,7 @@ SET(TEST_SOURCES TestHash.cxx TestTS.cxx TestVR.cxx + TestDicomString.cxx ) # add tests that require data diff --git a/Testing/TestAllEntryVerify.cxx b/Testing/TestAllEntryVerify.cxx index 995dcf26..a2dc4abc 100644 --- a/Testing/TestAllEntryVerify.cxx +++ b/Testing/TestAllEntryVerify.cxx @@ -194,11 +194,19 @@ bool ReferenceFileParser::Check() std::string testedValue = tested->GetEntryByNumber(group, element); if ( testedValue != j->second ) { - std::cout << Indent << "Uncorrect value for key " << key << std::endl - << Indent << " read value [" << testedValue << "]" << std::endl - << Indent << " reference value [" << j->second << "]" - << std::endl; + // Oops make sure this is only the \0 that differ + if( testedValue[j->second.size()] != '\0' || + strncmp(testedValue.c_str(), + j->second.c_str(), j->second.size()) != 0) + { + std::cout << Indent << "Uncorrect value for key " + << key << std::endl + << Indent << " read value [" + << testedValue << "]" << std::endl + << Indent << " reference value [" + << j->second << "]" << std::endl; return false; + } } } delete tested; diff --git a/Testing/TestDicomString.cxx b/Testing/TestDicomString.cxx new file mode 100644 index 00000000..f12073ed --- /dev/null +++ b/Testing/TestDicomString.cxx @@ -0,0 +1,24 @@ +#include "gdcmUtil.h" + +int TestDicomString(int , char* []) +{ + int i; + const char *s = "\0\0"; + std::string a(s,s+2); // will copy 2 '\0' + assert( a.size() == 2 ); + for(i=0;i<2;i++) + { + assert( a.c_str()[i] == '\0' ); + assert( a.data()[i] == '\0' ); + } + assert( a.c_str()[3] == '\0' ); + +/* +std::string zeros(x, 0); +char s1[] = "\0"; +char s2[] = "\0\0"; +char s3[] = "\0\0\0"; +char s4[] = "\0abc";*/ + + return 0; +} diff --git a/src/gdcmDocument.cxx b/src/gdcmDocument.cxx index 615f8acd..bdf5a01c 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/04 18:14:34 $ - Version: $Revision: 1.116 $ + Date: $Date: 2004/11/05 20:23:14 $ + Version: $Revision: 1.117 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -476,7 +476,8 @@ void Document::Write(std::ofstream* fp, FileType filetype) if (filetype == ImplicitVR) { - std::string ts = TransferSyntaxStrings[ImplicitVRLittleEndian]; + std::string ts = + Util::DicomString( TransferSyntaxStrings[ImplicitVRLittleEndian] ); ReplaceOrCreateByNumber(ts, 0x0002, 0x0010); /// \todo Refer to standards on page 21, chapter 6.2 @@ -489,7 +490,8 @@ void Document::Write(std::ofstream* fp, FileType filetype) if (filetype == ExplicitVR) { - std::string ts = TransferSyntaxStrings[ExplicitVRLittleEndian]; + std::string ts = + Util::DicomString( TransferSyntaxStrings[ExplicitVRLittleEndian] ); ReplaceOrCreateByNumber(ts, 0x0002, 0x0010); /// \todo Refer to standards on page 21, chapter 6.2 @@ -881,13 +883,10 @@ bool Document::SetEntryByNumber(std::string const& content, return false; } // Non even content must be padded with a space (020H)... - std::string finalContent = content; - if( finalContent.length() % 2 ) - { - finalContent += '\0'; // ... therefore we padd with (000H) .!?! - } + std::string finalContent = Util::DicomString( content.c_str() ); + assert( !(finalContent.size() % 2) ); valEntry->SetValue(finalContent); - + // Integers have a special treatement for their length: l = finalContent.length(); @@ -1586,7 +1585,8 @@ void Document::LoadDocEntry(DocEntry* entry) } // to be sure we are at the end of the value ... - Fp->seekg((long)entry->GetOffset()+(long)entry->GetLength(),std::ios_base::beg); + Fp->seekg((long)entry->GetOffset()+(long)entry->GetLength(), + std::ios_base::beg); return; } @@ -1648,15 +1648,28 @@ void Document::LoadDocEntry(DocEntry* entry) return; } - // We need an additional byte for storing \0 that is not on disk + // FIXME: We need an additional byte for storing \0 that is not on disk char *str = new char[length+1]; Fp->read(str, (size_t)length); - str[length] = '\0'; - std::string newValue = str; + str[length] = '\0'; //this is only useful when length is odd + // Special DicomString call to properly handle \0 and even length + std::string newValue; + if( length % 2 ) + { + newValue = Util::DicomString(str, length+1); + //dbg.Verbose(0, "Warning: bad length: ", length ); + dbg.Verbose(0, "For string :", newValue.c_str()); + // Since we change the length of string update it length + entry->SetReadLength(length+1); + } + else + { + newValue = Util::DicomString(str, length); + } delete[] str; if ( ValEntry* valEntry = dynamic_cast(entry) ) - { + { if ( Fp->fail() || Fp->eof())//Fp->gcount() == 1 { dbg.Verbose(1, "Document::LoadDocEntry", @@ -2180,7 +2193,7 @@ bool Document::IsDocEntryAnInteger(DocEntry *entry) { uint16_t element = entry->GetElement(); uint16_t group = entry->GetGroup(); - std::string vr = entry->GetVR(); + const std::string & vr = entry->GetVR(); uint32_t length = entry->GetLength(); // When we have some semantics on the element we just read, and if we diff --git a/src/gdcmHeader.cxx b/src/gdcmHeader.cxx index c68aa171..ed7402e2 100644 --- a/src/gdcmHeader.cxx +++ b/src/gdcmHeader.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmHeader.cxx,v $ Language: C++ - Date: $Date: 2004/11/04 15:59:37 $ - Version: $Revision: 1.197 $ + Date: $Date: 2004/11/05 20:23:14 $ + Version: $Revision: 1.198 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -115,8 +115,8 @@ void Header::Write(std::ofstream* fp,FileType filetype) if (i_lgPix != -2) { // no (GrPixel, NumPixel) element - std::string s_lgPix; - s_lgPix = Util::Format("%d", i_lgPix+12); + std::string s_lgPix = Util::Format("%d", i_lgPix+12); + s_lgPix = Util::DicomString( s_lgPix.c_str() ); ReplaceOrCreateByNumber(s_lgPix,GrPixel, 0x0000); } @@ -1178,6 +1178,7 @@ void Header::SetImageDataSize(size_t ImageDataSize) ImageDataSize += 8; car = Util::Format("%d", ImageDataSize); + car = Util::DicomString( car.c_str() ); SetEntryByNumber(car, GrPixel, NumPixel); } diff --git a/src/gdcmUtil.cxx b/src/gdcmUtil.cxx index f8dbe313..ebb8191d 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/03 20:52:13 $ - Version: $Revision: 1.59 $ + Date: $Date: 2004/11/05 20:23:14 $ + Version: $Revision: 1.60 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -168,7 +168,7 @@ std::string Util::GetPath(std::string const & fullName) } /** - * \ingroup Globals + * \ingroup Util * \brief Get the (last) name of a full path file name * @param fullName file/directory name to extract end name from */ @@ -189,6 +189,39 @@ std::string Util::GetName(std::string const & fullName) } } +/** + * \ingroup Util + * \brief Create a /DICOM/ string: + * It should a of even lenght (no odd length ever) + * It can contains as many \0 as you want. + * This function is similar to DicomString(const char*), + * except it doesn't take a lenght. + * It only pad with a null character if length is odd + */ +std::string Util::DicomString(const char* s) +{ + size_t l = strlen(s); + if( l%2 ) + { + l++; + } + std::string r(s, s+l); + assert( !(r.size() % 2) ); + return r; +} + +/** + * \ingroup Util + * \brief Create a /DICOM/ string: + * It should a of even length (no odd length ever) + * It can contains as many \0 as you want. + */ +std::string Util::DicomString(const char* s, size_t l) +{ + std::string r(s, s+l); + assert( !(r.size() % 2) ); + return r; +} template std::ostream& binary_write(std::ostream& os, const T& val) diff --git a/src/gdcmUtil.h b/src/gdcmUtil.h index 0d3334fe..a32b5f2c 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/03 20:52:13 $ - Version: $Revision: 1.39 $ + Date: $Date: 2004/11/05 20:23:14 $ + Version: $Revision: 1.40 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -49,7 +49,10 @@ public: static std::string GetPath(std::string const &fullName); static std::string GetName(std::string const &fullName); + static std::string DicomString(const char* s, size_t l); + static std::string DicomString(const char* s); }; + template std::ostream& binary_write(std::ostream& os, const T& val); std::ostream& binary_write(std::ostream& os, const uint16_t& val); diff --git a/src/gdcmValEntry.cxx b/src/gdcmValEntry.cxx index 6293e5dc..05244476 100644 --- a/src/gdcmValEntry.cxx +++ b/src/gdcmValEntry.cxx @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmValEntry.cxx,v $ Language: C++ - Date: $Date: 2004/11/03 20:52:13 $ - Version: $Revision: 1.32 $ + Date: $Date: 2004/11/05 20:23:14 $ + Version: $Revision: 1.33 $ Copyright (c) CREATIS (Centre de Recherche et d'Applications en Traitement de l'Image). All rights reserved. See Doc/License.txt or @@ -222,10 +222,10 @@ void ValEntry::Write(std::ofstream* fp, FileType filetype) tokens.clear(); return; } - - fp->write (GetValue().c_str(), (size_t)lgr ); // Elem value -// assert( lgr == GetValue().size() ); // FIXME ????? -// dbg.Assert(2, lgr == strlen(GetValue().c_str()), "Should be equal" ); + + //Due to seriously broken Theralys images I cannot put that here. + //assert( lgr == GetValue().size() ); + binary_write(*fp, GetValue()); } //----------------------------------------------------------------------------- diff --git a/src/gdcmValEntry.h b/src/gdcmValEntry.h index 6afc162f..84a1418e 100644 --- a/src/gdcmValEntry.h +++ b/src/gdcmValEntry.h @@ -3,8 +3,8 @@ Program: gdcm Module: $RCSfile: gdcmValEntry.h,v $ Language: C++ - Date: $Date: 2004/10/28 03:10:58 $ - Version: $Revision: 1.29 $ + Date: $Date: 2004/11/05 20:23:14 $ + 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 @@ -43,7 +43,7 @@ public: std::string const & GetValue() const { return Value; }; /// Sets the value (string) of the current Dicom Document Entry - void SetValue(std::string const & val) { Value = val; }; + void SetValue(std::string const & val) { Value = val; }; virtual void Print(std::ostream &os = std::cout); virtual void Write(std::ofstream *fp, FileType filetype); -- 2.45.2