[PATCH v3] libio: asprintf should write NULL upon failure

[Solar Designer]

On Sat, Aug 03, 2024 at 06:31:01PM +0200, Solar Designer wrote:
> Cc: Alan Coopersmith who helped get this into POSIX.
> 
> On Sat, Aug 03, 2024 at 01:08:37PM +0200, Florian Weimer wrote:
> > * Solar Designer:
> > 
> > >> +++ b/manual/stdio.texi
> > >> @@ -2524,7 +2524,9 @@ Allocation}) to hold the output, instead of putting the output in a
> > >>  buffer you allocate in advance.  The @var{ptr} argument should be the
> > >>  address of a @code{char *} object, and a successful call to
> > >>  @code{asprintf} stores a pointer to the newly allocated string at that
> > >> -location.
> > >> +location.  Current versions of @theglibc{} write a null pointer to
> > >> +@samp{*@var{ptr}} upon failure, but this is not required by the
> > >> +standard, and previous versions did not modify the pointer value.
> > >
> > > This makes it sound as if previous versions always did not modify the
> > > pointer value, but e.g. 2.34 would sometimes leave the pointer value
> > > unmodified and other times reset it to NULL.  I suggest replacing "did
> > > not modify the pointer value." with "sometimes left the pointer value
> > > unmodified.  Callers are required to check the function's return
> > > value."
> > 
> > I reviewed the older implementations, and I think we can say this, which
> > I think is more helpful to the programmer:
> > 
> > ???
> > Current and future versions of @theglibc{} write a null pointer to
> > @samp{*@var{ptr}} upon failure.  To achieve similar behavior with
> > previous versions, initialize @samp{*@var{ptr}} to a null pointer before
> > calling @code{asprintf}.
> > ???
> > 
> > What do you think?  Based on my review, even in the older
> > implementations, we never wrote a non-null pointer, proceeded to free
> > it, and then returned failure.  All the reallocation work was kept track
> > of in internal pointers, and the ptr argument was only used after the
> > format string had been processed.
> 
> On one hand, I agree this is probably right for glibc - at least I'm not
> aware of a counter-example.
> 
> On the other hand, it isn't sound advice to give to people writing code
> that might run on non-glibc.  POSIX doesn't guarantee any of this.
> 
> We could maybe include a wording closer to POSIX, which says:
> 
> "For asprintf(), if memory allocation was not possible, or if some other
> error occurs, the function shall return a negative value, and the
> contents of the location referenced by ptr are undefined, but shall not
> refer to allocated memory."
> 
> where by "shall not refer to allocated memory" I think they actually
> mean that asprintf() does not leak memory on error.
> 
> My current suggestion is a slight extension of my previous one:
> 
> Current versions of @theglibc{} write a null pointer to
> @samp{*@var{ptr}} upon failure, but this is not required by the
> standard, and previous versions sometimes left the pointer value
> unmodified.  Code portable across @theglibc{} versions and other
> systems should rely solely on the function's return value as
> indicator of success or failure.  Negative return value means the
> call has failed, the pointer value is undefined, and no memory is
> allocated.

What's the status on this?  I didn't mean to delay getting the code
change merged by my comments on the documentation.

Alexander