Re: [ping][PATCH v2.1][BZ #15339] Avoid returning EAI_SYSTEM when there's a network error

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

On 05/21/2013 11:54 AM, Siddhesh Poyarekar wrote:
> On Tue, May 21, 2013 at 11:36:12AM -0400, Carlos O'Donell wrote:
>>
>> The code before this is:
>>
>>                   nss_gethostbyname3_r fct = NULL;
>>                   if (req->ai_flags & AI_CANONNAME)
>>                     /* No need to use this function if we do not look for
>>                        the canonical name.  The function does not exist in
>>                        all NSS modules and therefore the lookup would
>>                        often fail.  */
>>                     fct = __nss_lookup_function (nip, "gethostbyname3_r");
>>                   if (fct == NULL)
>>                     /* We are cheating here.  The gethostbyname2_r
>>                        function does not have the same interface as
>>                        gethostbyname3_r but the extra arguments the
>>                        latter takes are added at the end.  So the
>>                        gethostbyname2_r code will just ignore them.  */
>>                     fct = __nss_lookup_function (nip, "gethostbyname2_r");
>>
>>                   if (fct != NULL)
>>                     {
>>
>>
>>>>  		  else
>>>> -		    status = NSS_STATUS_UNAVAIL;
>>>> +		    {
>>>> +		      status = NSS_STATUS_UNAVAIL;
>>>> +		      /* Could not load any of the lookup functions.  Indicate
>>>> +		         an internal error if the failure was due to a system
>>>> +			 error other than the file not being found.  */
>>>> +		      if (errno != 0 && errno != ENOENT)
>>>> +			__set_h_errno (NETDB_INTERNAL);
>>>> +		    }
>>>>  		}
>>
>> The real function being called is gethostbyname3_r, but if that for any
>> reason returns NULL we try gethostbyname2_r, which might also fail.
>> Here it would seem to me that we set errno potentially based on the errors
>> detected in calling the backup function instead of saving the real error
>> from the function we wanted to call.
>>
>> It looks to me like we need to save errno after the failed call to
>> gethostbyname3_r and use that to set h_errno.
>>
> 
> Well it's not necessary.  It's possible that the sequence was
> gethostbyname4_r -> gethostbyname3_r -> gethostbyname2_r, i.e. we
> couldn't find implementations for any of those functions for either
> the same or different reasons - we'd see that with AF_UNSPEC.  Given
> that they're implemented as fallbacks of each other, I thought it
> would be natural to look at the last error instead of the first one in
> case all fails.

I guess you're probably right in that if they all failed, it's no more
or less correct to return the error code of the last function you
called.

Could you please add a one sentence comment about this?
e.g. We use errno from the last failed fallback.

Cheers,
Carlos.