Re: [PATCH 02/30] resolv: Deprecate legacy interfaces in <resolv.h>

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

On 7/8/21 10:59 AM, Florian Weimer via Libc-alpha wrote:
> Debugging interfaces: p_*, fp_*, and sym_* could conceivably be
> used to produce debug out, but these functions have not been
> updated to parse more resource records, so they are not very useful
> today.  Likewise for ns_sprintrr and ns_sprintrrf.  ns_format_ttl and
> ns_parse_ttl are related to these.
> 
> Internal implementation details: res_isourserver is probably only
> useful in the implementation of a stub resolver, and so is
> res_nameinquery.
> 
> Unclear semantics and bad performance: ns_samedomain, ns_subdomain,
> ns_makecanon, ns_samename do textual converions & copies instead of
> checking equivalence of the wire format.
> 
> res_hostalias has been superseded by getaddrinfo with AI_CANONNAME.
> hostalias is not thread-safe.
> 
> Some functions have int as size arguments instead of size_t, so they
> do not follow current coding practices.  However, dn_expand and
> b64_ntop are somewhat widely used (to name just two examples), so
> deprecating them seems problematic.

I like the direction we go with this patch. I have always felt that
many of these functions should be deprecated and eventually removed.
I have even hemmed and hawed about either updating them or adding
man page documents for them, but I think they are ill suited to having
in glibc. Many of them belong in a dedicated DNS library where they
are kept up to date following the evolution of DNS.

I think we need a full list in the NEWS of all the deprecated interfaces
though just for the sake of searchability and to make it clear what
is being deprecated.

Please post v2 with NEWS list of deprecated functions and then I'll
ACK it for glibc 2.34.

Tested without regression on x86_64 and i686.

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

> ---
>  NEWS                  |  4 +++
>  resolv/arpa/nameser.h | 33 ++++++++++++-----
>  resolv/resolv.h       | 84 +++++++++++++++++++++++++++----------------
>  3 files changed, 82 insertions(+), 39 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 8e72946c3f..23ff7fd104 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -77,6 +77,10 @@ Deprecated and removed features, and other changes affecting compatibility:
>  * The function pthread_yield has been deprecated; programs should use
>    the equivalent standard function sched_yield instead.
>  
> +* Various rarely-used functions declared in <resolv.h> and
> +  <arpa/nameser.h> have been deprecated.  Applications are encouraged to
> +  use dedicated DNS processing libraries if applicable.

Please list the exact functions deprecated in NEWS.

> +
>  * The pthread cancellation handler is now installed with SA_RESTART and
>    pthread_cancel will always send the internal SIGCANCEL on a cancellation
>    request.  It should not be visible to application since the cancellation
> diff --git a/resolv/arpa/nameser.h b/resolv/arpa/nameser.h
> index a99d5ec508..017d7b194a 100644
> --- a/resolv/arpa/nameser.h
> +++ b/resolv/arpa/nameser.h
> @@ -52,6 +52,12 @@
>  #include <sys/types.h>
>  #include <stdint.h>
>  
> +#ifdef _LIBC
> +# define __NAMESER_DEPRECATED
> +#else
> +# define __NAMESER_DEPRECATED __attribute_deprecated__
> +#endif

Adhemerval noted that this could be problematic because this is a
public header.

I don't think this is the case because usr/include/sys/cdefs.h includes
a definition for __attribute_deprecated__ specifically for use in
all the other headers we have. We use __attribute_deprecated__ in
10+ headers today.

I reviewed the installed public headers and they look correct to me.

> +
>  /*
>   * Define constants based on RFC 883, RFC 1034, RFC 1035
>   */
> @@ -401,14 +407,18 @@ int		ns_skiprr (const unsigned char *, const unsigned char *,
>  int		ns_parserr (ns_msg *, ns_sect, int, ns_rr *) __THROW;
>  int		ns_sprintrr (const ns_msg *, const ns_rr *,
>  			     const char *, const char *, char *, size_t)
> -     __THROW;
> +  __THROW __NAMESER_DEPRECATED;
>  int		ns_sprintrrf (const unsigned char *, size_t, const char *,
>  			      ns_class, ns_type, unsigned long,
>  			      const unsigned char *, size_t, const char *,
> -			      const char *, char *, size_t) __THROW;
> -int		ns_format_ttl (unsigned long, char *, size_t) __THROW;
> -int		ns_parse_ttl (const char *, unsigned long *) __THROW;
> -uint32_t	ns_datetosecs (const char *, int *) __THROW;
> +			      const char *, char *, size_t)
> +  __THROW __NAMESER_DEPRECATED;

OK.

> +int		ns_format_ttl (unsigned long, char *, size_t)
> +  __THROW __NAMESER_DEPRECATED;

OK.

> +int		ns_parse_ttl (const char *, unsigned long *)
> +  __THROW __NAMESER_DEPRECATED;

OK.

> +uint32_t	ns_datetosecs (const char *, int *)
> +  __THROW __NAMESER_DEPRECATED;

OK.

>  int		ns_name_ntol (const unsigned char *, unsigned char *, size_t)
>       __THROW;
>  int		ns_name_ntop (const unsigned char *, char *, size_t) __THROW;
> @@ -431,10 +441,15 @@ int		ns_name_skip (const unsigned char **, const unsigned char *)
>  void		ns_name_rollback (const unsigned char *,
>  				  const unsigned char **,
>  				  const unsigned char **) __THROW;
> -int		ns_samedomain (const char *, const char *) __THROW;
> -int		ns_subdomain (const char *, const char *) __THROW;
> -int		ns_makecanon (const char *, char *, size_t) __THROW;
> -int		ns_samename (const char *, const char *) __THROW;
> +
> +int		ns_samedomain (const char *, const char *) __THROW
> +  __NAMESER_DEPRECATED;

OK.

> +int		ns_subdomain (const char *, const char *) __THROW
> +  __NAMESER_DEPRECATED;

OK.

> +int		ns_makecanon (const char *, char *, size_t) __THROW
> +  __NAMESER_DEPRECATED;

OK.

> +int		ns_samename (const char *, const char *) __THROW
> +  __NAMESER_DEPRECATED;

OK.

>  __END_DECLS
>  
>  #include <arpa/nameser_compat.h>
> diff --git a/resolv/resolv.h b/resolv/resolv.h
> index a039a9e636..d55942a6b6 100644
> --- a/resolv/resolv.h
> +++ b/resolv/resolv.h
> @@ -174,14 +174,27 @@ __END_DECLS
>  #define res_search		__res_search
>  #define res_send		__res_send
>  
> +#ifdef _LIBC
> +# define __RESOLV_DEPRECATED
> +# define __RESOLV_DEPRECATED_MSG(msg)
> +#else
> +# define __RESOLV_DEPRECATED __attribute_deprecated__
> +# define __RESOLV_DEPRECATED_MSG(msg) __attribute_deprecated_msg__ (msg)
> +#endif
> +
>  __BEGIN_DECLS
> -void		fp_nquery (const unsigned char *, int, FILE *) __THROW;
> -void		fp_query (const unsigned char *, FILE *) __THROW;
> -const char *	hostalias (const char *) __THROW;
> -void		p_query (const unsigned char *) __THROW;
> +void		fp_nquery (const unsigned char *, int, FILE *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

> +void		fp_query (const unsigned char *, FILE *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

> +const char *	hostalias (const char *) __THROW
> +  __RESOLV_DEPRECATED_MSG ("use getaddrinfo instead");

OK.

> +void		p_query (const unsigned char *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

>  void		res_close (void) __THROW;
>  int		res_init (void) __THROW;
> -int		res_isourserver (const struct sockaddr_in *) __THROW;
> +int		res_isourserver (const struct sockaddr_in *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

>  int		res_mkquery (int, const char *, int, int,
>  			     const unsigned char *, int, const unsigned char *,
>  			     unsigned char *, int) __THROW;
> @@ -238,50 +251,61 @@ int		res_hnok (const char *) __THROW;
>  int		res_ownok (const char *) __THROW;
>  int		res_mailok (const char *) __THROW;
>  int		res_dnok (const char *) __THROW;
> -int		sym_ston (const struct res_sym *, const char *, int *) __THROW;
> -const char *	sym_ntos (const struct res_sym *, int, int *) __THROW;
> -const char *	sym_ntop (const struct res_sym *, int, int *) __THROW;
> +int		sym_ston (const struct res_sym *, const char *, int *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

> +const char *	sym_ntos (const struct res_sym *, int, int *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

> +const char *	sym_ntop (const struct res_sym *, int, int *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

>  int		b64_ntop (const unsigned char *, size_t, char *, size_t)
> -     __THROW;
> +  __THROW;
>  int		b64_pton (char const *, unsigned char *, size_t) __THROW;
> -int		loc_aton (const char *__ascii, unsigned char *__binary) __THROW;
> -const char *	loc_ntoa (const unsigned char *__binary, char *__ascii) __THROW;
> +int		loc_aton (const char *__ascii, unsigned char *__binary) __THROW
> +  __RESOLV_DEPRECATED;

OK.

> +const char *	loc_ntoa (const unsigned char *__binary, char *__ascii) __THROW
> +  __RESOLV_DEPRECATED;

OK.

>  int		dn_skipname (const unsigned char *, const unsigned char *)
> -     __THROW;
> -void		putlong (uint32_t, unsigned char *) __THROW;
> -void		putshort (uint16_t, unsigned char *) __THROW;
> -const char *	p_class (int) __THROW;
> -const char *	p_time (uint32_t) __THROW;
> -const char *	p_type (int) __THROW;
> -const char *	p_rcode (int) __THROW;
> -const unsigned char * p_cdnname (const unsigned char *,
> -				 const unsigned char *, int, FILE *) __THROW;
> +  __THROW;
> +void		putlong (uint32_t, unsigned char *) __THROW
> +  __RESOLV_DEPRECATED_MSG ("use NS_PUT16 instead");

OK.

> +void		putshort (uint16_t, unsigned char *) __THROW
> +  __RESOLV_DEPRECATED_MSG ("use NS_PUT32 instead");

OK.

> +const char *	p_class (int) __THROW __RESOLV_DEPRECATED;
OK.

> +const char *	p_time (uint32_t) __THROW __RESOLV_DEPRECATED;

OK.

> +const char *	p_type (int) __THROW __RESOLV_DEPRECATED;

OK.

> +const char *	p_rcode (int) __THROW __RESOLV_DEPRECATED;

OK.

> +const unsigned char * p_cdnname (const unsigned char *, const unsigned char *,
> +				 int, FILE *) __THROW __RESOLV_DEPRECATED;

OK.

>  const unsigned char * p_cdname (const unsigned char *, const unsigned char *,
> -				FILE *) __THROW;
> +				FILE *) __THROW __RESOLV_DEPRECATED;

OK.

>  const unsigned char * p_fqnname (const unsigned char *__cp,
>  				 const unsigned char *__msg,
> -				 int, char *, int) __THROW;
> -const unsigned char * p_fqname (const unsigned char *,
> -				const unsigned char *, FILE *) __THROW;
> -const char *	p_option (unsigned long __option) __THROW;
> +				 int, char *, int) __THROW __RESOLV_DEPRECATED;

OK.

> +const unsigned char * p_fqname (const unsigned char *, const unsigned char *,
> +				FILE *) __THROW __RESOLV_DEPRECATED;

OK.

> +const char *	p_option (unsigned long __option) __THROW __RESOLV_DEPRECATED;

OK.

>  int		dn_count_labels (const char *) __THROW;
>  int		dn_comp (const char *, unsigned char *, int, unsigned char **,
>  			 unsigned char **) __THROW;
>  int		dn_expand (const unsigned char *, const unsigned char *,
>  			   const unsigned char *, char *, int) __THROW;
> -unsigned int	res_randomid (void) __THROW;
> +unsigned int	res_randomid (void) __THROW
> +  __RESOLV_DEPRECATED_MSG ("use getentropy instead");

OK. Good message.

>  int		res_nameinquery (const char *, int, int,
>  				 const unsigned char *,
> -				 const unsigned char *) __THROW;
> +				 const unsigned char *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

>  int		res_queriesmatch (const unsigned char *,
>  				  const unsigned char *,
>  				  const unsigned char *,
> -				  const unsigned char *) __THROW;
> +				  const unsigned char *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

>  /* Things involving a resolver context. */
>  int		res_ninit (res_state) __THROW;
> -void		fp_resstat (const res_state, FILE *) __THROW;
> +void		fp_resstat (const res_state, FILE *) __THROW
> +  __RESOLV_DEPRECATED;

OK.

>  const char *	res_hostalias (const res_state, const char *, char *, size_t)
> -     __THROW;
> +     __THROW __RESOLV_DEPRECATED_MSG ("use getaddrinfo instead");

OK.


>  int		res_nquery (res_state, const char *, int, int,
>  			    unsigned char *, int) __THROW;
>  int		res_nsearch (res_state, const char *, int, int,
> 


-- 
Cheers,
Carlos.