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

["H.J. Lu" <hjl.tools@gmail.com>]

On Thu, Aug 19, 2021 at 8:10 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
>
>
> 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.

Fixed.

>> +{
>> +  /* Copy 16KB data.  */
>> +  size_t bytes_move = 16384;
>
>
> I think this should be a function argument.

Fixed.

>> +  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.

Fixed.

>> +
>> +#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.

I sent out the v2 patch.

Thanks.

-- 
H.J.