[Insight-developers] Roll back MetaDataObject and Dicitonary changes

[Hans J. Johnson]

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.
...