Re: [eigen] portable reallocation...

[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