[Insight-developers] Small modification to ImportImageContainer

Luis Ibanez luis.ibanez at kitware.com
Thu Apr 2 11:31:26 EDT 2009


Hi Tom,

Thanks for committing the patch.

Yeap,
the AllocateManagedMemory() method suggestion
was intended simply to have symmetry. It is
not really necessary.


     Thanks


        Luis


----------------------
Tom Vercauteren wrote:
> Thanks Luis,
> 
> I have committed the patch:
> http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Common/itkImportImageContainer.h?root=Insight&r1=1.22&r2=1.23&sortby=date
> http://www.itk.org/cgi-bin/viewcvs.cgi/Code/Common/itkImportImageContainer.txx?root=Insight&r1=1.20&r2=1.21&sortby=date
> and the unit test
> http://www.itk.org/cgi-bin/viewcvs.cgi/Testing/Code/Common/itkFactoryTestLib.cxx?root=Insight&r1=1.4&r2=1.5&sortby=date
> 
> However i haven't implemented your suggestion
> (AllocateManagedMemory()) since it could only be called once with the
> Reserve function. The other places where AllocateElements is called
> are a bit more evolved, e.g. a memcpy is called between
> AllocateElements and the assignment of m_ImportPointer.
> 
> Tom
> 
> On Wed, Apr 1, 2009 at 19:40, Luis Ibanez <luis.ibanez at kitware.com> wrote:
> 
>>Hi Tom,
>>
>>Thanks a lot for sending the patch.
>>
>>This version look a lot better.
>>
>>The new implementation of the DeallocateManagedMemory()
>>has a better encapsulation.
>>
>>
>>It actually would be nice to have a symmetric method:
>>
>>             AllocateManagedMemory(),
>>
>>and to package in it the calls to:
>>
>>  {
>>   m_ImportPointer = this->AllocateElements(size);
>>   m_Capacity = size;
>>   m_Size = size;
>>   m_ContainerManageMemory = true;
>>   this->Modified();
>>   }
>>
>>that are repeated in Reserve().
>>
>>
>>but this could be part of a second commit...
>>
>>
>>I have no objections to the patch.
>>
>>
>>      Thanks
>>
>>
>>             Luis
>>
>>
>>----------------------------------------------------
>>On Tue, Mar 31, 2009 at 1:37 PM, Tom Vercauteren
>><tom.vercauteren at m4x.org> wrote:
>>
>>>Hi,
>>>
>>>Sorry for being so noisy on this topic.
>>>
>>>I have implemented a new version of ImportImageContainer. All the
>>>deallocation takes place within
>>> virtual void DeallocateManagedMemory();
>>>
>>>Here is what this function does:
>>> if (m_ImportPointer && m_ContainerManageMemory) delete [] m_ImportPointer;
>>> m_ImportPointer = 0; m_Capacity = 0; m_Size = 0;
>>>
>>>I have also put the members of ImportImageContainer back into the
>>>private section and added some protected set accessors for them.
>>>
>>>The attached patch provides all these modifications together with an
>>>enhanced  itkObjectFactoryTest2 unit test. itkFactoryTestLib.cxx
>>>(which is used by itkObjectFactoryTest2) implements
>>>TestImportImageContainer, a class that derives from
>>>ImportImageContainer but uses a std::allocator instead of using new
>>>and delete .
>>>
>>>Let me know, if such a patch seems more appropriate.
>>>
>>>Regards,
>>>Tom
>>>
>>
> 


More information about the Insight-developers mailing list