On 05/16/2013 02:06 AM, Siddhesh Poyarekar wrote:
Hi,
A patch I had pushed into 2.16 had an error which resulted in the regression described in the aforementioned bug as well as the bug described in bz @15339. Attached patch fixes this. If it looks OK, I'll apply it in rawhide and f19. I have also posted it upstream:
This class of change is interesting, in that nobody can easily review it because it was a test-driven change. The patch adjusts the code in response to feedback from tests run by users with observed versus expected output being adjusted.
Reviewing the code is easy, it looks right, it isn't invalid C, and it's only three lines of change.
However, the existing implementation is so complex and subtle that it would take me days to review this code well.
What would help in the review of this, and the upstream review, is to have a detailed breakdown of the analysis you did while fixing this.
Do you have such an analysis?
I wish we had some white-box testing for these internals, similar to the testing I did for gettext with systemtap where you inject a failure on purpose.
We need to look at better testing for all of the networking interfaces for glibc.
http://sourceware.org/ml/libc-alpha/2013-05/msg00567.html
Siddhesh
commit 69cbe0434e5ddf223ed17141ae9527fb83fde1fa Author: Siddhesh Poyarekar siddhesh@redhat.com Date: Wed May 15 20:06:20 2013 +0530
Set EAI_SYSTEM only when h_errno is NETDB_INTERNAL Fixes BZ #15339. NSS_STATUS_UNAVAIL may mean that a necessary input resource is not available. This could occur in a number of cases including when the network is down, system runs out of file descriptors, etc. The correct differentiator in such a case is the h_errno, which gives the nature of failure. In case of failures other than a simple 'not found', we set h_errno as NETDB_INTERNAL and let errno be the identifier for the exact error.
diff --git a/nss/getXXbyYY_r.c b/nss/getXXbyYY_r.c index 1067744..9e4d110 100644 --- a/nss/getXXbyYY_r.c +++ b/nss/getXXbyYY_r.c @@ -284,11 +284,11 @@ done: #endif *result = status == NSS_STATUS_SUCCESS ? resbuf : NULL; #ifdef NEED_H_ERRNO
- if (status == NSS_STATUS_UNAVAIL)
- /* Either we failed to lookup the functions or the functions themselves
had a system error. Set NETDB_INTERNAL here to let the caller know
that the errno may have the real reason for failure. */
*h_errnop = NETDB_INTERNAL;
- if (status == NSS_STATUS_UNAVAIL && !any_service && errno != ENOENT)
- /* This happens when we weren't able to use a service for reasons other
than the module not being found. In such a case, we'd want to tell the
caller that errno has the real reason for failure. */
- *h_errnop = NETDB_INTERNAL; else if (status != NSS_STATUS_SUCCESS && !any_service) /* We were not able to use any service. */ *h_errnop = NO_RECOVERY;
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c index d368306..cdd869d 100644 --- a/sysdeps/posix/getaddrinfo.c +++ b/sysdeps/posix/getaddrinfo.c @@ -1035,7 +1035,14 @@ gaih_inet (const char *name, const struct gaih_service *service, } } else
status = NSS_STATUS_UNAVAIL;
{
status = NSS_STATUS_UNAVAIL;
/* Could not load any of the lookup functions. Indicate
an internal error if the file was found but some other
error led to the failure. */
if (errno != 0 && errno != ENOENT)
__set_h_errno (NETDB_INTERNAL);
}
}
if (nss_next_action (nip, status) == NSS_ACTION_RETURN)
@@ -1049,7 +1056,7 @@ gaih_inet (const char *name, const struct gaih_service *service,
_res.options |= old_res_options & RES_USE_INET6;
if (status == NSS_STATUS_UNAVAIL)
if (h_errno == NETDB_INTERNAL) { result = GAIH_OKIFUNSPEC | -EAI_SYSTEM; goto free_and_return;
glibc mailing list glibc@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/glibc