[Insight-developers] Roll back MetaDataObject and Dicitonary changes

[Bill Lorensen]

Our sgi's have some RTTI probelms. Your code was correct. The ifdef is so that our SGI's can build. Our SGI's had unresolved references. BTW, any time you do a dynamic cast, you should check to see if the cast succeeded.

Please do not roll them back. Maybe we can put an SGI version test in. I've seen this problem before.

2) The borland compiler could not cast it's arguments properly for the signature you provided. I had to provide a char * signature.
Sometimes we have to work a little harder for portability

I'm leaving on vacation for a week and won't have connectivity. We can discuss this more when I get back.

Bill

At 12:06 PM 3/20/03 -0600, Hans J. Johnson wrote:
>Bill,
>
>I have been spent most of this morning reviewing the changes you made
>the itkMetaDataDictionary, and itkAnalyzeImageIO, and I would really
>like to roll as many of those changes back as possible.
>
>Will you please tell me why you feel these changes are necessary?  I do
>not have access to the borland compilers, (or VC7 today).
>
>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>1)^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>#if (defined(__sgi) && !defined(__GNUC__))
>      outval =
>        reinterpret_cast<MetaDataObject <T>
>*>(Dictionary[key].GetPointer())->GetMetaData
>ObjectValue();
>#else
>      outval =
>        dynamic_cast<MetaDataObject <T>
>*>(Dictionary[key].GetPointer())->GetMetaDataObje
>ctValue();
>#endif
>
>from page 10.4.11 pg 256 of the Stroustrup book:
>========================================================================
>The reinterpret_cast is the crudest and potentially nastiest of the type
>conversion operators.  In most caes, it simply yeilds a value with the
>same bit pattern as it's argument wit the type required .  Thus, it can
>be used for the inherently implementation-depend, dangerous, and
>occasionally absolutely necessary activity of converting interger values
>to pointers, and  vice versa.
>========================================================================
>I strongly believe that this needs to be a dynamic_cast down the
>inheritance structure.
>
>I have rolled this back in my experimental builds, and it compiles and
>the MetaDataDictionary tests pass with the SGI compilers MIPSPro
>compilers.
>
>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>2)^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>a)==============================================================
>template <class T>
>inline bool ExposeMetaData
>(
>  MetaDataDictionary &Dictionary,
>  const char *key, 
>  T &outval
>)
>    {
>      return ExposeMetaData(Dictionary, std::string(key), outval);
>    }
>
>} // end namespace itk
>
>b)==============================================================
>template <class T>
>inline bool ExposeMetaData
>(
>  MetaDataDictionary &Dictionary, 
>  const std::string key,
>  T &outval
>)
>    {
>        //blah the real work
>    }
>==================================================================
>How does (a) differ functionally from (b) when the key argument is send
>in as a const char *?
>
>In both cases a std::string is instantiated and inintialized with that
>const char * variable.
>
>If feel that the (a) method is not needed.  I have rolled this back in
>my experimental builds, and it compiles and the MetaDataDictionary test
>pass with the SGI and GCC 2.96, GCC 3.2 and icc, compilers. 
>
>
>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>3)^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>The (b) option 
>
>a)==============================================================
>{
>char temp[2048];
>strncpy(temp,"FirstValue",10);//Note this is necessary  because the
>array is not necessarily null terminated.
>temp[10]='\0';
>itk::EncapsulateMetaData<char *>(thisDic,"ITK_FileOriginator",temp);
>strncpy(temp,"SecondValue",18);//Note this is necessary because the
>array is not necessarily null terminated.
>temp[18]='\0';
>itk::EncapsulateMetaData<char *>(thisDic,"ITK_ImageFileBaseName",temp);
>}//End of scope
>b)==============================================================
>{
>char temp[2048];
>strncpy(temp,"FirstValue",10);//Note this is necessary  because the
>array is not necessarily null terminated.
>temp[10]='\0';
>itk::EncapsulateMetaData<std::string>(thisDic,"ITK_FileOriginator",temp);
>
>strncpy(temp,"SecondValue",18);//Note this is necessary because the
>array is not necessarily null terminated.
>temp[18]='\0';
>itk::EncapsulateMetaData<std::string>(thisDic,"ITK_ImageFileBaseName",temp);
>}//End of scope
>=========================================================================
>
>Assume the char pointer temp is at memory location 0x1234
>
>The (a) method will result in a meta data dictionary will be two entries
><std::string="ITK_FileOriginator",SmartPoiner<MetaDataObjectBase>->GetMetaDataValue()=char *=0x1234>
><std::string="ITK_ImageFileBaseName",SmartPoiner<MetaDataObjectBase>->GetMetaDataValue()=char *=0x1234>
>Also note that the temp memory is not valid outside of this small scope.
>The (b) method will result in a meta data dictionary will be two entries
><std::string="ITK_FileOriginator",SmartPoiner<MetaDataObjectBase>->GetMetaDataValue()=std::string("FirstValue")>
><std::string="ITK_ImageFileBaseName",SmartPoiner<MetaDataObjectBase>->GetMetaDataValue()=std::string("SecondValue")>
>
>I have tested this under the SGI compilers, and the gcc 2.96 and gcc
>3.2, icc, compilers. 
>
>Thanks for looking into this.
>
>Thanks,
>Hans
>
>
>
>PS: More info.
>
>Valgrind Memory checker:
>
>gcc 2.96 compilers:
>-------------------------------------------------------------------------------------
>valgrind --leak-check=yes -v 
>/IPLlinux/robin+home/scratch/hjohnson/src/buildhome/athlon/DEBUG/bin/itkCommonTests itkMetaDataDictionaryTest
>...
>==18107== LEAK SUMMARY:
>==18107==    definitely lost: 0 bytes in 0 blocks.
>==18107==    possibly lost:   0 bytes in 0 blocks.
>...
>-------------------------------------------------------------------------------------
> valgrind  --leak-check=yes -v
>/IPLlinux/robin+home/scratch/hjohnson/src/buildhome/athlon/DEBUG/bin/itkIOTests itkAnalyzeImageIOTest
>...
>==18132== LEAK SUMMARY:
>==18132==    definitely lost: 0 bytes in 0 blocks.
>==18132==    possibly lost:   0 bytes in 0 blocks.
>...
>
>
>
>_______________________________________________
>Insight-developers mailing list
>Insight-developers@public.kitware.com
>http://public.kitware.com/mailman/listinfo/insight-developers