Commit Message
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(-)
-Archie
Comments
On Mon, 18 May 2015, Archie Cobbs wrote:
> 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:
Even if you don't do (c) old symbol version behaves as now, are you sure
about doing (a) no new symbol version, rather than (b) new symbol version,
both are aliases, so new binaries won't run with old glibc?
On Mon, May 18, 2015 at 8:59 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Mon, 18 May 2015, Archie Cobbs wrote:
>
>> 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:
>
> Even if you don't do (c) old symbol version behaves as now, are you sure
> about doing (a) no new symbol version, rather than (b) new symbol version,
> both are aliases, so new binaries won't run with old glibc?
Yes I also agree (b) would be safer... but being new here I don't know
how to create symbol 'aliases' so either someone else will have to do
it (or explain to me how :)
Thanks,
-Archie
@@ -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.
@@ -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);
@@ -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)
@@ -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)
@@ -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: