Re: [PATCH v2 11/12] gaih_inet: Split result generation into its own function

[DJ Delorie <dj@redhat.com>]

Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:
> Simplify the loop a wee bit and clean up variable names too.

FYI combining "split out" and "minor changes" makes it harder to review,
because the minor changes are nearly impossible to spot in the large
chunks of "moving".

I had one comment about a leak (see below) but it's independent of this
patch set (maybe?).

LGTM.

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

> diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
> index 3aeeb0d1e0..eebed824a8 100644
> --- a/sysdeps/posix/getaddrinfo.c
> +++ b/sysdeps/posix/getaddrinfo.c
> @@ -1028,6 +1028,87 @@ get_local_addresses (const struct addrinfo *req, struct gaih_result *res)
>      }
>  }
>  
> +/* Generate results in PAI and its count in NADDRS.  Return 0 on success or an
> +   error code on failure.  */
> +
> +static int
> +generate_addrinfo (const struct addrinfo *req, struct gaih_result *res,
> +		   const struct gaih_servtuple *st, struct addrinfo **pai,
> +		   unsigned int *naddrs)
> +{
> + . . .

Ok.

> +      for (int i = 0; st[i].set; i++)
> +	{
> +	  struct addrinfo *ai;
> +	  ai = *pai = malloc (sizeof (struct addrinfo) + socklen);
> +	  if (ai == NULL)
> +	    return -EAI_MEMORY;

I think there might be a memory leak here if malloc fails on the second
or later iteration of the loop.  However, we would have had that leak
before, too, and if we're out of memory... this type of leak is likely
not what you'd be worried about.

> + . . .
> +  return 0;
> +}
> +

Ok.

>  
>  process_list:
> +  /* Set up the canonical name if we need it.  */
> +  if ((result = process_canonname (req, orig_name, &res)) != 0)
> +    goto free_and_return;
> -  {
> -    /* Set up the canonical name if we need it.  */
> -    if ((result = process_canonname (req, orig_name, &res)) != 0)
> -      goto free_and_return;
> -
> - . . .
> -      }
> -  }

Ok.

> +  result = generate_addrinfo (req, &res, st, pai, naddrs);

Ok.

> - free_and_return:
> +free_and_return:

Ok.