Re: [PATCH] Update string/test-memmove.c to cover 16KB copy

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Thu, Aug 19, 2021, 2:10 PM H.J. Lu via Libc-alpha <
libc-alpha@sourceware.org> wrote:

> ---
>  string/test-memmove.c | 49 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/string/test-memmove.c b/string/test-memmove.c
> index b271248b1d..812f6bde9e 100644
> --- a/string/test-memmove.c
> +++ b/string/test-memmove.c
> @@ -312,6 +312,52 @@ do_test2 (size_t offset)
>    munmap ((void *) large_buf, size);
>  }
>
> +static void
> +do_test3 (void)
>

I think it may be cleaner to make
bytes_move and offset function arguments.

+{
> +  /* Copy 16KB data.  */
> +  size_t bytes_move = 16384;
>

I think this should be a function argument.

+  size_t size = 16384 * 3;
> +  uint32_t *buf;
> +
> +  buf = mmap (NULL, size, PROT_READ | PROT_WRITE,
> +             MAP_PRIVATE | MAP_ANON, -1, 0);
> +
> +  if (buf == MAP_FAILED)
> +    error (EXIT_UNSUPPORTED, errno, "mmap failed");
> +
> +  size_t arr_size = bytes_move / sizeof (uint32_t);
> +  size_t i;
> +
> +  FOR_EACH_IMPL (impl, 0)
> +    {
> +      for (i = 0; i < arr_size; i++)
> +        buf[i] = (uint32_t) i;
> +
> +      uint32_t *dst = &buf[arr_size + 3];
>

I think + 3 should be a variable and passed to the function.

+
> +#ifdef TEST_BCOPY
> +      CALL (impl, (char *) buf, (char *) dst, bytes_move);
> +#else
> +      CALL (impl, (char *) dst, (char *) buf, bytes_move);
> +#endif
> +
> +      for (i = 0; i < arr_size; i++)
> +       {
> +         if (dst[i] != (uint32_t) i)
> +           {
> +             error (0, 0,
> +                    "Wrong result in function %s dst \"%p\" src \"%p\"
> offset \"%zd\"",
> +                    impl->name, dst, buf, i);
> +             ret = 1;
> +             break;
> +           }
> +       }
> +    }
> +
> +  munmap ((void *) buf, size);
> +}
> +
>  int
>  test_main (void)
>  {
> @@ -356,6 +402,9 @@ test_main (void)
>    do_test2 (0x200000);
>    do_test2 (0x4000000 - 1);
>    do_test2 (0x4000000);
> +
> +  do_test3 ();

+
>    return ret;
>  }
>
> --
> 2.31.1
>

Okay with this patch but see comments.

>
>