This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC][PATCH] Add reallocarray function.


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]