Re: glibc strerrorname_np

[Jonny Grant <jg@jguk.org>]

On 06/11/2021 12:51, Adhemerval Zanella wrote:
> 
> 
> 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.

Could you dircet me to the ailist file you mention please.

>>
>> 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.
Many thanks for your reply. May I check, are even the _np functions set in stone once they are released? glibc strerrorname_np was released a year ago. My disto Ubuntu LTS doesn't yet have this new glibc release containing this function.

>>
>>>   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.
> 

I was reading the fmemopen ticket you shared. May I ask - could you direct me to a "compat symbol" description? I did search and came across libc_hidden_proto and see it in use in /glibc/include/stdio.h the patch commit note says update fmemopen to the new POSIX spec.

Kind regards
Jonny