Permalink
Browse files

Avoid realloc(p, 0) due to standards breakage

Once upon a time, memory allocators made sense.  If you asked for zero
bytes of memory, you would get zero bytes of memory -- no questions
asked.  Those zero bytes of memory might be at NULL, which is arguably
a silly answer to a silly question, but that was fine; after all, you
can't access beyond the end of an allocation, so if the allocation is
for zero bytes you deserve what you get if you try to dereference a
NULL pointer.

This simple design meant that realloc(p, 0) was a synonym for free(p),
and realloc(NULL, N) was a synonym for malloc(N).  In fact, some
memory allocators took advantage of this and had just a single entry
point, for realloc, and defined malloc and free in terms of realloc.

Then someone came along and said "NULL should only be returned on
failure", and decided that their malloc would instead return unique
non-NULL pointers.  This resulted in a plethora of portability bugs
since it encouraged people to assume that malloc would only return
NULL on failure; but most people who made that mistake weren't
consistently checking for memory allocation failures anyway, so it
didn't hurt them.

The standards folks came along, and as usual when confronted with two
implementations, one sane and one insane, declared that both were
equally valid.

Then someone asked how this should work for realloc.  Remember, when
realloc fails, it does not free the input pointer; this is so that
programs which are designed to safely handle memory allocation failure
(i.e., not 99% of the code out there) can respond to it (say, by saving
the data to disk) rather than losing the data before being informed of
the allocation failure.

So what happens if realloc(p, 0) fails?  It's going to return NULL,
since that's what realloc does if it fails; but should it also free
the allocation p?  The sane answer is yes: The calling application
has explicitly said that it doesn't want that data, and since realloc
is returning NULL the application has no way to distinguish this from
a successful allocation of zero bytes.

Unfortunately, some people are not sane, and have decided that it's
equally valid for realloc(p, 0) to return NULL-meaning-failure and
*not free p*.  They argue that this prevents a double-free bug if the
application code is
    if ((foo = realloc(bar, 0)) == NULL)
        free(bar);
ignoring the fact that this is *already* a double-free bug on systems
where the C library opts for "return NULL when asked to allocate zero
bytes".  In other words, they introduce a memory leak in order to
avoid a double-free bug which they're in fact not avoiding anyway.

As a result, any portable C code which wants to avoid double-free bugs
and also avoid memory leaks *must* avoid calling realloc to resize an
allocation down to zero bytes.

tl;dr: -8 +24 work around idiotic C libraries
  • Loading branch information...
cperciva committed Jan 18, 2016
1 parent 909f3f5 commit cabe5fca76f6c38f872ea4a5967458e6f3bfe054
Showing with 24 additions and 8 deletions.
  1. +24 −8 datastruct/elasticarray.c
View
@@ -36,10 +36,18 @@ resize(struct elasticarray * EA, size_t nsize)
nalloc = EA->alloc;
}
/* Reallocate if necessary. */
if (nalloc != EA->alloc) {
nbuf = realloc(EA->buf, nalloc);
if ((nbuf == NULL) && (nalloc > 0))
/*
* Resizing to zero needs special handling due to some strange
* interpretations of what realloc(p, 0) should do if it fails
* to allocate zero bytes of memory.
*/
if (nalloc == 0) {
free(EA->buf);
EA->buf = NULL;
EA->alloc = 0;
} else if (nalloc != EA->alloc) {
/* Reallocate to a nonzero size if necessary. */
if ((nbuf = realloc(EA->buf, nalloc)) == NULL)
goto err0;
EA->buf = nbuf;
EA->alloc = nalloc;
@@ -204,10 +212,18 @@ elasticarray_truncate(struct elasticarray * EA)
{
void * nbuf;
/* If there is spare space, reallocate. */
if (EA->alloc > EA->size) {
nbuf = realloc(EA->buf, EA->size);
if ((nbuf == NULL) && (EA->size > 0))
/*
* Truncating down to zero needs special handling due to some strange
* interpretations of what realloc(p, 0) should do if it fails to
* allocate zero bytes of memory.
*/
if (EA->size == 0) {
free(EA->buf);
EA->buf = NULL;
EA->alloc = 0;
} else if (EA->alloc > EA->size) {
/* Reallocate to eliminate spare space if necessary. */
if ((nbuf = realloc(EA->buf, EA->size)) == NULL)
goto err0;
EA->buf = nbuf;
EA->alloc = EA->size;

9 comments on commit cabe5fc

@fanf2

This comment has been minimized.

Show comment
Hide comment
@fanf2

fanf2 Jan 18, 2016

A horrid problem.

I think the timeline in your rant is a bit off. The differing treatment of 0 allocations goes back to C89. Before C89, malloc and related functions were very badly specified, and realloc in particular was a disaster area - AIUI the straightforward correspondence between realloc and malloc and free was invented by the ANSI C committee. See for example the pre-standard man page http://www.tuhs.org/cgi-bin/utree.pl?file=4.3BSD-Reno/src/lib/libc/stdlib/malloc.3

The C standards (all versions) are quite clear that realloc(p,0) has to free p and return NULL; detecting failure of realloc is not completely trivial and that wrong example code should not be supported.

fanf2 replied Jan 18, 2016

A horrid problem.

I think the timeline in your rant is a bit off. The differing treatment of 0 allocations goes back to C89. Before C89, malloc and related functions were very badly specified, and realloc in particular was a disaster area - AIUI the straightforward correspondence between realloc and malloc and free was invented by the ANSI C committee. See for example the pre-standard man page http://www.tuhs.org/cgi-bin/utree.pl?file=4.3BSD-Reno/src/lib/libc/stdlib/malloc.3

The C standards (all versions) are quite clear that realloc(p,0) has to free p and return NULL; detecting failure of realloc is not completely trivial and that wrong example code should not be supported.

@KyleSanderson

This comment has been minimized.

Show comment
Hide comment
@KyleSanderson

KyleSanderson Jan 18, 2016

Unfortunately you seem to have forgotten that realloc preserves the existing malloc'd data, new allocation or not. Yes, the behaviour of 0 is really dumb. However, when you realloc, you're not throwing the data away, you're simply acquiring more memory.

KyleSanderson replied Jan 18, 2016

Unfortunately you seem to have forgotten that realloc preserves the existing malloc'd data, new allocation or not. Yes, the behaviour of 0 is really dumb. However, when you realloc, you're not throwing the data away, you're simply acquiring more memory.

@cperciva

This comment has been minimized.

Show comment
Hide comment
@cperciva

cperciva Jan 18, 2016

Member

@fanf2 I may well have gotten the timeline slightly off. Or perhaps my "once upon a time" was an intermediate point. The history really wasn't the point.

And the C11 standard is part of the problem. See in particular C11 TC2, which I've heard makes explicit that whether realloc(p, 0) frees p on failure is implementation-defined.

Member

cperciva replied Jan 18, 2016

@fanf2 I may well have gotten the timeline slightly off. Or perhaps my "once upon a time" was an intermediate point. The history really wasn't the point.

And the C11 standard is part of the problem. See in particular C11 TC2, which I've heard makes explicit that whether realloc(p, 0) frees p on failure is implementation-defined.

@cperciva

This comment has been minimized.

Show comment
Hide comment
@cperciva

cperciva Jan 18, 2016

Member

@KyleSanderson I haven't forgotten anything of the sort. That's exactly why I was using realloc. What on earth are you talking about?

Member

cperciva replied Jan 18, 2016

@KyleSanderson I haven't forgotten anything of the sort. That's exactly why I was using realloc. What on earth are you talking about?

@nwmcsween

This comment has been minimized.

Show comment
Hide comment
@nwmcsween

nwmcsween Jan 18, 2016

You have book keeping in all malloc implementations, in fact IMO it's bad / wrong to implicitly free on realloc(p, 0).

nwmcsween replied Jan 18, 2016

You have book keeping in all malloc implementations, in fact IMO it's bad / wrong to implicitly free on realloc(p, 0).

@cperciva

This comment has been minimized.

Show comment
Hide comment
@cperciva

cperciva Jan 18, 2016

Member

@nwmcsween You're entitled to your opinion. Your opinion is wrong.

Member

cperciva replied Jan 18, 2016

@nwmcsween You're entitled to your opinion. Your opinion is wrong.

@fanf2

This comment has been minimized.

Show comment
Hide comment
@fanf2

fanf2 Jan 19, 2016

@cperciva OK wow, I didn't read the current standard because I thought it was the same as C89 but I now discover that the equivalence realloc(p,0) === free(p) was present in C89 (including TC1 and TC2) but it was removed in C99. Awful.

fanf2 replied Jan 19, 2016

@cperciva OK wow, I didn't read the current standard because I thought it was the same as C89 but I now discover that the equivalence realloc(p,0) === free(p) was present in C89 (including TC1 and TC2) but it was removed in C99. Awful.

@fanf2

This comment has been minimized.

Show comment
Hide comment
@pgunn

This comment has been minimized.

Show comment
Hide comment
@pgunn

pgunn Jan 19, 2016

It would've been a lot saner to just state that having a second argument of 0 to realloc is illegal, people should never do it, and implementations can do random things (including aborting the program). It's bad practice to have such a different useful (but redundant) codepath result from calling a function with a magic numerical value (even if zero is often magical).

pgunn replied Jan 19, 2016

It would've been a lot saner to just state that having a second argument of 0 to realloc is illegal, people should never do it, and implementations can do random things (including aborting the program). It's bad practice to have such a different useful (but redundant) codepath result from calling a function with a magic numerical value (even if zero is often magical).

Please sign in to comment.