Re: [PATCH v2 03/12] getaddrinfo: Fix leak with AI_ALL [BZ #28852]

[DJ Delorie <dj@redhat.com>]

Siddhesh Poyarekar via Libc-alpha <libc-alpha@sourceware.org> writes:
> Use realloc in convert_hostent_to_gaih_addrtuple and fix up pointers in
> the result list so that a single block is maintained for
> hostbyname3_r/hostbyname2_r and freed in gaih_inet.  This result is
> never merged with any other results, since the hosts database does not
> permit merging.

It took me a while to realize that you're basically converting the data
structure from a linked list to an array, and ensuring that it's always
handled as an array.  That means the comment preceeding
convert_hostent_to_gaih_addrtuple() is no longer accurate and needs
updating.  Ideally, the users could be optimized to treat it as an array
instead of a list, but it may need to remain a list-like type for
compatibility.

LGTM with that comment change.

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

> -  while (*result)
> -    result = &(*result)->next;
> -

Don't skip to end of "list".  Ok.

> @@ -212,10 +209,30 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
>    if (count == 0 || h->h_length > sizeof (((struct gaih_addrtuple) {}).addr))
>      return true;
>  
> -  struct gaih_addrtuple *array = calloc (count, sizeof (*array));
> +  struct gaih_addrtuple *array = *result;
> +  size_t old = 0;
> +
> +  while (array != NULL)
> +    {
> +      old++;
> +      array = array->next;
> +    }
> +
> +  array = realloc (*result, (old + count) * sizeof (*array));
> +

Count existing members of *array* and resize base array.  Ok.  The rest
are initialized later, in the unchanged part of the code.

>    if (array == NULL)
>      return false;
>  
> +  *result = array;
> +
> +  /* Update the next pointers on reallocation.  */
> +  for (size_t i = 0; i < old; i++)
> +    array[i].next = array + i + 1;
> +
> +  array += old;
> +
> +  memset (array, 0, count * sizeof (*array));
> +

Ok.

>    for (size_t i = 0; i < count; ++i)
>      {
>        if (family == AF_INET && req->ai_family == AF_INET6)
> @@ -235,7 +252,6 @@ convert_hostent_to_gaih_addrtuple (const struct addrinfo *req,
>    array[0].name = h->h_name;
>    array[count - 1].next = NULL;
>  
> -  *result = array;
>    return true;
>  }

Ok.