Re: [PATCH 20/30] resolv: Move ns_makecanon into its own file, and into libc

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

On 7/8/21 11:05 AM, Florian Weimer via Libc-alpha wrote:
> But only as an internal symbol, __libc_ns_makecanon.  The libresolv
> ABI is preserved.  This is because the function is deprecated, and
> it does not make sense to add new symbol versions for deprecated
> functions.

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>
 
> Also reformat the implementation to GNU style.
> ---
>  include/arpa/nameser.h     |  3 ++-
>  resolv/Makefile            |  2 ++
>  resolv/Versions            |  1 +
>  resolv/ns_makecanon.c      | 50 ++++++++++++++++++++++++++++++++++++++
>  resolv/ns_samedomain.c     | 38 ++---------------------------
>  resolv/resolv-deprecated.c | 35 ++++++++++++++++++++++++++
>  6 files changed, 92 insertions(+), 37 deletions(-)
>  create mode 100644 resolv/ns_makecanon.c
>  create mode 100644 resolv/resolv-deprecated.c
> 
> diff --git a/include/arpa/nameser.h b/include/arpa/nameser.h
> index eff25178c8..a529cc86d3 100644
> --- a/include/arpa/nameser.h
> +++ b/include/arpa/nameser.h
> @@ -69,9 +69,10 @@ libresolv_hidden_proto (ns_sprintrr)
>  libresolv_hidden_proto (ns_sprintrrf)
>  libresolv_hidden_proto (ns_samedomain)
>  libresolv_hidden_proto (ns_samename)
> -libresolv_hidden_proto (ns_makecanon)

OK.

>  libresolv_hidden_proto (ns_format_ttl)
>  
> +extern __typeof (ns_makecanon) __libc_ns_makecanon;
> +libc_hidden_proto (__libc_ns_makecanon)

OK.

>  extern __typeof (ns_name_compress) __ns_name_compress;
>  libc_hidden_proto (__ns_name_compress)
>  extern __typeof (ns_name_ntop) __ns_name_ntop;
> diff --git a/resolv/Makefile b/resolv/Makefile
> index 221bdc153d..d1ad5e4cba 100644
> --- a/resolv/Makefile
> +++ b/resolv/Makefile
> @@ -35,6 +35,7 @@ routines := \
>    inet_addr \
>    inet_ntop \
>    inet_pton \
> +  ns_makecanon \

OK.

>    ns_name_compress \
>    ns_name_ntop \
>    ns_name_pack \
> @@ -151,6 +152,7 @@ libresolv-routines := \
>    res_mkquery \
>    res_query \
>    res_send \
> +  resolv-deprecated \

OK.

>    # libresolv-routines
>  
>  $(libanl-routines-var) += \
> diff --git a/resolv/Versions b/resolv/Versions
> index fc14cdb701..93078de25c 100644
> --- a/resolv/Versions
> +++ b/resolv/Versions
> @@ -72,6 +72,7 @@ libc {
>      __inet_pton_length;
>      __libc_dn_expand;
>      __libc_dn_skipname;
> +    __libc_ns_makecanon;

OK.

>      __libc_res_dnok;
>      __libc_res_hnok;
>      __ns_name_compress;
> diff --git a/resolv/ns_makecanon.c b/resolv/ns_makecanon.c
> new file mode 100644
> index 0000000000..a6c3c2737d
> --- /dev/null
> +++ b/resolv/ns_makecanon.c
> @@ -0,0 +1,50 @@
> +/* Add missing "." to domain names.

OK.

> + * Copyright (c) 2004 by Internet Systems Consortium, Inc. ("ISC")
> + * Copyright (c) 1995,1999 by Internet Software Consortium.
> + *
> + * Permission to use, copy, modify, and distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND INTERNET SOFTWARE CONSORTIUM DISCLAIMS
> + * ALL WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL INTERNET SOFTWARE
> + * CONSORTIUM BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL
> + * DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR
> + * PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS
> + * ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS
> + * SOFTWARE.
> + */
> +
> +#include <arpa/nameser.h>
> +#include <errno.h>
> +#include <string.h>
> +
> +/* Make a canonical copy of domain name SRC in DST.  Behavior:
> +      foo -> foo.
> +      foo. -> foo.
> +      foo.. -> foo.
> +      foo\. -> foo\..
> +      foo\\. -> foo\\.  */

OK.

> +int
> +__libc_ns_makecanon (const char *src, char *dst, size_t dstsize)
> +{
> +  size_t n = strlen (src);
> +
> +  if (n + sizeof "." > dstsize) /* sizeof == 2.  */
> +    {
> +      __set_errno (EMSGSIZE);
> +      return -1;
> +    }
> +  strcpy (dst, src);
> +  while (n >= 1U && dst[n - 1] == '.')   /* Ends in ".".  */
> +    if (n >= 2U && dst[n - 2] == '\\' && /* Ends in "\.".  */
> +        (n < 3U || dst[n - 3] != '\\'))  /* But not "\\.".  */
> +      break;
> +    else
> +      dst[--n] = '\0';

OK.

> +  dst[n++] = '.';
> +  dst[n] = '\0';
> +  return 0;

OK.

> +}
> +libc_hidden_def (__libc_ns_makecanon)
> diff --git a/resolv/ns_samedomain.c b/resolv/ns_samedomain.c
> index 5d1bf39fc7..cfff2516b0 100644
> --- a/resolv/ns_samedomain.c
> +++ b/resolv/ns_samedomain.c
> @@ -143,40 +143,6 @@ ns_subdomain(const char *a, const char *b) {
>  	return (ns_samename(a, b) != 1 && ns_samedomain(a, b));
>  }
>  
> -/*%
> - *	make a canonical copy of domain name "src"
> - *
> - * notes:
> - * \code
> - *	foo -> foo.
> - *	foo. -> foo.
> - *	foo.. -> foo.
> - *	foo\. -> foo\..
> - *	foo\\. -> foo\\.
> - * \endcode
> - */
> -
> -int
> -ns_makecanon(const char *src, char *dst, size_t dstsize) {
> -	size_t n = strlen(src);
> -
> -	if (n + sizeof "." > dstsize) {			/*%< Note: sizeof == 2 */
> -		__set_errno (EMSGSIZE);
> -		return (-1);
> -	}
> -	strcpy(dst, src);
> -	while (n >= 1U && dst[n - 1] == '.')		/*%< Ends in "." */
> -		if (n >= 2U && dst[n - 2] == '\\' &&	/*%< Ends in "\." */
> -		    (n < 3U || dst[n - 3] != '\\'))	/*%< But not "\\." */
> -			break;
> -		else
> -			dst[--n] = '\0';
> -	dst[n++] = '.';
> -	dst[n] = '\0';
> -	return (0);
> -}
> -libresolv_hidden_def (ns_makecanon)
> -

OK. Remove.

>  /*%
>   *	determine whether domain name "a" is the same as domain name "b"
>   *
> @@ -190,8 +156,8 @@ int
>  ns_samename(const char *a, const char *b) {
>  	char ta[NS_MAXDNAME], tb[NS_MAXDNAME];
>  
> -	if (ns_makecanon(a, ta, sizeof ta) < 0 ||
> -	    ns_makecanon(b, tb, sizeof tb) < 0)
> +	if (__libc_ns_makecanon(a, ta, sizeof ta) < 0 ||
> +	    __libc_ns_makecanon(b, tb, sizeof tb) < 0)

OK.

>  		return (-1);
>  	if (strcasecmp(ta, tb) == 0)
>  		return (1);
> diff --git a/resolv/resolv-deprecated.c b/resolv/resolv-deprecated.c
> new file mode 100644
> index 0000000000..cbd1078dc2
> --- /dev/null
> +++ b/resolv/resolv-deprecated.c
> @@ -0,0 +1,35 @@
> +/* Forwarders for deprecated libresolv functions which are implemented in libc.
> +   Copyright (C) 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/>.  */
> +
> +/* Some functions are used by the stub resolver implementation
> +   internally and thus have to be located in libc.  They have been
> +   historially exported for application use as well.  However, the
> +   stub resolver dependency on those functions is not intrinsic to
> +   what the stub resolver does, and it is possible that a future
> +   version of the stub resolver will not need them anymore.  The
> +   public symbols for these functions remain in libresolv, and are not
> +   moved to libc, to avoid adding new symbol versions for legacy
> +   functions.  */
> +
> +#include <arpa/nameser.h>
> +
> +int
> +ns_makecanon (const char *src, char *dst, size_t dstsize)
> +{
> +  return __libc_ns_makecanon (src, dst, dstsize);
> +}

OK.

> 


-- 
Cheers,
Carlos.