Re: [PATCH][BZ #14719] Return EAI_SYSTEM from getaddrinfo if we runout of fds

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

On Tue, 23 Oct 2012 14:01:48 +0200, Andreas wrote:
> This line is to long, please wrap at 78 characters.

Done.
 
> > +	case EMFILE:
> > +	case ENFILE:
> > +	  h_errno = NETDB_INTERNAL;
> 
> No break? In that case, please document that this is intented, e.g.
> say "Fall through"

It's intentional.  Added comments in both instances in the updated
patch.

> > +	case ECONNREFUSED:
> > +	case ETIMEDOUT:
> > +	  status = NSS_STATUS_UNAVAIL;
> > +	  break;
> > +	default:
> > +	  status = NSS_STATUS_NOTFOUND;
> > +	  break;
> 
> You're setting it to UNAVAIL also with ETIMEDOUT which wasn't done
> before. Is that correct? It seems to mirror the conditions above

Yes, that is intentional too since this is an incorrect difference in
the _nss_dns_gethostbyname3 and _nss_dns_gethostbyname4 implementations.

Here's an updated patch with fixes based on your suggestions.

Regards,
Siddhesh

diff --git a/nss/getXXbyYY_r.c b/nss/getXXbyYY_r.c
index f296ed1..b5fe4a2 100644
--- a/nss/getXXbyYY_r.c
+++ b/nss/getXXbyYY_r.c
@@ -284,8 +284,13 @@ done:
 #endif
   *result = status == NSS_STATUS_SUCCESS ? resbuf : NULL;
 #ifdef NEED_H_ERRNO
-  if (status != NSS_STATUS_SUCCESS && ! any_service)
-    /* We were not able to use any service.  */
+  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;
+  else if (status != NSS_STATUS_SUCCESS && !any_service)
+    /* We were no able to use any service.  */
     *h_errnop = NO_RECOVERY;
 #endif
 #ifdef POSTPROCESS
diff --git a/resolv/nss_dns/dns-host.c b/resolv/nss_dns/dns-host.c
index 6b62c05..f78b879 100644
--- a/resolv/nss_dns/dns-host.c
+++ b/resolv/nss_dns/dns-host.c
@@ -199,6 +199,11 @@ _nss_dns_gethostbyname3_r (const char *name, int af, struct hostent *result,
 	  status = NSS_STATUS_TRYAGAIN;
 	  h_errno = TRY_AGAIN;
 	  break;
+	/* System has run out of file descriptors.  */
+	case EMFILE:
+	case ENFILE:
+	  h_errno = NETDB_INTERNAL;
+	  /* Fall through.  */
 	case ECONNREFUSED:
 	case ETIMEDOUT:
 	  status = NSS_STATUS_UNAVAIL;
@@ -311,14 +316,26 @@ _nss_dns_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat,
 			      &ans2p, &nans2p, &resplen2);
   if (n < 0)
     {
-      if (errno == ESRCH)
+      switch (errno)
 	{
+	case ESRCH:
 	  status = NSS_STATUS_TRYAGAIN;
 	  h_errno = TRY_AGAIN;
+	  break;
+	/* System has run out of file descriptors.  */
+	case EMFILE:
+	case ENFILE:
+	  h_errno = NETDB_INTERNAL;
+	  /* Fall through.  */
+	case ECONNREFUSED:
+	case ETIMEDOUT:
+	  status = NSS_STATUS_UNAVAIL;
+	  break;
+	default:
+	  status = NSS_STATUS_NOTFOUND;
+	  break;
 	}
-      else
-	status = (errno == ECONNREFUSED
-		  ? NSS_STATUS_UNAVAIL : NSS_STATUS_NOTFOUND);
+
       *herrnop = h_errno;
       if (h_errno == TRY_AGAIN)
 	*errnop = EAGAIN;
diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
index 672571e..ad5ae01 100644
--- a/sysdeps/posix/getaddrinfo.c
+++ b/sysdeps/posix/getaddrinfo.c
@@ -1049,6 +1049,12 @@ gaih_inet (const char *name, const struct gaih_service *service,
 
 	  _res.options |= old_res_options & RES_USE_INET6;
 
+	  if (status == NSS_STATUS_UNAVAIL)
+	    {
+	      result = GAIH_OKIFUNSPEC | -EAI_SYSTEM;
+	      goto free_and_return;
+	    }
+
 	  if (no_data != 0 && no_inet6_data != 0)
 	    {
 	      /* If both requests timed out report this.  */