On 03/25/2011 05:59 AM, Paolo Bonzini wrote: > On 03/24/2011 08:47 PM, Eric Blake wrote: >> No argument by me if we enforce saner semantics to xrealloc. > > What's the saner semantic? One results in double frees, the other in > memory leaks. I think I prefer the latter, but perhaps the best choice > is just to crash...
Hmm, thinking about this some more: xmalloc guarantees that the result will be non-NULL (otherwise we called xalloc_die). Prior to yesterday's patch, xrealloc(NULL, 0) was guaranteed to return a unique zero-size pointer (assuming GNU semantics), but now it returns NULL (which means that Paul's patch yesterday introduced a regression). Furthermore, xrealloc(p, 0) returns NULL (both before and after Paul's patch yesterday, although the behavior before depended on the underlying realloc behavior). This means that xrealloc(p,0) violates the x* premise of guaranteeing a non-NULL result (and calling xalloc_die on failure). Would anyone object to this patch? diff --git i/ChangeLog w/ChangeLog index 8988e31..0c2e87e 100644 --- i/ChangeLog +++ w/ChangeLog @@ -1,3 +1,9 @@ +2011-03-25 Eric Blake <[email protected]> + + xmalloc: never return NULL for xrealloc + * lib/xmalloc.c (xrealloc): Revert yesterday's change that broke + xrealloc(NULL,0), and reject xrealloc(p,0). + 2010-11-30 Reuben Thomas <[email protected]> posix-modules: say what it does. diff --git i/lib/xmalloc.c w/lib/xmalloc.c index 4589e7d..e7eeaff 100644 --- i/lib/xmalloc.c +++ w/lib/xmalloc.c @@ -46,18 +46,23 @@ xmalloc (size_t n) return p; } -/* Change the size of an allocated block of memory P to N bytes, - with error checking. */ +/* Change the size of an allocated block of memory P to N bytes, with + error checking. This version explicitly rejects non-NULL P + combined with N of 0, since the GNU realloc semantics of freeing P + and returning NULL interfere with the promise of x*alloc methods + never returning NULL. */ void * xrealloc (void *p, size_t n) { if (!n) { - /* The GNU and C99 realloc behaviors disagree here. Act like - GNU, even if the underlying realloc is C99. */ - free (p); - return NULL; + /* The GNU and C99 realloc behaviors disagree here. We prefer + GNU semantics, but unless p is also NULL, those semantics + would require returning NULL. */ + if (p) + xalloc_die (); + n = 1; } p = realloc (p, n); -- Eric Blake [email protected] +1-801-349-2682 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
