Re: Add restrict annotations to all functions that require it

[Joseph Myers <joseph@codesourcery.com>]

On Sat, 26 Nov 2022, Seija K. via Libc-alpha wrote:

> The format value has to be a string literal, every time. Otherwise, you are
> not using these functions correctly. To reinforce this fact, I put
> __restrict over every example of this I could find.

There are several other changes in this patch that aren't adding 
__restrict.  Maybe they are a good idea, maybe they aren't, but they 
aren't described by the proposed commit message, and a patch should do one 
thing and do it well (so if those other changes are desirable, they should 
go in a separate patch with its own commit message explaining them).

For both patches, it would also be helpful to know whether they result in 
any changes to installed stripped shared libraries.

> diff --git a/string/memcpy.c b/string/memcpy.c
> index 9cf64530e73..920683e3d8c 100644
> --- a/string/memcpy.c
> +++ b/string/memcpy.c
> @@ -25,10 +25,10 @@
>  #endif
> 
>  void *
> -MEMCPY (void *dstpp, const void *srcpp, size_t len)
> +MEMCPY (void *__restrict dstpp, const void *__restrict srcpp, size_t len)
>  {
> -  unsigned long int dstp = (long int) dstpp;
> -  unsigned long int srcp = (long int) srcpp;
> +  unsigned long int dstp = (unsigned long int) dstpp;
> +  unsigned long int srcp = (unsigned long int) srcpp;

This is an example of the other changes.

> diff --git a/string/memfrob.c b/string/memfrob.c
> index 9707cb9fda1..025929f90cb 100644
> --- a/string/memfrob.c
> +++ b/string/memfrob.c
> @@ -22,7 +22,7 @@ memfrob (void *s, size_t n)
>  {
>    char *p = (char *) s;
> 
> -  while (n-- > 0)
> +  for (; n; n--)
>      *p++ ^= 42;

Likewise.

> diff --git a/string/memrchr.c b/string/memrchr.c
> index 8eb6829e458..7fd7f666c84 100644
> --- a/string/memrchr.c
> +++ b/string/memrchr.c
> @@ -63,9 +63,8 @@ MEMRCHR
>    const unsigned char *char_ptr;
>    const unsigned long int *longword_ptr;
>    unsigned long int longword, magic_bits, charmask;
> -  unsigned char c;
> 
> -  c = (unsigned char) c_in;
> +  const unsigned char c = (const unsigned char) c_in;
> 
>    /* Handle the last few characters by reading one character at a time.
>       Do this until CHAR_PTR is aligned on a longword boundary.  */
> @@ -182,7 +181,7 @@ MEMRCHR
> 
>    char_ptr = (const unsigned char *) longword_ptr;
> 
> -  while (n-- > 0)
> +  for (; n > 0; --n)

Likewise.

> diff --git a/string/memset.c b/string/memset.c
> index 1303dd7ad36..273bd5d6ff0 100644
> --- a/string/memset.c
> +++ b/string/memset.c
> @@ -26,7 +26,7 @@ void *
>  inhibit_loop_to_libcall
>  MEMSET (void *dstpp, int c, size_t len)
>  {
> -  long int dstp = (long int) dstpp;
> +  unsigned long int dstp = (unsigned long int) dstpp;

Likewise.

> diff --git a/string/stpncpy.c b/string/stpncpy.c
> index 61f0134fb20..7346d790a1a 100644
> --- a/string/stpncpy.c
> +++ b/string/stpncpy.c
> @@ -37,9 +37,9 @@ weak_alias (__stpncpy, stpncpy)
>  /* Copy no more than N characters of SRC to DEST, returning the address of
>     the terminating '\0' in DEST, if any, or else DEST + N.  */
>  char *
> -STPNCPY (char *dest, const char *src, size_t n)
> +STPNCPY (char * __restrict dest, const char * __restrict src, size_t n)
>  {
> -  size_t size = __strnlen (src, n);
> +  const size_t size = __strnlen (src, n);

Likewise.

> diff --git a/string/strlen.c b/string/strlen.c
> index 54f3fb8167a..f443fd16d86 100644
> --- a/string/strlen.c
> +++ b/string/strlen.c
> @@ -33,6 +33,9 @@ STRLEN (const char *str)
>    const unsigned long int *longword_ptr;
>    unsigned long int longword, himagic, lomagic;
> 
> +  if (sizeof (longword) > 8)
> +    abort ();
> +
>    /* Handle the first few characters by reading one character at a time.
>       Do this until CHAR_PTR is aligned on a longword boundary.  */
>    for (char_ptr = str; ((unsigned long int) char_ptr
> @@ -59,8 +62,6 @@ STRLEN (const char *str)
>        himagic = ((himagic << 16) << 16) | himagic;
>        lomagic = ((lomagic << 16) << 16) | lomagic;
>      }
> -  if (sizeof (longword) > 8)
> -    abort ();

Likewise (and if you change anything here, surely using a static assertion 
is better than this "if").

> diff --git a/string/strxfrm_l.c b/string/strxfrm_l.c
> index 188a3d826a6..79596a9c6de 100644
> --- a/string/strxfrm_l.c
> +++ b/string/strxfrm_l.c
> @@ -441,7 +441,7 @@ do_xfrm_cached (STRING_TYPE *dest, size_t n, const
> locale_data_t *l_data,
>    len = weights[idxarr[backw]++];
> 
>    if (needed + len < n)
> -    while (len-- > 0)
> +    for (; len; --len)
>        dest[needed++] = weights[idxarr[backw]++];
>    else
>      {
> @@ -457,7 +457,7 @@ do_xfrm_cached (STRING_TYPE *dest, size_t n, const
> locale_data_t *l_data,
>    /* Now handle the forward element.  */
>    len = weights[idxarr[idxcnt]++];
>    if (needed + len < n)
> -    while (len-- > 0)
> +    for (; len; --len)
>        dest[needed++] = weights[idxarr[idxcnt]++];
>    else
>      {
> @@ -488,7 +488,7 @@ do_xfrm_cached (STRING_TYPE *dest, size_t n, const
> locale_data_t *l_data,
>    size_t len = weights[idxarr[--backw]++];
> 
>    if (needed + len < n)
> -    while (len-- > 0)
> +    for (; len; --len)
>        dest[needed++] = weights[idxarr[backw]++];
>    else

Likewise.

> diff --git a/string/swab.c b/string/swab.c
> index 083a80091f7..6aecb93c5eb 100644
> --- a/string/swab.c
> +++ b/string/swab.c
> @@ -18,15 +18,15 @@
>  #include <unistd.h>
> 
>  void
> -swab (const void *bfrom, void *bto, ssize_t n)
> +swab (const void *__restrict bfrom, void *__restrict bto, ssize_t n)
>  {
> -  const char *from = (const char *) bfrom;
> -  char *to = (char *) bto;
> +  const unsigned char *from = (const unsigned char *) bfrom;
> +  unsigned char *to = (unsigned char *) bto;
> 
>    n &= ~((ssize_t) 1);
>    while (n > 1)
>      {
> -      const char b0 = from[--n], b1 = from[--n];
> +      const unsigned char b0 = from[--n], b1 = from[--n];

Likewise.

> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
> index 005d089501f..922eed6deae 100644
> --- a/sysdeps/x86_64/dl-machine.h
> +++ b/sysdeps/x86_64/dl-machine.h
> @@ -416,17 +416,14 @@ and creates an unsatisfiable circular dependency.\n",
>    value += reloc->r_addend;
>    *(unsigned int *) reloc_addr = value;
> 
> -  const char *fmt;

Likewise; lots of changes here with nothing apparently to do with 
restrict.

-- 
Joseph S. Myers
joseph@codesourcery.com