Re: [PATCH 18/30] resolv: Move __res_get_nsaddr to its own file and into libc

[Carlos O'Donell via Libc-alpha <libc-alpha@sourceware.org>]

On 7/8/21 11:04 AM, Florian Weimer via Libc-alpha wrote:
> Eliminate the use of the EXT macro from it because it does not
> add clarity.  The function was added to res_send.c in 2015, and
> the copyright year reflects that.

OK for glibc 2.34.

Tested without regression on x86_64 and i686.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  resolv/Makefile          |  1 +
>  resolv/Versions          |  1 +
>  resolv/res_get_nsaddr.c  | 39 +++++++++++++++++++++++++++++++++++++++
>  resolv/res_send.c        | 25 ++++---------------------
>  resolv/resolv-internal.h |  6 ++++++
>  5 files changed, 51 insertions(+), 21 deletions(-)
>  create mode 100644 resolv/res_get_nsaddr.c
> 
> diff --git a/resolv/Makefile b/resolv/Makefile
> index 7e142b9a14..6eed73de87 100644
> --- a/resolv/Makefile
> +++ b/resolv/Makefile
> @@ -46,6 +46,7 @@ routines := \
>    res-close \
>    res-name-checking \
>    res-state \
> +  res_get_nsaddr \

OK.

>    res_hconf \
>    res_init \
>    res_libc \
> diff --git a/resolv/Versions b/resolv/Versions
> index 3e3b89a0b2..fc14cdb701 100644
> --- a/resolv/Versions
> +++ b/resolv/Versions
> @@ -81,6 +81,7 @@ libc {
>      __ns_name_skip;
>      __ns_name_uncompress;
>      __ns_name_unpack;
> +    __res_get_nsaddr;

OK.

>      __res_iclose;
>      __resolv_context_get;
>      __resolv_context_get_override;
> diff --git a/resolv/res_get_nsaddr.c b/resolv/res_get_nsaddr.c
> new file mode 100644
> index 0000000000..593597bdd7
> --- /dev/null
> +++ b/resolv/res_get_nsaddr.c
> @@ -0,0 +1,39 @@
> +/* Name server address at specified index in res_state.

OK.

> +   Copyright (C) 2015-2021 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <assert.h>
> +#include <resolv.h>
> +#include <resolv-internal.h>
> +
> +struct sockaddr *
> +__res_get_nsaddr (res_state statp, unsigned int n)
> +{
> +  assert (n < statp->nscount);
> +
> +  if (statp->nsaddr_list[n].sin_family == 0
> +      && statp->_u._ext.nsaddrs[n] != NULL)
> +    /* statp->_u._ext.nsaddrs[n] holds an address that is larger than
> +       struct sockaddr, and user code did not update
> +       statp->nsaddr_list[n].  */
> +    return (struct sockaddr *) statp->_u._ext.nsaddrs[n];

OK.

> +  else
> +    /* User code updated statp->nsaddr_list[n], or statp->nsaddr_list[n]
> +       has the same content as statp->_u._ext.nsaddrs[n].  */
> +    return (struct sockaddr *) (void *) &statp->nsaddr_list[n];

OK.

> +}
> +libc_hidden_def (__res_get_nsaddr)
> diff --git a/resolv/res_send.c b/resolv/res_send.c
> index b6139c7d62..78f564ee85 100644
> --- a/resolv/res_send.c
> +++ b/resolv/res_send.c
> @@ -180,7 +180,6 @@ evNowTime(struct timespec *res) {
>  
>  /* Forward. */
>  
> -static struct sockaddr *get_nsaddr (res_state, unsigned int);

OK.

>  static int		send_vc(res_state, const u_char *, int,
>  				const u_char *, int,
>  				u_char **, int *, int *, int, u_char **,
> @@ -215,7 +214,7 @@ res_ourserver_p(const res_state statp, const struct sockaddr_in6 *inp)
>  
>  	    for (ns = 0;  ns < statp->nscount;  ns++) {
>  		const struct sockaddr_in *srv =
> -		    (struct sockaddr_in *) get_nsaddr (statp, ns);
> +		    (struct sockaddr_in *) __res_get_nsaddr (statp, ns);

OK.

>  
>  		if ((srv->sin_family == AF_INET) &&
>  		    (srv->sin_port == port) &&
> @@ -226,7 +225,7 @@ res_ourserver_p(const res_state statp, const struct sockaddr_in6 *inp)
>  	} else if (inp->sin6_family == AF_INET6) {
>  	    for (ns = 0;  ns < statp->nscount;  ns++) {
>  		const struct sockaddr_in6 *srv
> -		  = (struct sockaddr_in6 *) get_nsaddr (statp, ns);
> +		  = (struct sockaddr_in6 *) __res_get_nsaddr (statp, ns);

OK.

>  		if ((srv->sin6_family == AF_INET6) &&
>  		    (srv->sin6_port == inp->sin6_port) &&
>  		    !(memcmp(&srv->sin6_addr, &in6addr_any,
> @@ -608,22 +607,6 @@ res_send (const unsigned char *buf, int buflen, unsigned char *ans, int anssiz)
>  
>  /* Private */
>  
> -static struct sockaddr *
> -get_nsaddr (res_state statp, unsigned int n)
> -{
> -  assert (n < statp->nscount);
> -
> -  if (statp->nsaddr_list[n].sin_family == 0 && EXT(statp).nsaddrs[n] != NULL)
> -    /* EXT(statp).nsaddrs[n] holds an address that is larger than
> -       struct sockaddr, and user code did not update
> -       statp->nsaddr_list[n].  */
> -    return (struct sockaddr *) EXT(statp).nsaddrs[n];
> -  else
> -    /* User code updated statp->nsaddr_list[n], or statp->nsaddr_list[n]
> -       has the same content as EXT(statp).nsaddrs[n].  */
> -    return (struct sockaddr *) (void *) &statp->nsaddr_list[n];
> -}
> -

OK. Remove.

>  /* Close the resolver structure, assign zero to *RESPLEN2 if RESPLEN2
>     is not NULL, and return zero.  */
>  static int
> @@ -717,7 +700,7 @@ send_vc(res_state statp,
>  	const HEADER *hp = (HEADER *) buf;
>  	const HEADER *hp2 = (HEADER *) buf2;
>  	HEADER *anhp = (HEADER *) *ansp;
> -	struct sockaddr *nsap = get_nsaddr (statp, ns);
> +	struct sockaddr *nsap = __res_get_nsaddr (statp, ns);

OK.

>  	int truncating, connreset, n;
>  	/* On some architectures compiler might emit a warning indicating
>  	   'resplen' may be used uninitialized.  However if buf2 == NULL
> @@ -948,7 +931,7 @@ static int
>  reopen (res_state statp, int *terrno, int ns)
>  {
>  	if (EXT(statp).nssocks[ns] == -1) {
> -		struct sockaddr *nsap = get_nsaddr (statp, ns);
> +		struct sockaddr *nsap = __res_get_nsaddr (statp, ns);

OK.

>  		socklen_t slen;
>  
>  		/* only try IPv6 if IPv6 NS and if not failed before */
> diff --git a/resolv/resolv-internal.h b/resolv/resolv-internal.h
> index e1e8aa3ef3..c18b1d0472 100644
> --- a/resolv/resolv-internal.h
> +++ b/resolv/resolv-internal.h
> @@ -103,4 +103,10 @@ void __res_thread_freeres (void) attribute_hidden;
>     success, -1 on failure.  */
>  int __res_enable_icmp (int family, int fd) attribute_hidden;
>  
> +
> +/* Returns the name server address for the indicated index.  */
> +struct sockaddr *__res_get_nsaddr (res_state statp, unsigned int n);
> +libc_hidden_proto (__res_get_nsaddr)

OK.

> +
> +
>  #endif  /* _RESOLV_INTERNAL_H */
> 


-- 
Cheers,
Carlos.