diff mbox

Add tst-wcstod-round

Message ID 70c6242f-931e-56e7-e719-32d063303f00@linux.vnet.ibm.com
State New
Headers show

Commit Message

Paul E. Murphy Aug. 4, 2016, 2:50 p.m. UTC
This extends tst-strtod-round with a few trivial changes
to also test the wide character variants of strto* using
similar macros to other shared tests.

	* stdlib/tst-strtod-round.c:
	(L_): New macro to select string encoding.
	(FNPFX): New macro to select str or wcs prefix.
	(CHAR): New macro to choose wchar_t or char.
	(STRM): New macro to choose printf modifier for above.
	(STRTO): New macro to choose appropriate string -> real
	function.
	(TEST_WIDE): Conditional definition to enable wchar_t
	testing.
	(FNPFXS): "wcs" or "str" dependent on [TEST_WIDE].
	(STR): Support for above macro.
	(STRX): Likewise.

	(TEST): Update with above macros.
	(test): Likewise.
	(GEN_ONE_TEST): Likewise.
	(test_in_one_mode): Likewise.

	* wcsmbs/tst-wcstod-round.c: New file.

	* wcsmbs/Makefile: (tests): Add tst-wcstod-round
	(tst-wcstod-round): Add libm depencency for fesetround.
---
 stdlib/tst-strtod-round.c | 43 +++++++++++++++++++++++++++++++++----------
 wcsmbs/Makefile           |  3 +++
 wcsmbs/tst-wcstod-round.c |  3 +++
 3 files changed, 39 insertions(+), 10 deletions(-)
 create mode 100644 wcsmbs/tst-wcstod-round.c

Comments

Carlos O'Donell Aug. 4, 2016, 4:17 p.m. UTC | #1
On 08/04/2016 10:50 AM, Paul E. Murphy wrote:
> This extends tst-strtod-round with a few trivial changes
> to also test the wide character variants of strto* using
> similar macros to other shared tests.
> 
> 	* stdlib/tst-strtod-round.c:
> 	(L_): New macro to select string encoding.
> 	(FNPFX): New macro to select str or wcs prefix.
> 	(CHAR): New macro to choose wchar_t or char.
> 	(STRM): New macro to choose printf modifier for above.
> 	(STRTO): New macro to choose appropriate string -> real
> 	function.
> 	(TEST_WIDE): Conditional definition to enable wchar_t
> 	testing.
> 	(FNPFXS): "wcs" or "str" dependent on [TEST_WIDE].
> 	(STR): Support for above macro.
> 	(STRX): Likewise.
> 
> 	(TEST): Update with above macros.
> 	(test): Likewise.
> 	(GEN_ONE_TEST): Likewise.
> 	(test_in_one_mode): Likewise.
> 
> 	* wcsmbs/tst-wcstod-round.c: New file.
> 
> 	* wcsmbs/Makefile: (tests): Add tst-wcstod-round
> 	(tst-wcstod-round): Add libm depencency for fesetround.

At a high-level the patch is perfect.

At a implementation level I'd like to see a little more reorg
to avoid macro API typos.

See below. Otherwise this looks good to me.

> ---
>  stdlib/tst-strtod-round.c | 43 +++++++++++++++++++++++++++++++++----------
>  wcsmbs/Makefile           |  3 +++
>  wcsmbs/tst-wcstod-round.c |  3 +++
>  3 files changed, 39 insertions(+), 10 deletions(-)
>  create mode 100644 wcsmbs/tst-wcstod-round.c
> 
> diff --git a/stdlib/tst-strtod-round.c b/stdlib/tst-strtod-round.c
> index 509f96a..023d382 100644
> --- a/stdlib/tst-strtod-round.c
> +++ b/stdlib/tst-strtod-round.c
> @@ -31,9 +31,26 @@
>  
>  #include "tst-strtod.h"
>  
> +/* Build the test for wide or normal character strings.  */
> +#ifdef TEST_WIDE

You should always define TEST_WIDE and set it either to 0 or 1,
but instead I suggest removing it and doing what I suggest below.

You should still read this:
https://sourceware.org/glibc/wiki/Wundef

> +# define L_(str) L ## str
> +# define FNPFX wcs
> +# define CHAR wchar_t
> +# define STRM "%S"
> +# define snprintf swprintf
> +# define strfromf128 wcsfromf128

This above block of defines should live in tst-wcstod-round.c since
that's the code that needs them, like other similar tests.

> +#else
> +# define L_(str) str
> +# define FNPFX str
> +# define CHAR char
> +# define STRM "%s"

These are the defaults macro API values and should always bet set.
I suggest moving this file into a generic test source e.g.
tst-strtox-round.c and then have tst-wcstod-round define the above
defaults and include the generic version.

This avoid macro typos which result in the wrong thing being
tested.

> +#endif
> +
>  #define _CONCAT(a, b) a ## b
>  #define CONCAT(a, b) _CONCAT (a, b)
>  
> +#define STRTO(x) CONCAT (CONCAT (FNPFX, to), x)
> +
>  #if LDBL_MANT_DIG == 106 && LDBL_MAX_EXP == 1024
>  /* This is a stupid hack for IBM long double.  This test ignores
>     inexact values for long double due to the limitations of the
> @@ -110,7 +127,7 @@
>  	     ld106x, ld106d, ld106n, ld106z, ld106u,	\
>  	     ld113x, ld113d, ld113n, ld113z, ld113u)	\
>    {							\
> -    s,							\
> +    L_ (s),						\
>      { XNTRY (fx, dx, ld64ix, ld64mx, ld106x, ld113x) },	\
>      {							\
>      { ENTRY (fn, dn, ld64in, ld64mn, ld106n, ld113n) },	\
> @@ -131,7 +148,7 @@ struct test_results
>    };
>  
>  struct test {
> -  const char *s;
> +  const CHAR *s;
>    struct test_exactness exact;
>    struct test_results r[4];
>  };
> @@ -139,19 +156,25 @@ struct test {
>  /* Include the generated test data.  */
>  #include "tst-strtod-round-data.h"
>  
> +#define STRX(x) #x
> +#define STR(x) STRX (x)
> +#define FNPFXS STR (FNPFX)
> +
>  #define GEN_ONE_TEST(FSUF, FTYPE, FTOSTR, FTOSTRM, LSUF, CSUF)	\
>  {								\
> -  FTYPE f = strto ## FSUF (s, NULL);				\
> +  FTYPE f = STRTO (FSUF) (s, NULL);				\
>    if (f != expected->FSUF					\
>        || (copysign ## CSUF) (1.0 ## LSUF, f)			\
>  	 != (copysign ## CSUF) (1.0 ## LSUF, expected->FSUF))	\
>      {								\
> -      char efstr[FSTRLENMAX];					\
> -      char fstr[FSTRLENMAX];					\
> -      FTOSTR (efstr, FSTRLENMAX, "%" FTOSTRM "a", expected->FSUF); \
> -      FTOSTR (fstr, FSTRLENMAX, "%" FTOSTRM "a", f);		\
> -      printf ("strto" #FSUF " (%s) returned %s not %s"		\
> -	      " (%s)\n", s, fstr, efstr, mode_name);		\
> +      CHAR efstr[FSTRLENMAX];					\
> +      CHAR fstr[FSTRLENMAX];					\
> +      FTOSTR (efstr, FSTRLENMAX, L_("%") L_(FTOSTRM) L_("a"),   \
> +	      expected->FSUF);    				\
> +      FTOSTR (fstr, FSTRLENMAX, L_("%") L_(FTOSTRM) L_("a"), f);\
> +      printf (FNPFXS "to" #FSUF  " (" STRM ") returned " STRM   \
> +	      " not " STRM " (%s)\n",				\
> +	      s, fstr, efstr, mode_name);			\
>        if (ROUNDING_TESTS (FTYPE, rnd_mode) || exact->FSUF)	\
>  	result = 1;						\
>        else							\
> @@ -160,7 +183,7 @@ struct test {
>  }
>  
>  static int
> -test_in_one_mode (const char *s, const struct test_results *expected,
> +test_in_one_mode (const CHAR *s, const struct test_results *expected,
>  		  const struct test_exactness *exact, const char *mode_name,
>  		  int rnd_mode)
>  {
> diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile
> index 8b599f7..9384a10 100644
> --- a/wcsmbs/Makefile
> +++ b/wcsmbs/Makefile
> @@ -49,6 +49,7 @@ strop-tests :=  wcscmp wcsncmp wmemcmp wcslen wcschr wcsrchr wcscpy wcsnlen \
>  tests := tst-wcstof wcsmbs-tst1 tst-wcsnlen tst-btowc tst-mbrtowc \
>  	 tst-wcrtomb tst-wcpncpy tst-mbsrtowcs tst-wchar-h tst-mbrtowc2 \
>  	 tst-c16c32-1 wcsatcliff tst-wcstol-locale tst-wcstod-nan-locale \
> +	 tst-wcstod-round \
>  	 $(addprefix test-,$(strop-tests))
>  
>  include ../Rules
> @@ -68,6 +69,8 @@ $(objpfx)tst-wcstol-locale.out: $(gen-locales)
>  $(objpfx)tst-wcstod-nan-locale.out: $(gen-locales)
>  endif
>  
> +$(objpfx)tst-wcstod-round: $(libm)
> +
>  CFLAGS-wcwidth.c = -I../wctype
>  CFLAGS-wcswidth.c = -I../wctype
>  
> diff --git a/wcsmbs/tst-wcstod-round.c b/wcsmbs/tst-wcstod-round.c
> new file mode 100644
> index 0000000..474b235
> --- /dev/null
> +++ b/wcsmbs/tst-wcstod-round.c
> @@ -0,0 +1,3 @@
> +#include <wchar.h>
> +#define TEST_WIDE
> +#include <stdlib/tst-strtod-round.c>
>
Joseph Myers Aug. 4, 2016, 4:45 p.m. UTC | #2
On Thu, 4 Aug 2016, Paul E. Murphy wrote:

> +# define STRM "%S"

I think ISO %ls should be preferred over legacy %S.
diff mbox

Patch

diff --git a/stdlib/tst-strtod-round.c b/stdlib/tst-strtod-round.c
index 509f96a..023d382 100644
--- a/stdlib/tst-strtod-round.c
+++ b/stdlib/tst-strtod-round.c
@@ -31,9 +31,26 @@ 
 
 #include "tst-strtod.h"
 
+/* Build the test for wide or normal character strings.  */
+#ifdef TEST_WIDE
+# define L_(str) L ## str
+# define FNPFX wcs
+# define CHAR wchar_t
+# define STRM "%S"
+# define snprintf swprintf
+# define strfromf128 wcsfromf128
+#else
+# define L_(str) str
+# define FNPFX str
+# define CHAR char
+# define STRM "%s"
+#endif
+
 #define _CONCAT(a, b) a ## b
 #define CONCAT(a, b) _CONCAT (a, b)
 
+#define STRTO(x) CONCAT (CONCAT (FNPFX, to), x)
+
 #if LDBL_MANT_DIG == 106 && LDBL_MAX_EXP == 1024
 /* This is a stupid hack for IBM long double.  This test ignores
    inexact values for long double due to the limitations of the
@@ -110,7 +127,7 @@ 
 	     ld106x, ld106d, ld106n, ld106z, ld106u,	\
 	     ld113x, ld113d, ld113n, ld113z, ld113u)	\
   {							\
-    s,							\
+    L_ (s),						\
     { XNTRY (fx, dx, ld64ix, ld64mx, ld106x, ld113x) },	\
     {							\
     { ENTRY (fn, dn, ld64in, ld64mn, ld106n, ld113n) },	\
@@ -131,7 +148,7 @@  struct test_results
   };
 
 struct test {
-  const char *s;
+  const CHAR *s;
   struct test_exactness exact;
   struct test_results r[4];
 };
@@ -139,19 +156,25 @@  struct test {
 /* Include the generated test data.  */
 #include "tst-strtod-round-data.h"
 
+#define STRX(x) #x
+#define STR(x) STRX (x)
+#define FNPFXS STR (FNPFX)
+
 #define GEN_ONE_TEST(FSUF, FTYPE, FTOSTR, FTOSTRM, LSUF, CSUF)	\
 {								\
-  FTYPE f = strto ## FSUF (s, NULL);				\
+  FTYPE f = STRTO (FSUF) (s, NULL);				\
   if (f != expected->FSUF					\
       || (copysign ## CSUF) (1.0 ## LSUF, f)			\
 	 != (copysign ## CSUF) (1.0 ## LSUF, expected->FSUF))	\
     {								\
-      char efstr[FSTRLENMAX];					\
-      char fstr[FSTRLENMAX];					\
-      FTOSTR (efstr, FSTRLENMAX, "%" FTOSTRM "a", expected->FSUF); \
-      FTOSTR (fstr, FSTRLENMAX, "%" FTOSTRM "a", f);		\
-      printf ("strto" #FSUF " (%s) returned %s not %s"		\
-	      " (%s)\n", s, fstr, efstr, mode_name);		\
+      CHAR efstr[FSTRLENMAX];					\
+      CHAR fstr[FSTRLENMAX];					\
+      FTOSTR (efstr, FSTRLENMAX, L_("%") L_(FTOSTRM) L_("a"),   \
+	      expected->FSUF);    				\
+      FTOSTR (fstr, FSTRLENMAX, L_("%") L_(FTOSTRM) L_("a"), f);\
+      printf (FNPFXS "to" #FSUF  " (" STRM ") returned " STRM   \
+	      " not " STRM " (%s)\n",				\
+	      s, fstr, efstr, mode_name);			\
       if (ROUNDING_TESTS (FTYPE, rnd_mode) || exact->FSUF)	\
 	result = 1;						\
       else							\
@@ -160,7 +183,7 @@  struct test {
 }
 
 static int
-test_in_one_mode (const char *s, const struct test_results *expected,
+test_in_one_mode (const CHAR *s, const struct test_results *expected,
 		  const struct test_exactness *exact, const char *mode_name,
 		  int rnd_mode)
 {
diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile
index 8b599f7..9384a10 100644
--- a/wcsmbs/Makefile
+++ b/wcsmbs/Makefile
@@ -49,6 +49,7 @@  strop-tests :=  wcscmp wcsncmp wmemcmp wcslen wcschr wcsrchr wcscpy wcsnlen \
 tests := tst-wcstof wcsmbs-tst1 tst-wcsnlen tst-btowc tst-mbrtowc \
 	 tst-wcrtomb tst-wcpncpy tst-mbsrtowcs tst-wchar-h tst-mbrtowc2 \
 	 tst-c16c32-1 wcsatcliff tst-wcstol-locale tst-wcstod-nan-locale \
+	 tst-wcstod-round \
 	 $(addprefix test-,$(strop-tests))
 
 include ../Rules
@@ -68,6 +69,8 @@  $(objpfx)tst-wcstol-locale.out: $(gen-locales)
 $(objpfx)tst-wcstod-nan-locale.out: $(gen-locales)
 endif
 
+$(objpfx)tst-wcstod-round: $(libm)
+
 CFLAGS-wcwidth.c = -I../wctype
 CFLAGS-wcswidth.c = -I../wctype
 
diff --git a/wcsmbs/tst-wcstod-round.c b/wcsmbs/tst-wcstod-round.c
new file mode 100644
index 0000000..474b235
--- /dev/null
+++ b/wcsmbs/tst-wcstod-round.c
@@ -0,0 +1,3 @@ 
+#include <wchar.h>
+#define TEST_WIDE
+#include <stdlib/tst-strtod-round.c>