This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v2][BZ #16077] Get canonical name in getaddrinfo from hosts file for AF_INET
- From: Pavel Simerda <psimerda at redhat dot com>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: Andreas Schwab <schwab at suse dot de>, libc-alpha at sourceware dot org
- Date: Mon, 25 Nov 2013 10:44:37 -0500 (EST)
- Subject: Re: [PATCH v2][BZ #16077] Get canonical name in getaddrinfo from hosts file for AF_INET
- Authentication-results: sourceware.org; auth=none
- References: <20131023100431 dot GG7401 at spoyarek dot pnq dot redhat dot com> <mvmwqjwen32 dot fsf at hawking dot suse dot de> <20131125151649 dot GT19834 at spoyarek dot pnq dot redhat dot com>
----- Original Message -----
> From: "Siddhesh Poyarekar" <siddhesh@redhat.com>
> To: "Andreas Schwab" <schwab@suse.de>
> Cc: libc-alpha@sourceware.org
> Sent: Monday, November 25, 2013 4:16:49 PM
> Subject: [PATCH v2][BZ #16077] Get canonical name in getaddrinfo from hosts file for AF_INET
>
> On Mon, Nov 25, 2013 at 12:29:37PM +0100, Andreas Schwab wrote:
> > gethostbyname2 should be implemented on top of gethostbyname3, not the
> > other way round.
>
> OK, here's a new version then - I was trying to avoid a large change,
> but I realized I defeated my own purpose of trying to keep things
> consistent.
>
> I have junked the HOSTS_DB_LOOKUP macro and replaced if with
> _nss_files_gethostbyname3_r (reusing most of the macro code)
That sounds great!
> and then
> call it from _nss_files_gethostbyname_r and
> _nss_files_gethostbyname2_r.
Likewise!
> Verified that nothing breaks due to this
> by building a Fedora package with the patch and testing it for lookups
> from /etc/hosts on my test box.
Could we also check the (patched) code for the following bug?
https://sourceware.org/bugzilla/show_bug.cgi?id=14969
Cheers,
Pavel
> OK to commit?
>
> Siddhesh
>
> [BZ #16077]
> * nss/Versions (libnss_files): Add
> _nss_files_gethostbyname3_r.
> * nss/nss_files/files-hosts.c (_nss_files_gethostbyname3_r):
> New function.
> (HOST_DB_LOOKUP): Remove macro.
> (_nss_files_gethostbyname_r): Implement function without the
> HOST_DB_LOOKUP macro.
> (_nss_files_gethostbyname2_r): Likewise.
>
> diff --git a/nss/Versions b/nss/Versions
> index d13d570..f8ababc 100644
> --- a/nss/Versions
> +++ b/nss/Versions
> @@ -40,6 +40,7 @@ libnss_files {
> _nss_files_endhostent;
> _nss_files_gethostbyaddr_r;
> _nss_files_gethostbyname2_r;
> + _nss_files_gethostbyname3_r;
> _nss_files_gethostbyname4_r;
> _nss_files_gethostbyname_r;
> _nss_files_gethostent_r;
> diff --git a/nss/nss_files/files-hosts.c b/nss/nss_files/files-hosts.c
> index b1b71ec..9605ee6 100644
> --- a/nss/nss_files/files-hosts.c
> +++ b/nss/nss_files/files-hosts.c
> @@ -97,262 +97,12 @@ LINE_PARSER
> STRING_FIELD (result->h_name, isspace, 1);
> })
>
> -
> -
> -#define HOST_DB_LOOKUP(name, keysize, keypattern, break_if_match, proto...)
> \
> -enum nss_status \
> -_nss_files_get##name##_r (proto, \
> - struct STRUCTURE *result, char *buffer, \
> - size_t buflen, int *errnop H_ERRNO_PROTO) \
> -{ \
> - uintptr_t pad = -(uintptr_t) buffer % __alignof__ (struct hostent_data);
> \
> - buffer += pad; \
> - buflen = buflen > pad ? buflen - pad : 0; \
> - \
> - __libc_lock_lock (lock); \
> - \
> - /* Reset file pointer to beginning or open file. */ \
> - enum nss_status status = internal_setent (keep_stream); \
> - \
> - if (status == NSS_STATUS_SUCCESS) \
> - { \
> - /* Tell getent function that we have repositioned the file pointer.
> */ \
> - last_use = getby; \
> - \
> - while ((status = internal_getent (result, buffer, buflen, errnop
> \
> - H_ERRNO_ARG EXTRA_ARGS_VALUE)) \
> - == NSS_STATUS_SUCCESS) \
> - { break_if_match } \
> - \
> - 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 (&tmp_result_buf, tmp_buffer, \
> - tmp_buflen, errnop H_ERRNO_ARG \
> - EXTRA_ARGS_VALUE)) \
> - == 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 \
> - `break_if_match' value. The optimizer should do its \
> - job. */ \
> - do \
> - { \
> - break_if_match \
> - 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); \
> - } \
> - \
> - \
> - if (! keep_stream) \
> - internal_endent (); \
> - } \
> - \
> - __libc_lock_unlock (lock); \
> - \
> - return status; \
> -}
> -
> -
> #define EXTRA_ARGS_VALUE \
> , ((_res.options & RES_USE_INET6) ? AF_INET6 : AF_INET), \
> ((_res.options & RES_USE_INET6) ? AI_V4MAPPED : 0)
> #include "files-XXX.c"
> -HOST_DB_LOOKUP (hostbyname, ,,
> - {
> - LOOKUP_NAME_CASE (h_name, h_aliases)
> - }, const char *name)
> #undef EXTRA_ARGS_VALUE
>
> -
> -/* XXX Is using _res to determine whether we want to convert IPv4 addresses
> - to IPv6 addresses really the right thing to do? */
> -#define EXTRA_ARGS_VALUE \
> - , af, ((_res.options & RES_USE_INET6) ? AI_V4MAPPED : 0)
> -HOST_DB_LOOKUP (hostbyname2, ,,
> - {
> - LOOKUP_NAME_CASE (h_name, h_aliases)
> - }, const char *name, int af)
> -#undef EXTRA_ARGS_VALUE
> -
> -
> /* We only need to consider IPv4 mapped addresses if the input to the
> gethostbyaddr() function is an IPv6 address. */
> #define EXTRA_ARGS_VALUE \
> @@ -365,6 +115,263 @@ DB_LOOKUP (hostbyaddr, ,,,
> }, const void *addr, socklen_t len, int af)
> #undef EXTRA_ARGS_VALUE
>
> +enum nss_status
> +_nss_files_gethostbyname3_r (const char *name, int af, struct hostent
> *result,
> + char *buffer, size_t buflen, int *errnop,
> + int *herrnop, int32_t *ttlp, char **canonp)
> +{
> + uintptr_t pad = -(uintptr_t) buffer % __alignof__ (struct hostent_data);
> + buffer += pad;
> + buflen = buflen > pad ? buflen - pad : 0;
> +
> + __libc_lock_lock (lock);
> +
> + /* Reset file pointer to beginning or open file. */
> + enum nss_status status = internal_setent (keep_stream);
> +
> + if (status == NSS_STATUS_SUCCESS)
> + {
> + /* XXX Is using _res to determine whether we want to convert IPv4
> + addresses to IPv6 addresses really the right thing to do? */
> + int flags = ((_res.options & RES_USE_INET6) ? AI_V4MAPPED : 0);
> +
> + /* Tell getent function that we have repositioned the file pointer.
> */
> + last_use = getby;
> +
> + while ((status = internal_getent (result, buffer, buflen, errnop,
> + herrnop, af, flags))
> + == NSS_STATUS_SUCCESS)
> + {
> + LOOKUP_NAME_CASE (h_name, h_aliases)
> + }
> +
> + 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 (&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
> + `break_if_match' 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);
> + }
> +
> + if (! keep_stream)
> + internal_endent ();
> + }
> +
> + if (result && canonp && status == NSS_STATUS_SUCCESS)
> + *canonp = result->h_name;
> +
> + __libc_lock_unlock (lock);
> +
> + return status;
> +}
> +
> +enum nss_status
> +_nss_files_gethostbyname_r (const char *name, struct hostent *result,
> + char *buffer, size_t buflen, int *errnop,
> + int *herrnop)
> +{
> + int af = ((_res.options & RES_USE_INET6) ? AF_INET6 : AF_INET);
> +
> + return _nss_files_gethostbyname3_r (name, af, result, buffer, buflen,
> + errnop, herrnop, NULL, NULL);
> +}
> +
> +enum nss_status
> +_nss_files_gethostbyname2_r (const char *name, int af, struct hostent
> *result,
> + char *buffer, size_t buflen, int *errnop,
> + int *herrnop)
> +{
> + return _nss_files_gethostbyname3_r (name, af, result, buffer, buflen,
> + errnop, herrnop, NULL, NULL);
> +}
>
> enum nss_status
> _nss_files_gethostbyname4_r (const char *name, struct gaih_addrtuple **pat,
>