On Thu, Jun 19, 2008 at 10:49:59AM +0200, Chris Lalancette wrote:
Hello,
For 0.4.3, danpb's new memory management scheme went into libvirt. This is
fine, except that is subtly alters the semantics of malloc(), calloc(), and
realloc(). In particular, if you say:
foo = malloc(0);
glibc will happily return a non-NULL pointer to you. However, with the new
memory management stuff, if you say:
foo = VIR_ALLOC(0);
you will actually get a NULL pointer back. Personally, I think this is a
dangerous deviation from malloc() semantics that everyone is used to, and is
indeed causing problems with the remote driver. The short of it is that the
remote driver allocates memory on behalf of the remote side using VIR_ALLOC_N,
and this call is returning NULL so that the NULL checks elsewhere in the code
fire and return failure.
The attached patch fixes this situation by removing the 0 checks from the memory
allocation paths, and just lets them fall through to the normal malloc(),
calloc(), or realloc() routines, restoring old semantics.
Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
Agreed, it's a problem, +1, but
since Dan explicitely made the == 0 test to return NULL he probably
had a purpose about this (I suspect detecting 0 sized memory allocations).
So i would not apply the patch before he had a chance to comment on it,
but yes I personally thing we should reverse that bit. A zero lenght
allocation is legal, and may be later extended with realloc. This can
lead to vastly simplified code (to the expense of a seemingly useless
initial allocation).
Daniel
--
Red Hat Virtualization group
http://redhat.com/virtualization/
Daniel Veillard | virtualization library
http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit
http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine
http://rpmfind.net/