[PATCH] string: Add page tests for test-strncasecmp and test-strncpy

[Lucas A. M. Magalhaes]

Hi Raphael, Thanks for the patch :)

Overall it will be best if there was some comments for the magic numbers and
test functions. I think Paul point that. 

Quoting Raphael Moreira Zinsly via Libc-alpha (2020-05-29 10:53:05)
> Adds tests to check if strings placed at page bondaries are
> handled correctly by strncasecmp and strncpy similar to tests
> for strncmp and strnlen.  This should catch regressions in the
> optmized functions.
> ---
>  string/test-strncasecmp.c | 39 +++++++++++++++++++++++++++++++++++++++
>  string/test-strncpy.c     | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+)
> 
> diff --git a/string/test-strncasecmp.c b/string/test-strncasecmp.c
> index 6a9c27beae..0399f37117 100644
> --- a/string/test-strncasecmp.c
> +++ b/string/test-strncasecmp.c
> @@ -137,6 +137,44 @@ do_test (size_t align1, size_t align2, size_t n, size_t len, int max_char,
>      do_one_test (impl, s1, s2, n, exp_result);
>  }
>  
> +static void
> +do_page_tests (void)
> +{
> +  size_t i, offset1, offset2;
> +  char *s1, *s2, *s3;
> +  int exp_result;
> +
> +  s1 = (char *) buf1;

What is the size of buf1 1 page_size?

> +  for (i = 0; i < page_size - 1; i++)
> +    s1[i] = 23;
> +  s1[i] = 0;
> +
> +  s3 = strdup (s1);
> +  offset2 = 2636;
> +
> +  for (i = 0; i < 64; ++i)
> +    {
> +      offset1 = 3988 + i;
> +
The max size of offset1 is 4052.  AFAIK this will never cross a page. So
the check bellow is not necessarie.
> +      if (offset1 >= page_size
> +         || offset2 >= page_size)
> +       return;

Why do you check offset2 here?  As it never increments you could check it
outside of the loop. But AFAIK 2636 will never cross a page.
> +
> +      s1 = (char *) buf1;
I notice that o the other function you use (CHAR *) instead of (char *)
> +      s2 = s3;
> +      s1 += offset1;

Well, a sum is a sum. But Why don't use a simpler for like:

s1 = magic_number1 + buf1
s2 = magic_number2 + s3

for (i = 0; i < 64; ++i){
	s1++;
	s2++;
	do test
{

> +      s2 += offset2;
> +
> +      exp_result= *s1;
> +
> +      FOR_EACH_IMPL (impl, 0)
> +       {
> +         check_result (impl, s1, s2, page_size, -exp_result);
> +         check_result (impl, s2, s1, page_size, exp_result);
> +       }
> +    }
> +}
> +
>  static void
>  do_random_tests (void)
>  {
> @@ -334,6 +372,7 @@ test_locale (const char *locale)
>      }
>  
>    do_random_tests ();
> +  do_page_tests ();
>  }
>  
>  int
> diff --git a/string/test-strncpy.c b/string/test-strncpy.c
> index c978753ad8..30c69dd34b 100644
> --- a/string/test-strncpy.c
> +++ b/string/test-strncpy.c
> @@ -155,6 +155,42 @@ do_test (size_t align1, size_t align2, size_t len, size_t n, int max_char)
>      do_one_test (impl, s2, s1, len, n);
>  }
>  
> +static void
> +do_page_tests (void)
> +{
> +  size_t i, len, start_offset, offset;
> +  CHAR *s1, *s2;
> +  /* Calculate the null character offset.  */
> +  size_t last_offset = (page_size / sizeof (CHAR)) - 1;
> +
> +  s2 = (CHAR *) buf1;
> +  s1 = (CHAR *) buf2;
> +  MEMSET (s1, '\1', last_offset);
> +  s1[last_offset] = '\0';
Ok.

> +
> +  /* Place short strings ending at page boundary.  */
> +  offset = last_offset;
> +  for (i = 0; i < 128; i++)
> +    {
> +      offset--;
> +      len = last_offset - offset;
> +      FOR_EACH_IMPL (impl, 0)
> +       do_one_test (impl, s2, (CHAR *) (s1 + offset), len, len);
> +    }
Ok.
> +
> +  /* Place long strings ending at page boundary.  */
> +  start_offset = (last_offset + 1) / 2;
> +  for (i = 0; i < 64; ++i)
> +    {
> +      offset = start_offset + i;
> +      if (offset >= (last_offset + 1))
> +       break;

Is this possible?

> +      len = last_offset - offset;
> +      FOR_EACH_IMPL (impl, 0)
> +       do_one_test (impl, s2, (CHAR *) (s1 + offset), page_size, len);
> +    }
> +}
> +
>  static void
>  do_random_tests (void)
>  {
> @@ -317,6 +353,7 @@ test_main (void)
>      }
>  
>    do_random_tests ();
> +  do_page_tests ();
>    return ret;
>  }
>  
> -- 
> 2.26.2
> 
Ok.

---
Lucas A. M. Magalhães