Re: [Xen-devel] [PATCH 08/13 v5] libxl: don't leak ptr in libxl_list_vm error case [and 1 more messages]

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Andrew Cooper writes ("Re: [PATCH 08/13 v5] libxl: don't leak ptr in 
libxl_list_vm error case [and 1 more messages]"):
> On 13/12/2013 16:52, Ian Jackson wrote:
> > I think so.  I think they should promise that if you pass size==0 you
> > get a non-null pointer.  Calling realloc with size==0 should crash.
> 
> Can we not?
> 
> Having a non-NULL pointer to a 0 length buffer is madness, whose use
> should not be further encouraged.

I don't know where you get the idea that such a pointer is "madness".
It's a permitted response from malloc() according to the spec and
indeed it's what glibc's malloc does!

> Furthermore, code which ends calling libxl__*alloc() with a size of 0
> *is* buggy, and should suffer an abort(),

Calling malloc(0) is certainly not buggy.  It has behaviour which is
clearly defined by the spec, and if you are careful with your error
handling it is possible to use it correct.  It's just a bit awkward.

libxl__*alloc should have an interface at least as capable as malloc()
- that is, libxl__zalloc(,0) should be fine.

Or to put it another way calling libxl_*alloc(,0) is only buggy
because we've made it be buggy (or defined it to be so).

I wouldn't mind if libxl_*alloc(,0) succeeded and returned NULL, but I
think having them return non-NULL is probably better.

> just as much as attempting to realloc to a size of 0.

realloc(,0) is problematic in the standard C API because it's not
possible to tell, if you get NULL back, whether the original block was
freed.

The problem with libxl__realloc(,,0) is that it's not clear whether
the caller intends to free the block, or resize it to size 0.  I think
none of our use patterns will end up calling it.  But I am open to
arguments that libxl__realloc(,,0) should be defined to succeed and
return NULL, or to succeed and return non-NULL.


But since this is controversial I should probably just apply Matthew's
patch, which is a clear improvement, while we argue about it.  So:

Committed-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel