From patchwork Mon May 18 13:14:19 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Archie Cobbs X-Patchwork-Id: 6783 Received: (qmail 77710 invoked by alias); 18 May 2015 13:14:45 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 77690 invoked by uid 89); 18 May 2015 13:14:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-ie0-f173.google.com X-Received: by 10.50.30.197 with SMTP id u5mr14480530igh.9.1431954880226; Mon, 18 May 2015 06:14:40 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <5559C31D.5070400@redhat.com> References: <55520F8F.9020308@redhat.com> <5552183C.2070809@redhat.com> <555385F4.5000409@redhat.com> <555432DE.1020608@redhat.com> <5559C31D.5070400@redhat.com> From: Archie Cobbs Date: Mon, 18 May 2015 08:14:19 -0500 Message-ID: Subject: Re: asprintf() issue To: Florian Weimer Cc: "Carlos O'Donell" , Joseph Myers , libc-alpha@sourceware.org, Michael Kerrisk-manpages On Mon, May 18, 2015 at 5:46 AM, Florian Weimer 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(-) -Archie diff --git a/ChangeLog b/ChangeLog index b7f3c61..03be78a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2015-05-18 Archie Cobbs + + * 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 * .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: