Re: [eigen] portable reallocation...

[ Thread Index | Date Index | More lists.tuxfamily.org/eigen Archives ]


Thanks, good catch.
Benoit

2010/2/27 Eamon Nerbonne <emn13@xxxxxxxxxxxx>:
> Hi all,
> Minor nitpick, in Memory.h line 100 ei_mm_alloc is missing an inline
> keyword; I'm seeing "multiple definition" errors.
>
> --eamon@xxxxxxxxxxxx - Tel#:+31-6-15142163
>
>
> On Sat, Feb 27, 2010 at 17:40, Hauke Heibel <hauke.heibel@xxxxxxxxxxxxxx>
> wrote:
>>
>> On Sat, Feb 27, 2010 at 4:26 PM, Benoit Jacob <jacob.benoit.1@xxxxxxxxx>
>> wrote:
>> > You patch looks good globally.
>> >
>> > A few comments:
>> >
>> > 1)
>> >
>> > +  // Comment from Hauke: IIRC, 'if (ptr)' is not required - you are
>> > free to call free(0)
>> >   if(ptr)
>> >     std::free(*(reinterpret_cast<void**>(ptr) - 1));
>> >
>> > but if ptr==0 then here we're dereferencing the 0 pointer, that's
>> > what's illegal. It's precisely to allow to do ei_aligned_free(0) that
>> > we have to do this if(ptr) here.
>>
>> I should have seen the -1 ... it is crystal clear that this does not work.
>>
>> > 2)
>> >
>> > Naming nitpicks:
>> >
>> > conservative_resize: I actually agree that it looks better than
>> > conservativeResize, but we have this convention about capitalization,
>> > and following it consistently has the merit of making it easier for
>> > the user to remember the names.
>> >
>> > eigen_mm_realloc:  why not ei_mm_realloc ?  like elsewhere in eigen.
>> >
>> > 3)
>> >
>> > Finally: is this well covered by existing unit tests? (Just checking)
>>
>> All done and I think this test has full coverage.
>>
>> - Hauke
>>
>>
>
>



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/