Re: asprintf() issue

[Archie Cobbs <archie dot cobbs at gmail dot com>]

On Mon, May 18, 2015 at 5:46 AM, Florian Weimer <fweimer@redhat.com> wrote:
> > IMO we should be conservative and do (c), and document in NEWS, Release wiki
> > page, and hopefully the manual.
>
> I don't think this is worth the cost.  (Even such little changes add up
> and eventually impact linking time and code size.)  It does not even fix
> a bug, and application code can easily set *ptr to NULL before calling
> asprintf, to get uniform behavior across all known implementations (if
> that simplifies application code).

This makes sense to me as well. What about this then:

 ChangeLog             | 7 +++++++
 argp/argp-help.c      | 6 ++----
 debug/vasprintf_chk.c | 6 +++++-
 libio/vasprintf.c     | 6 +++++-
 manual/stdio.texi     | 7 +++++++
 5 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index b7f3c61..03be78a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2015-05-18  Archie Cobbs <archie.cobbs@gmail.com>
+
+    * libio/vasprintf.c: Set pointer to NULL on error.
+    * debug/vasprintf_chk.c: Likewise.
+    * argp/argp-help.c: Expect NULL on error from _IO_vasprintf().
+    * manual/stdio.texi: Specify that *asprintf() sets NULL on error.
+
 2015-05-18  Siddhesh Poyarekar  <siddhesh@redhat.com>

     * .gitignore: Ignore generated *.pyc.
diff --git a/argp/argp-help.c b/argp/argp-help.c
index b055e45..8705b76 100644
--- a/argp/argp-help.c
+++ b/argp/argp-help.c
@@ -1769,8 +1769,7 @@ __argp_error (const struct argp_state *state,
const char *fmt, ...)
 #ifdef _LIBC
       char *buf;

-      if (_IO_vasprintf (&buf, fmt, ap) < 0)
-        buf = NULL;
+      _IO_vasprintf (&buf, fmt, ap);

       __fxprintf (stream, "%s: %s\n",
               state ? state->name : __argp_short_program_name (), buf);
@@ -1839,8 +1838,7 @@ __argp_failure (const struct argp_state *state,
int status, int errnum,
 #ifdef _LIBC
           char *buf;

-          if (_IO_vasprintf (&buf, fmt, ap) < 0)
-        buf = NULL;
+          _IO_vasprintf (&buf, fmt, ap);

           __fxprintf (stream, ": %s", buf);

diff --git a/debug/vasprintf_chk.c b/debug/vasprintf_chk.c
index 8ecb9e8..a85d069 100644
--- a/debug/vasprintf_chk.c
+++ b/debug/vasprintf_chk.c
@@ -47,7 +47,10 @@ __vasprintf_chk (char **result_ptr, int flags,
const char *format,
      we know we will never seek on the stream.  */
   string = (char *) malloc (init_string_size);
   if (string == NULL)
-    return -1;
+    {
+      *result_ptr = NULL;
+      return -1;
+    }
 #ifdef _IO_MTSAFE_IO
   sf._sbf._f._lock = NULL;
 #endif
@@ -67,6 +70,7 @@ __vasprintf_chk (char **result_ptr, int flags, const
char *format,
   if (ret < 0)
     {
       free (sf._sbf._f._IO_buf_base);
+      *result_ptr = NULL;
       return ret;
     }
   /* Only use realloc if the size we need is of the same (binary)
diff --git a/libio/vasprintf.c b/libio/vasprintf.c
index 7f9c105..a423409 100644
--- a/libio/vasprintf.c
+++ b/libio/vasprintf.c
@@ -49,7 +49,10 @@ _IO_vasprintf (result_ptr, format, args)
      we know we will never seek on the stream.  */
   string = (char *) malloc (init_string_size);
   if (string == NULL)
-    return -1;
+    {
+      *result_ptr = NULL;
+      return -1;
+    }
 #ifdef _IO_MTSAFE_IO
   sf._sbf._f._lock = NULL;
 #endif
@@ -63,6 +66,7 @@ _IO_vasprintf (result_ptr, format, args)
   if (ret < 0)
     {
       free (sf._sbf._f._IO_buf_base);
+      *result_ptr = NULL;
       return ret;
     }
   /* Only use realloc if the size we need is of the same (binary)
diff --git a/manual/stdio.texi b/manual/stdio.texi
index e407170..b9ea0ae 100644
--- a/manual/stdio.texi
+++ b/manual/stdio.texi
@@ -2551,6 +2551,13 @@ The return value is the number of characters
allocated for the buffer, or
 less than zero if an error occurred.  Usually this means that the buffer
 could not be allocated.

+If an error occurs, *@var{ptr} is set to @code{NULL}.
+
+@strong{Compatibility Note:} In versions of @theglibc{} prior to 2.22,
+*@var{ptr} is unmodified if an error occurs.  To ensure consistent
+behavior across all versions, callers should initialize *@var{ptr}
+to NULL prior to invoking {@code asprintf}.
+
 Here is how to use @code{asprintf} to get the same result as the
 @code{snprintf} example, but more easily:


-Archie

-- 
Archie L. Cobbs