Re: [PATCH] Clean up redundancies between string.h and strings.h.

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 16/02/2017 13:03, Zack Weinberg wrote:
> string.h contains redundant, slightly different declarations of all
> the functions whose official home is strings.h, and strings.h in turn
> has a bunch of messy #ifdeffage to avoid a conflict.  This patch removes
> all of that and just has string.h include strings.h under __USE_MISC.
> I also corrected some comments regarding str[n]casecmp_l, which were
> not standardized when they were added to glibc, but are now.
> 
> (Archaeology indicates that bits/string2.h inlines once used __bzero;
> it doesn't anymore, and I believe Wilco has a patch to remove the
> #define that remains.  The remaining uses of __bzero are all in
> sunrpc.  If we got rid of them we could also simplify all the sysdeps
> definitions of memset a bit, but I'm not sure it's worth bothering.)
> 
> Tested on x86-64-linux, OK to commit?

LGTM, checked with make check run-builts-tests=no on aarch64, arm, and
powerpc64le just for sanity.

> 
> zw
> 
> 	* string/string.h [__USE_MISC]: Include strings.h.
> 	(__bzero, bcmp, bcopy, bzero, index, rindex)
> 	(strcasecmp, strncasecmp, strcasecmp_l, strncasecmp_l)
> 	(ffs, ffsl, ffsll): Don't declare.
> 
> 	* string/strings.h: Do not suppress the file if string.h has
> 	already been included.
> 	(bcmp, bcopy, bzero, strcasecmp, strncasecmp): Add __nonnull
> 	annotations.
> 	(index, rindex): Define inline forwarders even if
> 	__CORRECT_ISO_CPP_STRING_H_PROTO is defined.
> 	(ffs): Use __attribute_const__.
> 	(ffsl, ffsll): Declare here.
> 	(strcasecmp_l, strncasecmp_l): Correct comments; these functions
> 	have now been standardized.
> 
> 	* include/string.h (__bzero): Declare here.
> ---
>  include/string.h |   2 ++
>  string/string.h  | 108 ++-----------------------------------------------------
>  string/strings.h |  70 ++++++++++++++++++------------------
>  3 files changed, 39 insertions(+), 141 deletions(-)
> 
> diff --git a/include/string.h b/include/string.h
> index 45eca3c11c..f166de9c43 100644
> --- a/include/string.h
> +++ b/include/string.h
> @@ -41,6 +41,8 @@ extern void *__memrchr (const void *__s, int __c, size_t __n)
>  extern void *__memchr (const void *__s, int __c, size_t __n)
>       __attribute_pure__;
>  
> +extern void __bzero (void *__s, size_t __n) __THROW __nonnull ((1));
> +
>  extern int __ffs (int __i) __attribute__ ((const));
>  
>  extern char *__strerror_r (int __errnum, char *__buf, size_t __buflen);
> diff --git a/string/string.h b/string/string.h
> index b31afadfaa..053cf334a7 100644
> --- a/string/string.h
> +++ b/string/string.h
> @@ -440,117 +440,13 @@ extern char *strerror_r (int __errnum, char *__buf, size_t __buflen)
>  extern char *strerror_l (int __errnum, __locale_t __l) __THROW;
>  #endif
>  
> -
> -/* We define this function always since `bzero' is sometimes needed when
> -   the namespace rules does not allow this.  */
> -extern void __bzero (void *__s, size_t __n) __THROW __nonnull ((1));
> -
>  #ifdef __USE_MISC
> -/* Copy N bytes of SRC to DEST (like memmove, but args reversed).  */
> -extern void bcopy (const void *__src, void *__dest, size_t __n)
> -     __THROW __nonnull ((1, 2));
> +# include <strings.h>
>  
> -/* Set N bytes of S to 0.  */
> -extern void bzero (void *__s, size_t __n) __THROW __nonnull ((1));
> -
> -/* As bzero, but the compiler will not delete a call to this
> +/* Set N bytes of S to 0.  The compiler will not delete a call to this
>     function, even if S is dead after the call.  */
>  extern void explicit_bzero (void *__s, size_t __n) __THROW __nonnull ((1));
>  
> -/* Compare N bytes of S1 and S2 (same as memcmp).  */
> -extern int bcmp (const void *__s1, const void *__s2, size_t __n)
> -     __THROW __attribute_pure__ __nonnull ((1, 2));
> -
> -/* Find the first occurrence of C in S (same as strchr).  */
> -# ifdef __CORRECT_ISO_CPP_STRING_H_PROTO
> -extern "C++"
> -{
> -extern char *index (char *__s, int __c)
> -     __THROW __asm ("index") __attribute_pure__ __nonnull ((1));
> -extern const char *index (const char *__s, int __c)
> -     __THROW __asm ("index") __attribute_pure__ __nonnull ((1));
> -
> -#  if defined __OPTIMIZE__ && !defined __CORRECT_ISO_CPP_STRINGS_H_PROTO
> -__extern_always_inline char *
> -index (char *__s, int __c) __THROW
> -{
> -  return __builtin_index (__s, __c);
> -}
> -
> -__extern_always_inline const char *
> -index (const char *__s, int __c) __THROW
> -{
> -  return __builtin_index (__s, __c);
> -}
> -#  endif
> -}
> -# else
> -extern char *index (const char *__s, int __c)
> -     __THROW __attribute_pure__ __nonnull ((1));
> -# endif
> -
> -/* Find the last occurrence of C in S (same as strrchr).  */
> -# ifdef __CORRECT_ISO_CPP_STRING_H_PROTO
> -extern "C++"
> -{
> -extern char *rindex (char *__s, int __c)
> -     __THROW __asm ("rindex") __attribute_pure__ __nonnull ((1));
> -extern const char *rindex (const char *__s, int __c)
> -     __THROW __asm ("rindex") __attribute_pure__ __nonnull ((1));
> -
> -#  if defined __OPTIMIZE__ && !defined __CORRECT_ISO_CPP_STRINGS_H_PROTO
> -__extern_always_inline char *
> -rindex (char *__s, int __c) __THROW
> -{
> -  return __builtin_rindex (__s, __c);
> -}
> -
> -__extern_always_inline const char *
> -rindex (const char *__s, int __c) __THROW
> -{
> -  return __builtin_rindex (__s, __c);
> -}
> -#endif
> -}
> -# else
> -extern char *rindex (const char *__s, int __c)
> -     __THROW __attribute_pure__ __nonnull ((1));
> -# endif
> -
> -/* Return the position of the first bit set in I, or 0 if none are set.
> -   The least-significant bit is position 1, the most-significant 32.  */
> -extern int ffs (int __i) __THROW __attribute__ ((__const__));
> -
> -/* The following two functions are non-standard but necessary for non-32 bit
> -   platforms.  */
> -# ifdef	__USE_GNU
> -extern int ffsl (long int __l) __THROW __attribute__ ((__const__));
> -__extension__ extern int ffsll (long long int __ll)
> -     __THROW __attribute__ ((__const__));
> -# endif
> -
> -/* Compare S1 and S2, ignoring case.  */
> -extern int strcasecmp (const char *__s1, const char *__s2)
> -     __THROW __attribute_pure__ __nonnull ((1, 2));
> -
> -/* Compare no more than N chars of S1 and S2, ignoring case.  */
> -extern int strncasecmp (const char *__s1, const char *__s2, size_t __n)
> -     __THROW __attribute_pure__ __nonnull ((1, 2));
> -#endif /* Use misc.  */
> -
> -#ifdef	__USE_GNU
> -/* Again versions of a few functions which use the given locale instead
> -   of the global one.  */
> -extern int strcasecmp_l (const char *__s1, const char *__s2,
> -			 __locale_t __loc)
> -     __THROW __attribute_pure__ __nonnull ((1, 2, 3));
> -
> -extern int strncasecmp_l (const char *__s1, const char *__s2,
> -			  size_t __n, __locale_t __loc)
> -     __THROW __attribute_pure__ __nonnull ((1, 2, 4));
> -#endif
> -
> -#ifdef	__USE_MISC
>  /* Return the next DELIM-delimited token from *STRINGP,
>     terminating it with a '\0', and update *STRINGP to point past it.  */
>  extern char *strsep (char **__restrict __stringp,
> diff --git a/string/strings.h b/string/strings.h
> index 69b75b1219..43207af09c 100644
> --- a/string/strings.h
> +++ b/string/strings.h
> @@ -18,35 +18,31 @@
>  #ifndef	_STRINGS_H
>  #define	_STRINGS_H	1
>  
> -/* We don't need and should not read this file if <string.h> was already
> -   read. The one exception being that if __USE_MISC isn't defined, then
> -   these aren't defined in string.h, so we need to define them here.  */
> -#if !defined _STRING_H || !defined __USE_MISC
> -
> -# include <features.h>
> -# define __need_size_t
> -# include <stddef.h>
> +#include <features.h>
> +#define __need_size_t
> +#include <stddef.h>
>  
>  /* Tell the caller that we provide correct C++ prototypes.  */
> -# if defined __cplusplus && __GNUC_PREREQ (4, 4)
> -#  define __CORRECT_ISO_CPP_STRINGS_H_PROTO
> -# endif
> +#if defined __cplusplus && __GNUC_PREREQ (4, 4)
> +# define __CORRECT_ISO_CPP_STRINGS_H_PROTO
> +#endif
>  
>  __BEGIN_DECLS
>  
> -# if defined __USE_MISC || !defined __USE_XOPEN2K8
> +#if defined __USE_MISC || !defined __USE_XOPEN2K8
>  /* Compare N bytes of S1 and S2 (same as memcmp).  */
>  extern int bcmp (const void *__s1, const void *__s2, size_t __n)
> -     __THROW __attribute_pure__;
> +     __THROW __attribute_pure__ __nonnull ((1, 2));
>  
>  /* Copy N bytes of SRC to DEST (like memmove, but args reversed).  */
> -extern void bcopy (const void *__src, void *__dest, size_t __n) __THROW;
> +extern void bcopy (const void *__src, void *__dest, size_t __n)
> +  __THROW __nonnull ((1, 2));
>  
>  /* Set N bytes of S to 0.  */
> -extern void bzero (void *__s, size_t __n) __THROW;
> +extern void bzero (void *__s, size_t __n) __THROW __nonnull ((1));
>  
>  /* Find the first occurrence of C in S (same as strchr).  */
> -#  ifdef __CORRECT_ISO_CPP_STRINGS_H_PROTO
> +# ifdef __CORRECT_ISO_CPP_STRINGS_H_PROTO
>  extern "C++"
>  {
>  extern char *index (char *__s, int __c)
> @@ -54,7 +50,7 @@ extern char *index (char *__s, int __c)
>  extern const char *index (const char *__s, int __c)
>       __THROW __asm ("index") __attribute_pure__ __nonnull ((1));
>  
> -#   if defined __OPTIMIZE__ && !defined __CORRECT_ISO_CPP_STRING_H_PROTO
> +#  if defined __OPTIMIZE__
>  __extern_always_inline char *
>  index (char *__s, int __c) __THROW
>  {
> @@ -66,15 +62,15 @@ index (const char *__s, int __c) __THROW
>  {
>    return __builtin_index (__s, __c);
>  }
> -#   endif
> +#  endif
>  }
> -#  else
> +# else
>  extern char *index (const char *__s, int __c)
>       __THROW __attribute_pure__ __nonnull ((1));
> -#  endif
> +# endif
>  
>  /* Find the last occurrence of C in S (same as strrchr).  */
> -#  ifdef __CORRECT_ISO_CPP_STRINGS_H_PROTO
> +# ifdef __CORRECT_ISO_CPP_STRINGS_H_PROTO
>  extern "C++"
>  {
>  extern char *rindex (char *__s, int __c)
> @@ -82,7 +78,7 @@ extern char *rindex (char *__s, int __c)
>  extern const char *rindex (const char *__s, int __c)
>       __THROW __asm ("rindex") __attribute_pure__ __nonnull ((1));
>  
> -#   if defined __OPTIMIZE__ && !defined __CORRECT_ISO_CPP_STRING_H_PROTO
> +#  if defined __OPTIMIZE__
>  __extern_always_inline char *
>  rindex (char *__s, int __c) __THROW
>  {
> @@ -94,39 +90,45 @@ rindex (const char *__s, int __c) __THROW
>  {
>    return __builtin_rindex (__s, __c);
>  }
> -#   endif
> +#  endif
>  }
> -#  else
> +# else
>  extern char *rindex (const char *__s, int __c)
>       __THROW __attribute_pure__ __nonnull ((1));
> -#  endif
>  # endif
> +#endif
>  
>  #if defined __USE_MISC || !defined __USE_XOPEN2K8 || defined __USE_XOPEN2K8XSI
>  /* Return the position of the first bit set in I, or 0 if none are set.
>     The least-significant bit is position 1, the most-significant 32.  */
> -extern int ffs (int __i) __THROW __attribute__ ((const));
> +extern int ffs (int __i) __THROW __attribute_const__;
>  #endif
>  
> +/* The following two functions are non-standard but necessary for non-32 bit
> +   platforms.  */
> +# ifdef	__USE_GNU
> +extern int ffsl (long int __l) __THROW __attribute_const__;
> +__extension__ extern int ffsll (long long int __ll)
> +     __THROW __attribute_const__;
> +# endif
> +
>  /* Compare S1 and S2, ignoring case.  */
>  extern int strcasecmp (const char *__s1, const char *__s2)
> -     __THROW __attribute_pure__;
> +     __THROW __attribute_pure__ __nonnull ((1, 2));
>  
>  /* Compare no more than N chars of S1 and S2, ignoring case.  */
>  extern int strncasecmp (const char *__s1, const char *__s2, size_t __n)
> -     __THROW __attribute_pure__;
> +     __THROW __attribute_pure__ __nonnull ((1, 2));
>  
>  #ifdef	__USE_XOPEN2K8
> -/* The following functions are equivalent to the both above but they
> -   take the locale they use for the collation as an extra argument.
> -   This is not standardsized but something like will come.  */
>  # include <xlocale.h>
>  
> -/* Again versions of a few functions which use the given locale instead
> -   of the global one.  */
> +/* Compare S1 and S2, ignoring case, using collation rules from LOC.  */
>  extern int strcasecmp_l (const char *__s1, const char *__s2, __locale_t __loc)
>       __THROW __attribute_pure__ __nonnull ((1, 2, 3));
>  
> +/* Compare no more than N chars of S1 and S2, ignoring case, using
> +   collation rules from LOC.  */
>  extern int strncasecmp_l (const char *__s1, const char *__s2,
>  			  size_t __n, __locale_t __loc)
>       __THROW __attribute_pure__ __nonnull ((1, 2, 4));
> @@ -134,8 +136,6 @@ extern int strncasecmp_l (const char *__s1, const char *__s2,
>  
>  __END_DECLS
>  
> -#endif	/* string.h  */
> -
>  #if __GNUC_PREREQ (3,4) && __USE_FORTIFY_LEVEL > 0 \
>      && defined __fortify_function
>  /* Functions with security checks.  */
>