[Xen-devel] Re: [PATCH V7 4/7] libxl, Introduce libxl__realloc.

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Thu, 21 Jul 2011, Ian Campbell wrote:

> On Wed, 2011-07-20 at 22:24 +0100, Anthony PERARD wrote:
> > Signed-off-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > ---
> >  tools/libxl/libxl_internal.c |   20 ++++++++++++++++++++
> >  tools/libxl/libxl_internal.h |    1 +
> >  2 files changed, 21 insertions(+), 0 deletions(-)
> >
> > diff --git a/tools/libxl/libxl_internal.c b/tools/libxl/libxl_internal.c
> > index e259278..527ebc5 100644
> > --- a/tools/libxl/libxl_internal.c
> > +++ b/tools/libxl/libxl_internal.c
> > @@ -102,6 +102,26 @@ void *libxl__calloc(libxl__gc *gc, size_t nmemb, 
> > size_t size)
> >      return ptr;
> >  }
> >
> > +void *libxl__realloc(libxl__gc *gc, void *ptr, size_t new_size)
> > +{
> > +    void *new_ptr = realloc(ptr, new_size);
>
> On failure realloc will return NULL but not free the old pointer, so I
> think in that case you will set alloc_ptrs[i] to NULL but not actually
> free the old pointer, hence leaking it.

Oops, I'll fix that, and just return new_ptr (without removing the pointer
from gc).

> I think you can just check for new_ptr == NULL and return NULL up front.
> Normally that would leak the old ptr but by leaving it in the gc array
> we avoid that pitfall.
>
> If new_size==0 then realloc behaves as free. I reckon you can just
> return NULL then too and allow the gc to clean up. Or you could outlaw
> such uses in this interface and abort(), that seems harsh however.

I think I will remove the pointer from gc, so libxl__realloc will behave
as expected and free stuff if asked.

> BTW libxl__free_all does cope correctly with NULLs mid-way through
> alloc_ptrs[] which did concern me initially.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel