From: Adhemerval Zanella <adhemerval.zanella@linaro.org>
To: Jonny Grant <jg@jguk.org>, libc-alpha@sourceware.org
Subject: Re: glibc strerrorname_np
Date: Sat, 6 Nov 2021 09:51:06 -0300 [thread overview]
Message-ID: <4c43eab7-2ca8-1b90-5fb9-c84e26a96ef0@linaro.org> (raw)
In-Reply-To: <b642f9cf-55b1-6aaf-4fea-1fc19616aedd@jguk.org>
On 05/11/2021 19:23, Jonny Grant wrote:
>
>
> On 05/11/2021 13:01, Adhemerval Zanella wrote:
>>
>>
>> On 05/11/2021 08:51, Jonny Grant wrote:
>>>>
>>>
>>> Hi Adhemerval
>>>
>>> Thank you for your reply. Personally I understood an ABI break would be the return type, the name, or the parameters. But the proposed change is not so. Changing to return a string, should be fine.
>>
>> It is still an ABI break, code that checks NULL for invalid input will
>> stop to work.
>>
>>>
>>> ie, in relation to strerror() C99 and POSIX.1-2008 require the return value to be non-NULL. (my view is it is always better not to return a NULL from such string functions that could then cause a SEGV.
>>>
>>> strerror(1000) returns a string "Unknown error 1000"
>>>
>>> Better to simply align with glibc strerror() approach?
>>>
>>> Feels like there is still time to change it, as it is _np. Aligning with strerror(), or just "" as you had mentioned seems reasonable.
>>
>> I give you that it is indeed a better return code, and it is not a matter
>> of timing, but rather I don't think it really worth the ABI break and
>> the required code complexity to do so.
>>
>> It would require:
>>
>> 1. Change the strerrorname_np to return "" on invalid code.
>
> Please find attached the patch.
>
>> 2. Keep the compat symbol that returns NULL and add a compat symbol.
>> 3. Exports a new symbol with version on 2.35 with the new semantic
>> and update the ailist.
>
> May I check, why would a new symbol be needed? I'd expect it is only a change to strerrorname_np and any test code you have that presently checks for NULL return.
As I said before it is an ABI break, since users that check for invalid
errno against NULL will start to fail. For such change we *do need* all
the trouble of adding a compat symbol with current semantic.
>
>> 4. Update the documentation and sync with man-pages.
>
> The man-page update is minor, I could handle that.
>
>
>>> https://man7.org/linux/man-pages/man3/strerror.3.html
>>>
>>> It's common for some returns to change, eg glibc 2.13 changed strerror_r() behaviour to return the actual error code, as opposed to returning -1 and setting errno.
>>
>> And such change did got without burden and extra complexity. Just check
>> the multiple preprocessor checks it requires to get the right definition
>> depending of the system support on the misc/error.c (imported from gnulib).
>>
>
> Ok, I think the change I propose does not affect the definition, as the function signature is the same. Maybe I misunderstand something.
The ABI break is not only for function signature and input alignment/size,
but also for function semantic. Just check the fmemopen
(fdb7d390dd0d96e4a8239c46f3aa64598b90842b), where we kept the old buggy
implementation since even when it is not POSIX compliant because we do
not know if users do depend of such behavior.
next prev parent reply other threads:[~2021-11-06 12:51 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-04 20:23 Jonny Grant
2021-11-04 20:46 ` Adhemerval Zanella
2021-11-04 22:52 ` Jonny Grant
2021-11-04 23:28 ` Adhemerval Zanella
2021-11-05 11:51 ` Jonny Grant
2021-11-05 13:01 ` Adhemerval Zanella
2021-11-05 22:23 ` Jonny Grant
2021-11-06 12:51 ` Adhemerval Zanella [this message]
2021-11-07 17:37 ` Zack Weinberg
2021-11-08 13:56 ` Adhemerval Zanella
2021-11-08 18:42 ` Zack Weinberg
2021-11-08 18:52 ` Adhemerval Zanella
2021-11-08 19:56 ` Florian Weimer
2021-11-08 20:28 ` Adhemerval Zanella
2021-11-08 22:22 ` Jonny Grant
2021-11-09 12:30 ` Adhemerval Zanella
2021-11-09 23:01 ` Jonny Grant
2021-11-10 1:37 ` Adhemerval Zanella
2021-11-08 8:36 ` Florian Weimer
2021-11-08 13:36 ` Jonny Grant
2021-11-08 13:42 ` Florian Weimer
2021-11-08 22:14 ` Jonny Grant
2021-11-08 14:01 ` Adhemerval Zanella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4c43eab7-2ca8-1b90-5fb9-c84e26a96ef0@linaro.org \
--to=adhemerval.zanella@linaro.org \
--cc=jg@jguk.org \
--cc=libc-alpha@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).