[v2] Make tst-strtod2 and tst-strtod5 type-generic

Message ID 82e5c8ec-c3fc-60db-6a6b-7ed22a56d67f@redhat.com
State Committed
Commit 8de031bcb9adfa736c0caed2c79d10947b8d8f48
Headers
Series [v2] Make tst-strtod2 and tst-strtod5 type-generic |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Joseph Myers Aug. 13, 2024, 10:14 p.m. UTC
  Make tst-strtod2 and tst-strtod5 type-generic

Some of the strtod tests use type-generic machinery in tst-strtod.h to
test the strto* functions for all floating types, while others only
test double even when the tests are in fact meaningful for all
floating types.

Convert tst-strtod2 and tst-strtod5 to use the type-generic machinery
so they test all floating types.  I haven't tried to convert them to
use newer test interfaces in other ways, just made the changes
necessary to use the type-generic machinery.

Tested for x86_64.

---

This is intended the be the first of several such patches improving
strto* test coverage.  For example, tst-strtod1i.c, tst-strtod3.c,
tst-strtod4.c and tst-strtod5i.c can be changed similarly to cover all
floating types, with some adjustments to take account of how they are
actually testing an internal interface __strtod_internal.
tst-strtod-underflow.c will be a more complicated case to adjust
because each floating format needs its own test input strings.

Changed in v2: one hardcoded "strtod" reference in a printf in cases
of test failure was updated to reflect the function under test.
  

Comments

Joseph Myers Aug. 20, 2024, 7:31 p.m. UTC | #1
Ping.  This patch 
<https://sourceware.org/pipermail/libc-alpha/2024-August/159183.html> is 
pending review.
  
Joseph Myers Aug. 30, 2024, 1:04 p.m. UTC | #2
Ping^2.  This patch 
<https://sourceware.org/pipermail/libc-alpha/2024-August/159183.html> is 
still pending review.
  
Joseph Myers Sept. 6, 2024, 7:31 p.m. UTC | #3
Ping^3.  This patch 
<https://sourceware.org/pipermail/libc-alpha/2024-August/159183.html> is 
still pending review.
  
Simon Chopin Sept. 15, 2024, 11:15 a.m. UTC | #4
Hi Joseph

On ven. 06 sept. 2024 19:31:56, Joseph Myers wrote:
> Ping^3.  This patch
> <https://sourceware.org/pipermail/libc-alpha/2024-August/159183.html> is
> still pending review.

My apologies. This patch is in my queue, and I haven't had much time
this summer for reviews. I'll have a look this coming week.

>
> --
> Joseph S. Myers
> josmyers@redhat.com
>
  
Joseph Myers Sept. 20, 2024, 9:30 p.m. UTC | #5
Ping^4.  This patch 
<https://sourceware.org/pipermail/libc-alpha/2024-August/159183.html> is 
still pending review.
  
Carlos O'Donell Sept. 20, 2024, 9:59 p.m. UTC | #6
On 8/13/24 6:14 PM, Joseph Myers wrote:
> Make tst-strtod2 and tst-strtod5 type-generic
> 
> Some of the strtod tests use type-generic machinery in tst-strtod.h to
> test the strto* functions for all floating types, while others only
> test double even when the tests are in fact meaningful for all
> floating types.
> 
> Convert tst-strtod2 and tst-strtod5 to use the type-generic machinery
> so they test all floating types.  I haven't tried to convert them to
> use newer test interfaces in other ways, just made the changes
> necessary to use the type-generic machinery.
> 
> Tested for x86_64.

LGTM.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
> 
> This is intended the be the first of several such patches improving
> strto* test coverage.  For example, tst-strtod1i.c, tst-strtod3.c,
> tst-strtod4.c and tst-strtod5i.c can be changed similarly to cover all
> floating types, with some adjustments to take account of how they are
> actually testing an internal interface __strtod_internal.
> tst-strtod-underflow.c will be a more complicated case to adjust
> because each floating format needs its own test input strings.
> 
> Changed in v2: one hardcoded "strtod" reference in a printf in cases
> of test failure was updated to reflect the function under test.
> 
> diff --git a/stdlib/tst-strtod2.c b/stdlib/tst-strtod2.c
> index a7df82ebbd..2cb0953fa9 100644
> --- a/stdlib/tst-strtod2.c
> +++ b/stdlib/tst-strtod2.c
> @@ -1,43 +1,61 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  
> -struct test
> -{
> -  const char *str;
> -  double result;
> -  size_t offset;
> -} tests[] =
> -{
> -  { "0xy", 0.0, 1 },
> -  { "0x.y", 0.0, 1 },
> -  { "0x0.y", 0.0, 4 },
> -  { "0x.0y", 0.0, 4 },
> -  { ".y", 0.0, 0 },
> -  { "0.y", 0.0, 2 },
> -  { ".0y", 0.0, 2 }
> -};
> +#include "tst-strtod.h"

OK. Include the type-generic machinery from tst-strtod.h.

> +
> +#define TEST_STRTOD(FSUF, FTYPE, FTOSTR, LSUF, CSUF)			\
> +struct test_strto ## FSUF						\
> +{									\
> +  const char *str;							\
> +  FTYPE result;								\
> +  size_t offset;							\
> +} tests_strto ## FSUF[] =						\
> +{									\
> +  { "0xy", 0.0 ## LSUF, 1 },						\
> +  { "0x.y", 0.0 ## LSUF, 1 },						\
> +  { "0x0.y", 0.0 ## LSUF, 4 },						\
> +  { "0x.0y", 0.0 ## LSUF, 4 },						\
> +  { ".y", 0.0 ## LSUF, 0 },						\
> +  { "0.y", 0.0 ## LSUF, 2 },						\
> +  { ".0y", 0.0 ## LSUF, 2 }						\

OK. 7 tests before 7 after.

> +};									\
> +									\
> +static int								\
> +test_strto ## FSUF (void)						\

OK. Generic test.

> +{									\
> +  int status = 0;							\
> +  for (size_t i = 0;							\
> +       i < sizeof (tests_strto ## FSUF) / sizeof (tests_strto ## FSUF[0]); \
> +       ++i)								\
> +    {									\
> +      char *ep;								\
> +      FTYPE r = strto ## FSUF (tests_strto ## FSUF[i].str, &ep);	\
> +      if (r != tests_strto ## FSUF[i].result)				\
> +	{								\
> +	  char buf1[FSTRLENMAX], buf2[FSTRLENMAX];			\
> +	  FTOSTR (buf1, sizeof (buf1), "%g", r);			\
> +	  FTOSTR (buf2, sizeof (buf2), "%g", tests_strto ## FSUF[i].result); \
> +	  printf ("test %zu r = %s, expect %s\n", i, buf1, buf2);	\
> +	  status = 1;							\
> +	}								\
> +      if (ep != tests_strto ## FSUF[i].str + tests_strto ## FSUF[i].offset) \
> +	{								\
> +	  printf ("test %zu strto" #FSUF				\
> +		  " parsed %tu characters, expected %zu\n",		\
> +		  i, ep - tests_strto ## FSUF[i].str,			\
> +		  tests_strto ## FSUF[i].offset);			\
> +	  status = 1;							\
> +	}								\
> +    }									\
> +  return status;							\
> +}
> +
> +GEN_TEST_STRTOD_FOREACH (TEST_STRTOD)
>  
>  static int
>  do_test (void)
>  {
> -  int status = 0;
> -  for (size_t i = 0; i < sizeof (tests) / sizeof (tests[0]); ++i)
> -    {
> -      char *ep;
> -      double r = strtod (tests[i].str, &ep);
> -      if (r != tests[i].result)
> -	{
> -	  printf ("test %zu r = %g, expect %g\n", i, r, tests[i].result);
> -	  status = 1;
> -	}
> -      if (ep != tests[i].str + tests[i].offset)
> -	{
> -	  printf ("test %zu strtod parsed %tu characters, expected %zu\n",
> -		  i, ep - tests[i].str, tests[i].offset);
> -	  status = 1;
> -	}
> -    }
> -  return status;

OK. This is all converted above into the macro.

> +  return STRTOD_TEST_FOREACH (test_strto);

OK. Use generic machinery for testing.

>  }
>  
>  #define TEST_FUNCTION do_test ()
> diff --git a/stdlib/tst-strtod5.c b/stdlib/tst-strtod5.c
> index 29153ec005..7eb9b3a2d7 100644
> --- a/stdlib/tst-strtod5.c
> +++ b/stdlib/tst-strtod5.c
> @@ -22,35 +22,75 @@
>  #include <string.h>
>  #include <math.h>
>  
> +#include "tst-strtod.h"
> +
>  #define NBSP "\xc2\xa0"
>  
> -static const struct
> -{
> -  const char *in;
> -  double expected;
> -} tests[] =
> -  {
> -    { "0", 0.0 },
> -    { "000", 0.0 },
> -    { "-0", -0.0 },
> -    { "-000", -0.0 },
> -    { "0,", 0.0 },
> -    { "-0,", -0.0 },
> -    { "0,0", 0.0 },
> -    { "-0,0", -0.0 },
> -    { "0e-10", 0.0 },
> -    { "-0e-10", -0.0 },
> -    { "0,e-10", 0.0 },
> -    { "-0,e-10", -0.0 },
> -    { "0,0e-10", 0.0 },
> -    { "-0,0e-10", -0.0 },
> -    { "0e-1000000", 0.0 },
> -    { "-0e-1000000", -0.0 },
> -    { "0,0e-1000000", 0.0 },
> -    { "-0,0e-1000000", -0.0 },
> -  };
> -#define NTESTS (sizeof (tests) / sizeof (tests[0]))
> +#define TEST_STRTOD(FSUF, FTYPE, FTOSTR, LSUF, CSUF)			\
> +static const struct							\
> +{									\
> +  const char *in;							\
> +  FTYPE expected;							\
> +} tests_strto ## FSUF[] =						\
> +  {									\
> +    { "0", 0.0 ## LSUF },						\
> +    { "000", 0.0 ## LSUF },						\
> +    { "-0", -0.0 ## LSUF },						\
> +    { "-000", -0.0 ## LSUF },						\
> +    { "0,", 0.0 ## LSUF },						\
> +    { "-0,", -0.0 ## LSUF },						\
> +    { "0,0", 0.0 ## LSUF },						\
> +    { "-0,0", -0.0 ## LSUF },						\
> +    { "0e-10", 0.0 ## LSUF },						\
> +    { "-0e-10", -0.0 ## LSUF },						\
> +    { "0,e-10", 0.0 ## LSUF },						\
> +    { "-0,e-10", -0.0 ## LSUF },					\
> +    { "0,0e-10", 0.0 ## LSUF },						\
> +    { "-0,0e-10", -0.0 ## LSUF },					\
> +    { "0e-1000000", 0.0 ## LSUF },					\
> +    { "-0e-1000000", -0.0 ## LSUF },					\
> +    { "0,0e-1000000", 0.0 ## LSUF },					\
> +    { "-0,0e-1000000", -0.0 ## LSUF },					\

OK. 18 tests before 18 tests after conversion.

> +  };									\
> +									\
> +									\
> +static int								\
> +test_strto ## FSUF (void)						\
> +{									\
> +  int status = 0;							\
> +									\
> +  for (int i = 0;							\
> +       i < sizeof (tests_strto ## FSUF) / sizeof (tests_strto ## FSUF[0]); \
> +       ++i)								\
> +    {									\
> +      char *ep;								\
> +      FTYPE r = strto ## FSUF (tests_strto ## FSUF[i].in, &ep);		\
> +									\
> +      if (*ep != '\0')							\
> +	{								\
> +	  printf ("%d: got rest string \"%s\", expected \"\"\n", i, ep); \
> +	  status = 1;							\
> +	}								\
> +									\
> +      if (r != tests_strto ## FSUF[i].expected				\
> +	  || (copysign ## CSUF (10.0 ## LSUF, r)			\
> +	      != copysign ## CSUF (10.0 ## LSUF,			\
> +				   tests_strto ## FSUF[i].expected)))	\
> +	{								\
> +	  char buf1[FSTRLENMAX], buf2[FSTRLENMAX];			\
> +	  FTOSTR (buf1, sizeof (buf1), "%g", r);			\
> +	  FTOSTR (buf2, sizeof (buf2), "%g",				\
> +		  tests_strto ## FSUF[i].expected);			\
> +	  printf ("%d: got wrong results %s, expected %s\n",		\
> +		  i, buf1, buf2);					\
> +	  status = 1;							\
> +	}								\
> +    }									\
> +									\
> +  return status;							\

OK. Generic test.

> +}
>  
> +GEN_TEST_STRTOD_FOREACH (TEST_STRTOD)
>  
>  static int
>  do_test (void)
> @@ -61,29 +101,7 @@ do_test (void)
>        return 1;
>      }
>  
> -  int status = 0;
> -
> -  for (int i = 0; i < NTESTS; ++i)
> -    {
> -      char *ep;
> -      double r = strtod (tests[i].in, &ep);
> -
> -      if (*ep != '\0')
> -	{
> -	  printf ("%d: got rest string \"%s\", expected \"\"\n", i, ep);
> -	  status = 1;
> -	}
> -
> -      if (r != tests[i].expected
> -	  || copysign (10.0, r) != copysign (10.0, tests[i].expected))
> -	{
> -	  printf ("%d: got wrong results %g, expected %g\n",
> -		  i, r, tests[i].expected);
> -	  status = 1;
> -	}
> -    }
> -
> -  return status;

OK. Refactor to above.

> +  return STRTOD_TEST_FOREACH (test_strto);
>  }
>  
>  #include <support/test-driver.c>
>
  

Patch

diff --git a/stdlib/tst-strtod2.c b/stdlib/tst-strtod2.c
index a7df82ebbd..2cb0953fa9 100644
--- a/stdlib/tst-strtod2.c
+++ b/stdlib/tst-strtod2.c
@@ -1,43 +1,61 @@ 
 #include <stdio.h>
 #include <stdlib.h>
 
-struct test
-{
-  const char *str;
-  double result;
-  size_t offset;
-} tests[] =
-{
-  { "0xy", 0.0, 1 },
-  { "0x.y", 0.0, 1 },
-  { "0x0.y", 0.0, 4 },
-  { "0x.0y", 0.0, 4 },
-  { ".y", 0.0, 0 },
-  { "0.y", 0.0, 2 },
-  { ".0y", 0.0, 2 }
-};
+#include "tst-strtod.h"
+
+#define TEST_STRTOD(FSUF, FTYPE, FTOSTR, LSUF, CSUF)			\
+struct test_strto ## FSUF						\
+{									\
+  const char *str;							\
+  FTYPE result;								\
+  size_t offset;							\
+} tests_strto ## FSUF[] =						\
+{									\
+  { "0xy", 0.0 ## LSUF, 1 },						\
+  { "0x.y", 0.0 ## LSUF, 1 },						\
+  { "0x0.y", 0.0 ## LSUF, 4 },						\
+  { "0x.0y", 0.0 ## LSUF, 4 },						\
+  { ".y", 0.0 ## LSUF, 0 },						\
+  { "0.y", 0.0 ## LSUF, 2 },						\
+  { ".0y", 0.0 ## LSUF, 2 }						\
+};									\
+									\
+static int								\
+test_strto ## FSUF (void)						\
+{									\
+  int status = 0;							\
+  for (size_t i = 0;							\
+       i < sizeof (tests_strto ## FSUF) / sizeof (tests_strto ## FSUF[0]); \
+       ++i)								\
+    {									\
+      char *ep;								\
+      FTYPE r = strto ## FSUF (tests_strto ## FSUF[i].str, &ep);	\
+      if (r != tests_strto ## FSUF[i].result)				\
+	{								\
+	  char buf1[FSTRLENMAX], buf2[FSTRLENMAX];			\
+	  FTOSTR (buf1, sizeof (buf1), "%g", r);			\
+	  FTOSTR (buf2, sizeof (buf2), "%g", tests_strto ## FSUF[i].result); \
+	  printf ("test %zu r = %s, expect %s\n", i, buf1, buf2);	\
+	  status = 1;							\
+	}								\
+      if (ep != tests_strto ## FSUF[i].str + tests_strto ## FSUF[i].offset) \
+	{								\
+	  printf ("test %zu strto" #FSUF				\
+		  " parsed %tu characters, expected %zu\n",		\
+		  i, ep - tests_strto ## FSUF[i].str,			\
+		  tests_strto ## FSUF[i].offset);			\
+	  status = 1;							\
+	}								\
+    }									\
+  return status;							\
+}
+
+GEN_TEST_STRTOD_FOREACH (TEST_STRTOD)
 
 static int
 do_test (void)
 {
-  int status = 0;
-  for (size_t i = 0; i < sizeof (tests) / sizeof (tests[0]); ++i)
-    {
-      char *ep;
-      double r = strtod (tests[i].str, &ep);
-      if (r != tests[i].result)
-	{
-	  printf ("test %zu r = %g, expect %g\n", i, r, tests[i].result);
-	  status = 1;
-	}
-      if (ep != tests[i].str + tests[i].offset)
-	{
-	  printf ("test %zu strtod parsed %tu characters, expected %zu\n",
-		  i, ep - tests[i].str, tests[i].offset);
-	  status = 1;
-	}
-    }
-  return status;
+  return STRTOD_TEST_FOREACH (test_strto);
 }
 
 #define TEST_FUNCTION do_test ()
diff --git a/stdlib/tst-strtod5.c b/stdlib/tst-strtod5.c
index 29153ec005..7eb9b3a2d7 100644
--- a/stdlib/tst-strtod5.c
+++ b/stdlib/tst-strtod5.c
@@ -22,35 +22,75 @@ 
 #include <string.h>
 #include <math.h>
 
+#include "tst-strtod.h"
+
 #define NBSP "\xc2\xa0"
 
-static const struct
-{
-  const char *in;
-  double expected;
-} tests[] =
-  {
-    { "0", 0.0 },
-    { "000", 0.0 },
-    { "-0", -0.0 },
-    { "-000", -0.0 },
-    { "0,", 0.0 },
-    { "-0,", -0.0 },
-    { "0,0", 0.0 },
-    { "-0,0", -0.0 },
-    { "0e-10", 0.0 },
-    { "-0e-10", -0.0 },
-    { "0,e-10", 0.0 },
-    { "-0,e-10", -0.0 },
-    { "0,0e-10", 0.0 },
-    { "-0,0e-10", -0.0 },
-    { "0e-1000000", 0.0 },
-    { "-0e-1000000", -0.0 },
-    { "0,0e-1000000", 0.0 },
-    { "-0,0e-1000000", -0.0 },
-  };
-#define NTESTS (sizeof (tests) / sizeof (tests[0]))
+#define TEST_STRTOD(FSUF, FTYPE, FTOSTR, LSUF, CSUF)			\
+static const struct							\
+{									\
+  const char *in;							\
+  FTYPE expected;							\
+} tests_strto ## FSUF[] =						\
+  {									\
+    { "0", 0.0 ## LSUF },						\
+    { "000", 0.0 ## LSUF },						\
+    { "-0", -0.0 ## LSUF },						\
+    { "-000", -0.0 ## LSUF },						\
+    { "0,", 0.0 ## LSUF },						\
+    { "-0,", -0.0 ## LSUF },						\
+    { "0,0", 0.0 ## LSUF },						\
+    { "-0,0", -0.0 ## LSUF },						\
+    { "0e-10", 0.0 ## LSUF },						\
+    { "-0e-10", -0.0 ## LSUF },						\
+    { "0,e-10", 0.0 ## LSUF },						\
+    { "-0,e-10", -0.0 ## LSUF },					\
+    { "0,0e-10", 0.0 ## LSUF },						\
+    { "-0,0e-10", -0.0 ## LSUF },					\
+    { "0e-1000000", 0.0 ## LSUF },					\
+    { "-0e-1000000", -0.0 ## LSUF },					\
+    { "0,0e-1000000", 0.0 ## LSUF },					\
+    { "-0,0e-1000000", -0.0 ## LSUF },					\
+  };									\
+									\
+									\
+static int								\
+test_strto ## FSUF (void)						\
+{									\
+  int status = 0;							\
+									\
+  for (int i = 0;							\
+       i < sizeof (tests_strto ## FSUF) / sizeof (tests_strto ## FSUF[0]); \
+       ++i)								\
+    {									\
+      char *ep;								\
+      FTYPE r = strto ## FSUF (tests_strto ## FSUF[i].in, &ep);		\
+									\
+      if (*ep != '\0')							\
+	{								\
+	  printf ("%d: got rest string \"%s\", expected \"\"\n", i, ep); \
+	  status = 1;							\
+	}								\
+									\
+      if (r != tests_strto ## FSUF[i].expected				\
+	  || (copysign ## CSUF (10.0 ## LSUF, r)			\
+	      != copysign ## CSUF (10.0 ## LSUF,			\
+				   tests_strto ## FSUF[i].expected)))	\
+	{								\
+	  char buf1[FSTRLENMAX], buf2[FSTRLENMAX];			\
+	  FTOSTR (buf1, sizeof (buf1), "%g", r);			\
+	  FTOSTR (buf2, sizeof (buf2), "%g",				\
+		  tests_strto ## FSUF[i].expected);			\
+	  printf ("%d: got wrong results %s, expected %s\n",		\
+		  i, buf1, buf2);					\
+	  status = 1;							\
+	}								\
+    }									\
+									\
+  return status;							\
+}
 
+GEN_TEST_STRTOD_FOREACH (TEST_STRTOD)
 
 static int
 do_test (void)
@@ -61,29 +101,7 @@  do_test (void)
       return 1;
     }
 
-  int status = 0;
-
-  for (int i = 0; i < NTESTS; ++i)
-    {
-      char *ep;
-      double r = strtod (tests[i].in, &ep);
-
-      if (*ep != '\0')
-	{
-	  printf ("%d: got rest string \"%s\", expected \"\"\n", i, ep);
-	  status = 1;
-	}
-
-      if (r != tests[i].expected
-	  || copysign (10.0, r) != copysign (10.0, tests[i].expected))
-	{
-	  printf ("%d: got wrong results %g, expected %g\n",
-		  i, r, tests[i].expected);
-	  status = 1;
-	}
-    }
-
-  return status;
+  return STRTOD_TEST_FOREACH (test_strto);
 }
 
 #include <support/test-driver.c>