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: Tue, 02 Sep 2014 13:03:00 -0000	[thread overview]
Message-ID: <20140902130251.GK12888@brightrain.aerifal.cx> (raw)
In-Reply-To: <54058AD5.2010304@redhat.com>

On Tue, Sep 02, 2014 at 11:16:05AM +0200, Florian Weimer wrote:
> On 09/01/2014 07:24 PM, Rich Felker wrote:
> 
> >>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.
> 
> That's why I wrote that reallocarray has to be a macro.
> 
> On the other hand, I'm not particularly worried about the aliasing
> violation because according to one reading of the standard, realloc
> returns a pointer to untyped (but partially initialized) memory,
> which needs out-of-language support anyway.

This is a non-sequitur. Even if "realloc needs out-of-language
support" (a topic I don't want to mix with this discussion), that
would not be a reason to preclude unrelated compiler optimization. The
fact that T* and void* cannot alias is unrelated to anything about the
contents of the member realloc returns.

> >Also, reallocarray is already defined by OpenBSD and perhaps others
> >with a particular signature.
> 
> We'd have to give the fixed version a separate name, to avoid
> another strerror_r-like fiasco.

I think it's much better just to avoid it. Like I said the "fix"
actually makes a worse interface that's clunkier to use. And if the
caller is just going to abort when reallocarray fails (this is very
bad practice, but it's common GNU practice) then there's no point in
using a temporary anyway. Direct assignment works fine.

Rich

      reply	other threads:[~2014-09-02 13:03 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
2014-09-02  9:29     ` Florian Weimer
2014-09-02 13:03       ` Rich Felker [this message]

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=20140902130251.GK12888@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).