[ITK-dev] #include <nifti1_io.h> leaking from itkNiftiImageIO.h into user code (+ 3 ways to fix it)

[Lowekamp, Bradley (NIH/NLM/LHC) [C]]

Hello,

I looked into this a little the other day. I tried #2, but it did not work due to the way `nifty_image` is a typedef an anonymous structure.

Your #3 approach looks good to me. I look forward to seeing the patch.

You can also make ITKNIFTI a private dependency for the module by undoing the following:
https://github.com/InsightSoftwareConsortium/ITK/commit/67e308fc8470c8119323a8954d53c777966c5ad5


On 6/22/17, 10:18 AM, "Niels Dekker" <niels-xtk at xs4all.nl> wrote:

    We're glad to see our niftilib-name-mangling patch made it into the 
    master branch: 
    https://github.com/InsightSoftwareConsortium/ITK/commit/370c3116d3edac3a3bc36e03e1b1b655df0f04af
    
    Now we do have another nifty ITK issue! itkNiftiImageIO.h has an 
    '#include <nifti1_io.h>', which leaks into user code:
    https://github.com/InsightSoftwareConsortium/ITK/blob/61e8029ef53e2d37cfa3c5bad999e582326e36ee/Modules/IO/NIFTI/include/itkNiftiImageIO.h#L26
    Our project uses both ITK and the NiftyReg library, and thereby 
    indirectly uses two different versions of the niftilib library, with 
    different versions of <nifti1_io.h>, so we foresee potential conflicts 
    in the current situation. itkNiftiImageIO.h only needs <nifti1_io.h> for 
    its 'nifti_image' typedef, as it has a private member, 
    itk::NiftiImageIO::m_NiftiImage of type 'nifti_image *'. So basically 
    <nifti1_io.h> is just an implementation detail of itk::NiftiImageIO. It 
    is not part of the public interface of the ITK class.
    
    Do you think it would be feasible to move '#include <nifti1_io.h>' from 
    itkNiftiImageIO.h to its CXX file, itkNiftiImageIO.cxx?
    
    We can think of three different ways to achieve this:
    
      1. The void pointer approach: Declare itk::NiftiImageIO::m_NiftiImage 
    as a 'void *', instead of 'nifti_image *', and cast m_NiftiImage to 
    'nifti_image *' within itkNiftiImageIO.cxx, whenever needed. I would 
    expect more than 200 casts, doing static_cast< nifti_image 
    *>(this->m_NiftiImage).
    
      2. The Pimpl way : Move the declaration of m_NiftiImage, and possibly 
    any other data member of itk::NiftiImageIO, from itkNiftiImageIO.h to 
    itkNiftiImageIO.cxx, according to the Pimpl idiom 
    http://wiki.c2.com/?PimplIdiom  All access to these data members should 
    then be adapted accordingly, so this would result in many modifications 
    of existing code in itkNiftiImageIO.cxx. Optionally, private member 
    function declarations could also be moved from itkNiftiImageIO.h to 
    itkNiftiImageIO.cxx this way. It seems like a lot of work.
    
      3. The proxy approach: Declare itk::NiftiImageIO::m_NiftiImage as a 
    reference to a proxy, which behaves like a 'nifti_image *'. This way, no 
    existing lines of code from itkNiftiImageIO.cxx would need to be 
    changed, except for the initialization of m_NiftiImage. The proxy class 
    should be forward-declared in itkNiftiImageIO.h, and defined in 
    itkNiftiImageIO.cxx, for example as follows:
    
    	class NiftiImageProxy
    	{
    	  nifti_image* m_ptr;
    	public:
    	  NiftiImageProxy(nifti_image* ptr) ; // Converts pointer to proxy.
    	  operator nifti_image*();  // Converts proxy to pointer.
    	  nifti_image* operator->();
    	};
    
    We did fully implement and test the proxy approach, and we can make a 
    patch, if you think that's an interesting option for ITK. What do you 
    think?
    
    Kind regards, also on behalf of my colleague Floris Berendsen,
    
         Niels
    -- 
    Niels Dekker
    Scientific programmer
    LKEB, Leiden University Medical Center, Netherlands
    _______________________________________________
    Powered by www.kitware.com
    
    Visit other Kitware open-source projects at
    http://www.kitware.com/opensource/opensource.html
    
    Kitware offers ITK Training Courses, for more information visit:
    http://kitware.com/products/protraining.php
    
    Please keep messages on-topic and check the ITK FAQ at:
    http://www.itk.org/Wiki/ITK_FAQ
    
    Follow this link to subscribe/unsubscribe:
    http://public.kitware.com/mailman/listinfo/insight-developers