diff mbox series

Update string/test-memmove.c to cover 16KB copy

Message ID 20210819191021.3766894-1-hjl.tools@gmail.com
State Superseded
Headers show
Series Update string/test-memmove.c to cover 16KB copy | expand

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

H.J. Lu Aug. 19, 2021, 7:10 p.m. UTC
---
 string/test-memmove.c | 49 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

Comments

Noah Goldstein Aug. 20, 2021, 3:09 a.m. UTC | #1
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.

>
>
H.J. Lu Aug. 20, 2021, 2:08 p.m. UTC | #2
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.
diff mbox series

Patch

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)
+{
+  /* Copy 16KB data.  */
+  size_t bytes_move = 16384;
+  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];
+
+#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;
 }