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.

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

Cheers, Jonny