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 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


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