public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: Rich Felker <dalias@libc.org>
To: Florian Weimer <fweimer@redhat.com>
Cc: "Rüdiger Sonderfeld" <ruediger@c-plusplus.de>, libc-alpha@sourceware.org
Subject: Re: [RFC][PATCH] Add reallocarray function.
Date: Mon, 01 Sep 2014 17:24:00 -0000	[thread overview]
Message-ID: <20140901172403.GD12888@brightrain.aerifal.cx> (raw)
In-Reply-To: <54049556.1090709@redhat.com>

On Mon, Sep 01, 2014 at 05:48:38PM +0200, Florian Weimer wrote:
> On 05/18/2014 10:32 PM, Rüdiger Sonderfeld wrote:
> >+/* Re-allocate the previously allocated block in PTR, making the new
> >+   block large enough for NMEMB elements of SIZE bytes each.  */
> >+/* __attribute_malloc__ is not used, because if realloc returns
> >+   the same pointer that was passed to it, aliasing needs to be allowed
> >+   between objects pointed by the old and new pointers.  */
> >+extern void *reallocarray (void *__ptr, size_t __nmemb, size_t __size)
> >+     __THROW __attribute_warn_unused_result__;
> 
> I'm not sure if this is still on the table, but experience shows
> that the realloc interface is error-prone for another reason: The
> straight way to write an a reallocation,
> 
>   ptr = realloc(ptr, new_size);
> 
> leads to a memory leak on error.  It would be less error-prone to
> have reallocarray to update the pointer directly on success, e.g.:
> 
>   if (reallocarray(&ptr, new_count, sizeof(T)) < 0) {
>     // handle error
>   }

No, allocation functions which take void** are an extremely bad idiom
because they encourage UB. It's rare that ptr actually has type void*
(usually it has type T* for whatever type T the array members have).
So to use an allocation function that takes void**, a correct program
must use a temporary pointer, as in:

void *tmp = ptr;
if (reallocarray(&tmp, new_count, sizeof(T)) < 0) { ... }
ptr = tmp;

Note that this is even more awkward than the current situation with
realloc. However, almost everyone will actually write:

if (reallocarray((void **)&ptr, new_countl sizeof(T)) < 0) { ... }

thereby invoking UB via an aliasing violation.

Also, reallocarray is already defined by OpenBSD and perhaps others
with a particular signature. Defining an incompatible function with
the same name is just asking for trouble. In particular, if any
program using the proposed GNU variant with your semantics, but links
to a library which provides its own replacement function with the BSD
semantics, the symbols will conflict and the wrong function will get
called.

> However, this cannot be implemented as a C function, only as a macro.

Perhaps you meant to solve the aliasing violation a macro? It's
possible with GNU C statement-expressions, but ugly, and not really
clean. Perhaps there's a way to do it without any nonstandard
extensions too, but I don't think this kind of ugly hack that LOOKS
LIKE an aliasing violation until you decipher the macro belongs in
glibc.

Rich

  reply	other threads:[~2014-09-01 17:24 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-18 21:05 Rüdiger Sonderfeld
2014-05-18 21:43 ` Paul Eggert
2014-05-18 21:50   ` [RFC][PATCH v2] " Rüdiger Sonderfeld
2014-05-19  4:27     ` Rüdiger Sonderfeld
2014-05-19 15:30     ` Joseph S. Myers
2014-05-20  4:35       ` Rich Felker
2014-05-20  8:17         ` Paul Eggert
2014-05-20  8:19         ` Andreas Schwab
2014-05-20 15:45           ` Paul Eggert
2014-05-20 20:47             ` Rich Felker
2014-05-20 20:56               ` Paul Eggert
2014-05-20 12:50       ` Rüdiger Sonderfeld
2014-05-20 14:18         ` Paul Eggert
2014-05-21 12:39           ` Rüdiger Sonderfeld
2014-05-22  5:45             ` Paul Eggert
2014-09-01 15:48 ` [RFC][PATCH] " Florian Weimer
2014-09-01 17:24   ` Rich Felker [this message]
2014-09-02  9:29     ` Florian Weimer
2014-09-02 13:03       ` Rich Felker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140901172403.GD12888@brightrain.aerifal.cx \
    --to=dalias@libc.org \
    --cc=fweimer@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=ruediger@c-plusplus.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).