Re: [PATCH] getnameinfo: Do not restore errno on error

[Carlos O'Donell <carlos at redhat dot com>]

On 03/08/2016 11:46 AM, Florian Weimer wrote:
> On 03/08/2016 04:33 PM, Zack Weinberg wrote:
>> On Tue, Mar 8, 2016 at 10:20 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>> On 03/08/2016 04:14 PM, Zack Weinberg wrote:
>>>> On Tue, Mar 8, 2016 at 9:27 AM, Florian Weimer <fweimer@redhat.com> wrote:
>>>>> POSIX does not require it, and this behavior is not documented
>>>>> in the manual page, either.
>>>>
>>>> This might be OK in the actual error case, but you're stomping on
>>>> errno in the *non*-error case too, which, even if allowed, should be
>>>> avoided as a matter of QoI.
>>>
>>> We currently do not have this as a general goal for glibc functions.
>>
>> ... well, maybe we *should*.
> 
> I doubt it.  We support interposition, so this will never be very
> portable even within GNU.
> 
>>> I don't think getnameinfois special so that an exception is warranted (
>>> (unlike, say, free).
>>
>> It's special in that it currently *does* preserve errno on success, so
>> taking that out is a step in the wrong direction.
> 
> Why?
> 
> I doubt applications which rely on a preserved errno value exist, and if
> they do so, they are not portable, and can even break when tested with
> cwrap/nss_wrapper.

I agree with Andreas and Florian.

POSIX (Issue 7) says:
~~~
The value of errno should only be examined when it is indicated to be 
valid by a function's return value.
~~~
(would be better if it said "must only")

Which means that after a failed call to getnameinfo with NI_NAMEREQD
which returns EAI_NONAME, the value of errno cannot be examined.

The user can't do anything useful with the value, so we have no need
to restore it.

If we were to broadly accept that all functions should not modify
errno on success and on failures that don't document it being valid,
then we would be saving and restoring errno in almost all functions.

This is an unacceptable performance burden that isn't portable.
It also isn't clear what use cases this would help developers with.

Florian's patch looks good to me.

-- 
Cheers,
Carlos.