Re: [PATCH v2 09/12] gaih_inet: make gethosts into a function

[DJ Delorie <dj@redhat.com>]

Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:
> The macro is quite a pain to debug, so make gethosts into a function to
> make it easier to maintain.

LGTM

Reviewed-by: DJ Delorie <dj@redhat.com>

> diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
> index c33cc2507b..e71f21f3c9 100644
> --- a/sysdeps/posix/getaddrinfo.c
> +++ b/sysdeps/posix/getaddrinfo.c
> @@ -268,63 +268,54 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req, int family,
>    return true;
>  }
>  
> -#define gethosts(_family) \
> - {									      \
> -  struct hostent th;							      \
> -  char *localcanon = NULL;						      \
> -  no_data = 0;								      \
> -  while (1)								      \
> -    {									      \
> -      status = DL_CALL_FCT (fct, (name, _family, &th,			      \
> -				  tmpbuf->data, tmpbuf->length,		      \
> -				  &errno, &h_errno, NULL, &localcanon));      \
> -      if (status != NSS_STATUS_TRYAGAIN || h_errno != NETDB_INTERNAL	      \
> -	  || errno != ERANGE)						      \
> -	break;								      \
> -      if (!scratch_buffer_grow (tmpbuf))				      \
> -	{								      \
> -	  __resolv_context_put (res_ctx);				      \
> -	  result = -EAI_MEMORY;						      \
> -	  goto out;							      \
> -	}								      \
> -    }									      \
> -  if (status == NSS_STATUS_NOTFOUND					      \
> -      || status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)	      \
> -    {									      \
> -      if (h_errno == NETDB_INTERNAL)					      \
> -	{								      \
> -	  __resolv_context_put (res_ctx);				      \
> -	  result = -EAI_SYSTEM;						      \
> -	  goto out;							      \
> -	}								      \
> -      if (h_errno == TRY_AGAIN)						      \
> -	no_data = EAI_AGAIN;						      \
> -      else								      \
> -	no_data = h_errno == NO_DATA;					      \
> -    }									      \
> -  else if (status == NSS_STATUS_SUCCESS)				      \
> -    {									      \
> -      if (!convert_hostent_to_gaih_addrtuple (req, _family, &th, res))	      \
> -	{								      \
> -	  __resolv_context_put (res_ctx);				      \
> -	  result = -EAI_SYSTEM;						      \
> -	  goto out;							      \
> -	}								      \
> -									      \
> -      if (localcanon != NULL && res->canon == NULL)			      \
> -	{								      \
> -	  char *canonbuf = __strdup (localcanon);			      \
> -	  if (canonbuf == NULL)						      \
> -	    {								      \
> -	      __resolv_context_put (res_ctx);				      \
> -	      result = -EAI_SYSTEM;					      \
> -	      goto out;							      \
> -	    }								      \
> -	  res->canon = canonbuf;					      \
> -	}								      \
> -    }									      \
> - }
> +static int
> +gethosts (nss_gethostbyname3_r fct, int family, const char *name,
> +	  const struct addrinfo *req, struct scratch_buffer *tmpbuf,
> +	  struct gaih_result *res, enum nss_status *statusp, int *no_datap)
> +{
> +  struct hostent th;
> +  char *localcanon = NULL;
> +  enum nss_status status;
> +
> +  *no_datap = 0;
> +  while (1)
> +    {
> +      *statusp = status = DL_CALL_FCT (fct, (name, family, &th,
> +					     tmpbuf->data, tmpbuf->length,
> +					     &errno, &h_errno, NULL,
> +					     &localcanon));
> +      if (status != NSS_STATUS_TRYAGAIN || h_errno != NETDB_INTERNAL
> +	  || errno != ERANGE)
> +	break;
> +      if (!scratch_buffer_grow (tmpbuf))
> +	return -EAI_MEMORY;
> +    }
> +  if (status == NSS_STATUS_NOTFOUND
> +      || status == NSS_STATUS_TRYAGAIN || status == NSS_STATUS_UNAVAIL)
> +    {
> +      if (h_errno == NETDB_INTERNAL)
> +	return -EAI_SYSTEM;
> +      if (h_errno == TRY_AGAIN)
> +	*no_datap = EAI_AGAIN;
> +      else
> +	*no_datap = h_errno == NO_DATA;
> +    }
> +  else if (status == NSS_STATUS_SUCCESS)
> +    {
> +      if (!convert_hostent_to_gaih_addrtuple (req, family, &th, res))
> +	return -EAI_SYSTEM;
> +
> +      if (localcanon != NULL && res->canon == NULL)
> +	{
> +	  char *canonbuf = __strdup (localcanon);
> +	  if (canonbuf == NULL)
> +	    return  -EAI_SYSTEM;
> +	  res->canon = canonbuf;
> +	}
> +    }
>  
> +  return 0;
> +}
>  
>  /* This function is called if a canonical name is requested, but if
>     the service function did not provide it.  It tries to obtain the
> @@ -741,7 +732,12 @@ get_nss_addresses (const char *name, const struct addrinfo *req,
>  	      if (req->ai_family == AF_INET6
>  		  || req->ai_family == AF_UNSPEC)
>  		{
> -		  gethosts (AF_INET6);
> +		  if ((result = gethosts (fct, AF_INET6, name, req, tmpbuf,
> +					  res, &status, &no_data)) != 0)
> +		    {
> +		      __resolv_context_put (res_ctx);
> +		      goto out;
> +		    }
>  		  no_inet6_data = no_data;
>  		  inet6_status = status;
>  		}
> @@ -753,7 +749,12 @@ get_nss_addresses (const char *name, const struct addrinfo *req,
>  			 know we are not going to need them.  */
>  		      && ((req->ai_flags & AI_ALL) || !res->got_ipv6)))
>  		{
> -		  gethosts (AF_INET);
> +		  if ((result = gethosts (fct, AF_INET, name, req, tmpbuf,
> +					  res, &status, &no_data)) != 0)
> +		    {
> +		      __resolv_context_put (res_ctx);
> +		      goto out;
> +		    }
>  
>  		  if (req->ai_family == AF_INET)
>  		    {