[PATCH 3/3] getaddrinfo: Refactor code for readability

[DJ Delorie]

Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:
> The close_retry goto jump is confusing and clumsy to read, so refactor
> the code a bit to make it easier to follow.

LGTM with or without the tweak noted below.

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

> diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c

> +static bool
> +try_connect (int *fdp, int *afp, struct sockaddr_in6 *source_addrp,
> +	     const struct sockaddr *addr, socklen_t addrlen, int family)
> +{
> +  int fd = *fdp;
> +  int af = *afp;
> +  socklen_t sl = sizeof (*source_addrp);
> +  bool retry = false;
> +
> +  do
> +    {
> +      if (fd != -1 && __connect (fd, addr, addrlen) == 0
> +	  && __getsockname (fd, (struct sockaddr *) source_addrp, &sl) == 0)
> +	return true;
> +      else if (errno == EAFNOSUPPORT && af == AF_INET6 && family == AF_INET)
> +	{
> +	  /* This could mean IPv6 sockets are IPv6-only.  */
> +	  if (fd != -1)
> +	    __close_nocancel_nostatus (fd);
> +	  *afp = af = AF_INET;
> +	  *fdp = fd = __socket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC,
> +				IPPROTO_IP);
> +	  retry = true;
> +	}
> +      else
> +	return false;
> +    }
> +  while (retry);
> +
> +  __builtin_unreachable ();
> +}

The do/while loop must follow one of three paths: return true,
retry=true, or return false.  The builtin_unreachable() agrees.  Thus,
the whole retry logic is unneeded and can be removed, using a
do/while(1) loop instead.

I'll OK this as-is though, but if your task is to simplify... then
simplify :-)

> @@ -2346,7 +2377,6 @@ getaddrinfo (const char *name, const char *service,
>  	      if (fd == -1 || (af == AF_INET && q->ai_family == AF_INET6))
>  		{
>  		  if (fd != -1)
> -		  close_retry:

Ok.

> +	      if (try_connect (&fd, &af, &results[i].source_addr, q->ai_addr,
> +			       q->ai_addrlen, q->ai_family))
> -	      socklen_t sl = sizeof (results[i].source_addr);
> -	      if (fd != -1
> -		  && __connect (fd, q->ai_addr, q->ai_addrlen) == 0
> -		  && __getsockname (fd,
> -				    (struct sockaddr *) &results[i].source_addr,
> -				    &sl) == 0)

Matches moved code, ok.

> -		  results[i].source_addr_len = sl;
> +		  results[i].source_addr_len = sizeof (results[i].source_addr);

Ok.

> -	      else if (errno == EAFNOSUPPORT && af == AF_INET6
> -		       && q->ai_family == AF_INET)
> -		/* This could mean IPv6 sockets are IPv6-only.  */
> -		goto close_retry;

Matches moved code, ok.