This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] nss_files: Refactor gethostbyname3 multi case into separate function
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Tue, 5 Sep 2017 14:15:04 -0300
- Subject: Re: [PATCH] nss_files: Refactor gethostbyname3 multi case into separate function
- Authentication-results: sourceware.org; auth=none
- References: <20170904173046.DA957439942E3@oldenburg.str.redhat.com>
On 04/09/2017 14:30, Florian Weimer wrote:
> This is in preparation of further cleanup work.
>
> 2017-09-04 Florian Weimer <fweimer@redhat.com>
>
> * nss/nss_files/files-hosts.c (gethostbyname3_multi): New
> function.
> (_nss_files_gethostbyname3_r): Call it.
LGTM.
>
> diff --git a/nss/nss_files/files-hosts.c b/nss/nss_files/files-hosts.c
> index bccb6a5780..867c10c2ef 100644
> --- a/nss/nss_files/files-hosts.c
> +++ b/nss/nss_files/files-hosts.c
> @@ -115,6 +115,206 @@ DB_LOOKUP (hostbyaddr, ,,,
> }, const void *addr, socklen_t len, int af)
> #undef EXTRA_ARGS_VALUE
>
> +static enum nss_status
> +gethostbyname3_multi (FILE * stream, const char *name, int af,
> + struct hostent *result, char *buffer, size_t buflen,
> + int *errnop, int *herrnop, int flags)
> +{
> + /* We have to get all host entries from the file. */
> + size_t tmp_buflen = MIN (buflen, 4096);
> + char tmp_buffer_stack[tmp_buflen]
> + __attribute__ ((__aligned__ (__alignof__ (struct hostent_data))));
> + char *tmp_buffer = tmp_buffer_stack;
> + struct hostent tmp_result_buf;
> + int naddrs = 1;
> + int naliases = 0;
> + char *bufferend;
> + bool tmp_buffer_malloced = false;
> + enum nss_status status;
> +
> + while (result->h_aliases[naliases] != NULL)
> + ++naliases;
> +
> + bufferend = (char *) &result->h_aliases[naliases + 1];
> +
> + again:
> + while ((status = internal_getent (stream, &tmp_result_buf, tmp_buffer,
> + tmp_buflen, errnop, herrnop, af,
> + flags))
> + == NSS_STATUS_SUCCESS)
> + {
> + int matches = 1;
> + struct hostent *old_result = result;
> + result = &tmp_result_buf;
> + /* The following piece is a bit clumsy but we want to use the
> + `LOOKUP_NAME_CASE' value. The optimizer should do its
> + job. */
> + do
> + {
> + LOOKUP_NAME_CASE (h_name, h_aliases)
> + result = old_result;
> + }
> + while ((matches = 0));
> +
> + if (matches)
> + {
> + /* We could be very clever and try to recycle a few bytes
> + in the buffer instead of generating new arrays. But
> + we are not doing this here since it's more work than
> + it's worth. Simply let the user provide a bit bigger
> + buffer. */
> + char **new_h_addr_list;
> + char **new_h_aliases;
> + int newaliases = 0;
> + size_t newstrlen = 0;
> + int cnt;
> +
> + /* Count the new aliases and the length of the strings. */
> + while (tmp_result_buf.h_aliases[newaliases] != NULL)
> + {
> + char *cp = tmp_result_buf.h_aliases[newaliases];
> + ++newaliases;
> + newstrlen += strlen (cp) + 1;
> + }
> + /* If the real name is different add it also to the
> + aliases. This means that there is a duplication
> + in the alias list but this is really the user's
> + problem. */
> + if (strcmp (old_result->h_name,
> + tmp_result_buf.h_name) != 0)
> + {
> + ++newaliases;
> + newstrlen += strlen (tmp_result_buf.h_name) + 1;
> + }
> +
> + /* Make sure bufferend is aligned. */
> + assert ((bufferend - (char *) 0) % sizeof (char *) == 0);
> +
> + /* Now we can check whether the buffer is large enough.
> + 16 is the maximal size of the IP address. */
> + if (bufferend + 16 + (naddrs + 2) * sizeof (char *)
> + + roundup (newstrlen, sizeof (char *))
> + + (naliases + newaliases + 1) * sizeof (char *)
> + >= buffer + buflen)
> + {
> + *errnop = ERANGE;
> + *herrnop = NETDB_INTERNAL;
> + status = NSS_STATUS_TRYAGAIN;
> + goto out;
> + }
> +
> + new_h_addr_list =
> + (char **) (bufferend
> + + roundup (newstrlen, sizeof (char *))
> + + 16);
> + new_h_aliases =
> + (char **) ((char *) new_h_addr_list
> + + (naddrs + 2) * sizeof (char *));
> +
> + /* Copy the old data in the new arrays. */
> + for (cnt = 0; cnt < naddrs; ++cnt)
> + new_h_addr_list[cnt] = old_result->h_addr_list[cnt];
> +
> + for (cnt = 0; cnt < naliases; ++cnt)
> + new_h_aliases[cnt] = old_result->h_aliases[cnt];
> +
> + /* Store the new strings. */
> + cnt = 0;
> + while (tmp_result_buf.h_aliases[cnt] != NULL)
> + {
> + new_h_aliases[naliases++] = bufferend;
> + bufferend = (__stpcpy (bufferend,
> + tmp_result_buf.h_aliases[cnt])
> + + 1);
> + ++cnt;
> + }
> +
> + if (cnt < newaliases)
> + {
> + new_h_aliases[naliases++] = bufferend;
> + bufferend = __stpcpy (bufferend,
> + tmp_result_buf.h_name) + 1;
> + }
> +
> + /* Final NULL pointer. */
> + new_h_aliases[naliases] = NULL;
> +
> + /* Round up the buffer end address. */
> + bufferend += (sizeof (char *)
> + - ((bufferend - (char *) 0)
> + % sizeof (char *))) % sizeof (char *);
> +
> + /* Now the new address. */
> + new_h_addr_list[naddrs++] =
> + memcpy (bufferend, tmp_result_buf.h_addr,
> + tmp_result_buf.h_length);
> +
> + /* Also here a final NULL pointer. */
> + new_h_addr_list[naddrs] = NULL;
> +
> + /* Store the new array pointers. */
> + old_result->h_aliases = new_h_aliases;
> + old_result->h_addr_list = new_h_addr_list;
> +
> + /* Compute the new buffer end. */
> + bufferend = (char *) &new_h_aliases[naliases + 1];
> + assert (bufferend <= buffer + buflen);
> +
> + result = old_result;
> + }
> + }
> +
> + if (status == NSS_STATUS_TRYAGAIN)
> + {
> + size_t newsize = 2 * tmp_buflen;
> + if (tmp_buffer_malloced)
> + {
> + char *newp = realloc (tmp_buffer, newsize);
> + if (newp != NULL)
> + {
> + assert ((((uintptr_t) newp)
> + & (__alignof__ (struct hostent_data) - 1))
> + == 0);
> + tmp_buffer = newp;
> + tmp_buflen = newsize;
> + goto again;
> + }
> + }
> + else if (!__libc_use_alloca (buflen + newsize))
> + {
> + tmp_buffer = malloc (newsize);
> + if (tmp_buffer != NULL)
> + {
> + assert ((((uintptr_t) tmp_buffer)
> + & (__alignof__ (struct hostent_data) - 1))
> + == 0);
> + tmp_buffer_malloced = true;
> + tmp_buflen = newsize;
> + goto again;
> + }
> + }
> + else
> + {
> + tmp_buffer
> + = extend_alloca (tmp_buffer, tmp_buflen,
> + newsize
> + + __alignof__ (struct hostent_data));
> + tmp_buffer = (char *) (((uintptr_t) tmp_buffer
> + + __alignof__ (struct hostent_data)
> + - 1)
> + & ~(__alignof__ (struct hostent_data)
> + - 1));
> + goto again;
> + }
> + }
> + else
> + status = NSS_STATUS_SUCCESS;
> + out:
> + if (tmp_buffer_malloced)
> + free (tmp_buffer);
> + return status;
> +}
> +
> enum nss_status
> _nss_files_gethostbyname3_r (const char *name, int af, struct hostent *result,
> char *buffer, size_t buflen, int *errnop,
> @@ -143,199 +343,8 @@ _nss_files_gethostbyname3_r (const char *name, int af, struct hostent *result,
>
> if (status == NSS_STATUS_SUCCESS
> && _res_hconf.flags & HCONF_FLAG_MULTI)
> - {
> - /* We have to get all host entries from the file. */
> - size_t tmp_buflen = MIN (buflen, 4096);
> - char tmp_buffer_stack[tmp_buflen]
> - __attribute__ ((__aligned__ (__alignof__ (struct hostent_data))));
> - char *tmp_buffer = tmp_buffer_stack;
> - struct hostent tmp_result_buf;
> - int naddrs = 1;
> - int naliases = 0;
> - char *bufferend;
> - bool tmp_buffer_malloced = false;
> -
> - while (result->h_aliases[naliases] != NULL)
> - ++naliases;
> -
> - bufferend = (char *) &result->h_aliases[naliases + 1];
> -
> - again:
> - while ((status = internal_getent (stream, &tmp_result_buf, tmp_buffer,
> - tmp_buflen, errnop, herrnop, af,
> - flags))
> - == NSS_STATUS_SUCCESS)
> - {
> - int matches = 1;
> - struct hostent *old_result = result;
> - result = &tmp_result_buf;
> - /* The following piece is a bit clumsy but we want to use the
> - `LOOKUP_NAME_CASE' value. The optimizer should do its
> - job. */
> - do
> - {
> - LOOKUP_NAME_CASE (h_name, h_aliases)
> - result = old_result;
> - }
> - while ((matches = 0));
> -
> - if (matches)
> - {
> - /* We could be very clever and try to recycle a few bytes
> - in the buffer instead of generating new arrays. But
> - we are not doing this here since it's more work than
> - it's worth. Simply let the user provide a bit bigger
> - buffer. */
> - char **new_h_addr_list;
> - char **new_h_aliases;
> - int newaliases = 0;
> - size_t newstrlen = 0;
> - int cnt;
> -
> - /* Count the new aliases and the length of the strings. */
> - while (tmp_result_buf.h_aliases[newaliases] != NULL)
> - {
> - char *cp = tmp_result_buf.h_aliases[newaliases];
> - ++newaliases;
> - newstrlen += strlen (cp) + 1;
> - }
> - /* If the real name is different add it also to the
> - aliases. This means that there is a duplication
> - in the alias list but this is really the user's
> - problem. */
> - if (strcmp (old_result->h_name,
> - tmp_result_buf.h_name) != 0)
> - {
> - ++newaliases;
> - newstrlen += strlen (tmp_result_buf.h_name) + 1;
> - }
> -
> - /* Make sure bufferend is aligned. */
> - assert ((bufferend - (char *) 0) % sizeof (char *) == 0);
> -
> - /* Now we can check whether the buffer is large enough.
> - 16 is the maximal size of the IP address. */
> - if (bufferend + 16 + (naddrs + 2) * sizeof (char *)
> - + roundup (newstrlen, sizeof (char *))
> - + (naliases + newaliases + 1) * sizeof (char *)
> - >= buffer + buflen)
> - {
> - *errnop = ERANGE;
> - *herrnop = NETDB_INTERNAL;
> - status = NSS_STATUS_TRYAGAIN;
> - goto out;
> - }
> -
> - new_h_addr_list =
> - (char **) (bufferend
> - + roundup (newstrlen, sizeof (char *))
> - + 16);
> - new_h_aliases =
> - (char **) ((char *) new_h_addr_list
> - + (naddrs + 2) * sizeof (char *));
> -
> - /* Copy the old data in the new arrays. */
> - for (cnt = 0; cnt < naddrs; ++cnt)
> - new_h_addr_list[cnt] = old_result->h_addr_list[cnt];
> -
> - for (cnt = 0; cnt < naliases; ++cnt)
> - new_h_aliases[cnt] = old_result->h_aliases[cnt];
> -
> - /* Store the new strings. */
> - cnt = 0;
> - while (tmp_result_buf.h_aliases[cnt] != NULL)
> - {
> - new_h_aliases[naliases++] = bufferend;
> - bufferend = (__stpcpy (bufferend,
> - tmp_result_buf.h_aliases[cnt])
> - + 1);
> - ++cnt;
> - }
> -
> - if (cnt < newaliases)
> - {
> - new_h_aliases[naliases++] = bufferend;
> - bufferend = __stpcpy (bufferend,
> - tmp_result_buf.h_name) + 1;
> - }
> -
> - /* Final NULL pointer. */
> - new_h_aliases[naliases] = NULL;
> -
> - /* Round up the buffer end address. */
> - bufferend += (sizeof (char *)
> - - ((bufferend - (char *) 0)
> - % sizeof (char *))) % sizeof (char *);
> -
> - /* Now the new address. */
> - new_h_addr_list[naddrs++] =
> - memcpy (bufferend, tmp_result_buf.h_addr,
> - tmp_result_buf.h_length);
> -
> - /* Also here a final NULL pointer. */
> - new_h_addr_list[naddrs] = NULL;
> -
> - /* Store the new array pointers. */
> - old_result->h_aliases = new_h_aliases;
> - old_result->h_addr_list = new_h_addr_list;
> -
> - /* Compute the new buffer end. */
> - bufferend = (char *) &new_h_aliases[naliases + 1];
> - assert (bufferend <= buffer + buflen);
> -
> - result = old_result;
> - }
> - }
> -
> - if (status == NSS_STATUS_TRYAGAIN)
> - {
> - size_t newsize = 2 * tmp_buflen;
> - if (tmp_buffer_malloced)
> - {
> - char *newp = realloc (tmp_buffer, newsize);
> - if (newp != NULL)
> - {
> - assert ((((uintptr_t) newp)
> - & (__alignof__ (struct hostent_data) - 1))
> - == 0);
> - tmp_buffer = newp;
> - tmp_buflen = newsize;
> - goto again;
> - }
> - }
> - else if (!__libc_use_alloca (buflen + newsize))
> - {
> - tmp_buffer = malloc (newsize);
> - if (tmp_buffer != NULL)
> - {
> - assert ((((uintptr_t) tmp_buffer)
> - & (__alignof__ (struct hostent_data) - 1))
> - == 0);
> - tmp_buffer_malloced = true;
> - tmp_buflen = newsize;
> - goto again;
> - }
> - }
> - else
> - {
> - tmp_buffer
> - = extend_alloca (tmp_buffer, tmp_buflen,
> - newsize
> - + __alignof__ (struct hostent_data));
> - tmp_buffer = (char *) (((uintptr_t) tmp_buffer
> - + __alignof__ (struct hostent_data)
> - - 1)
> - & ~(__alignof__ (struct hostent_data)
> - - 1));
> - goto again;
> - }
> - }
> - else
> - status = NSS_STATUS_SUCCESS;
> - out:
> - if (tmp_buffer_malloced)
> - free (tmp_buffer);
> - }
> + status = gethostbyname3_multi
> + (stream, name, af, result, buffer, buflen, errnop, herrnop, flags);
>
> internal_endent (&stream);
> }
>