This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: asprintf() issue
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Florian Weimer <fweimer at redhat dot com>, Joseph Myers <joseph at codesourcery dot com>
- Cc: Archie Cobbs <archie dot cobbs at gmail dot com>, libc-alpha at sourceware dot org, Michael Kerrisk-manpages <mtk dot manpages at gmail dot com>
- Date: Wed, 20 May 2015 00:30:23 -0400
- Subject: Re: asprintf() issue
- Authentication-results: sourceware.org; auth=none
- References: <CANSoFxt-cdc-+C4u-rTENMtY4X9RpRSuv+axDswSPxbDgag8_Q at mail dot gmail dot com> <55520F8F dot 9020308 at redhat dot com> <CANSoFxvac6_uBgwzWm5q6U+GcWzzKtDtDP0BVvE4eL08zXHs5Q at mail dot gmail dot com> <5552183C dot 2070809 at redhat dot com> <CANSoFxv7uO2Niq+wVKsC9xoDYuNgqHFxJnLrkgNqfKpFwzde=Q at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1505131601320 dot 30846 at digraph dot polyomino dot org dot uk> <555385F4 dot 5000409 at redhat dot com> <alpine dot DEB dot 2 dot 10 dot 1505131722190 dot 30846 at digraph dot polyomino dot org dot uk> <555432DE dot 1020608 at redhat dot com> <5559C31D dot 5070400 at redhat dot com>
On 05/18/2015 06:46 AM, Florian Weimer wrote:
> On 05/14/2015 07:30 AM, Carlos O'Donell wrote:
>> On 05/13/2015 01:24 PM, Joseph Myers wrote:
>>> On Wed, 13 May 2015, Carlos O'Donell wrote:
>>>
>>>> My preference is that we set it to NULL. This will aid in debugging as any
>>>> dereferences to NULL will immediately trap. Leaving the value unchanged
>>>> could result in further manipulation of an invalid memory location and
>>>> program corruption.
>>>
>>> If we do this, do we then want to
>>>
>>> (a) not have a new symbol version; or
>>>
>>> (b) have a new symbol version with the old version being an alias of the
>>> new (so that new binaries that may rely on it being set to NULL don't run
>>> with old glibc - similar to the symbol versioning of <fenv.h> functions
>>> whose return type changed from void to int in C99 TC1, for example); or
>>>
>>> (c) have a new symbol version with the old version not changing *ptr on
>>> error?
>>
>> 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).
I disagree that the compat symbol is not worth the cost.
Such a change stands to break binaries that were previously working, and in
glibc we stand by our community commitment not to break user code.
In this case it's harder to draw a clear line because the API is BSD-based
and our assurances are weaker, but we should not throw away backwards
compatibility without a serious discussion.
Yes, I agree it is easy to fix the code, but that should not be required
if you keep your binary and run it on a newer system.
The impact on linking and code size are minor, and if we want to fix that
we've had at least two patches to make dynamic linking significantly faster
by fixing the O(n^2) symbol sort algorithm in the core code.
> So it turns out the asprintf manual pages is correct as-is, because the
> manual pages have a tendency to describe more than just current GNU libc
> behavior.
Correctness is not all that relevant. The better question is what behaviour
is better for our users? Pedantically speaking saying it's undefined is always
correct because the implementation can do what it wants.
I think we should do the following:
(a) Rewrite the man page to say we set the pointer to NULL on error.
Document that this behaviour changed in glibc 2.22.
(b) Add a versioned function to support old binaries, and have the
new implementation set the pointer to NULL on error.
Then we're done and move on to the next problem.
Cheers,
Carlos.