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
|