[v3] Add tests for the long double version of ecvt and fcvt

Changes since v2:

  - Remove the macro L and its uses.  Let the compiler handle the
    conversion.
  - Define EXTRA_ECVT_TESTS to contain the two additional tests for
    double and DBL_MANT_DIG == 53.  Otherwise, define it to nothing.

Changes since v1:

  - Rebased on top of the new patch that adapts the test to the new
    testing framework [1] (approved but not yet applied).

[1] https://sourceware.org/ml/libc-alpha/2018-11/msg00030.html

-- 8< --
The test case misc/tst-efgcvt.c only tests the double variants of the
Old-fashioned System V number-to-string functions: ecvt, fcvt, and their
re-entrant counterparts.  With a few macros, the code can be reused for
the long double versions of these functions.  A future patch will reuse
it again for IEEE long double on powerpc64le.

Tested for powerpc and powerpc64le.

	* misc/Makefile (tests): Remove tst-efgcvt.  Add tst-dbl-efgcvt
	and tst-ldbl-efgcvt.
	* misc/tst-efgcvt.c: Renamed to misc/tst-efgcvt-template.c.
	* misc/tst-efgcvt-template.c: Renamed from misc/tst-efgcvt.c.
	(struct testcase, efcvt_func, efcvt_r_func, ecvt_tests)
	(fcvt_tests, output_error, output_r_error, do_test): Use the
	macros defined in tst-dbl-efgcvt.c and tst-ldbl-efgcvt.c to:
	select the type of floating-point variables and arguments; to
	produce extra tests for double and conversion specifiers for
	printf; and to set the names of called functions.
	* misc/tst-dbl-efgcvt.c: New file that defines the macros used
	in tst-efgcvt-template.c.
	* misc/tst-ldbl-efgcvt.c: Likewise.
---
 misc/Makefile                                |  4 +--
 misc/tst-dbl-efgcvt.c                        | 37 ++++++++++++++++++++
 misc/{tst-efgcvt.c => tst-efgcvt-template.c} | 50 +++++++++++++---------------
 misc/tst-ldbl-efgcvt.c                       | 30 +++++++++++++++++
 4 files changed, 93 insertions(+), 28 deletions(-)
 create mode 100644 misc/tst-dbl-efgcvt.c
 rename misc/{tst-efgcvt.c => tst-efgcvt-template.c} (78%)
 create mode 100644 misc/tst-ldbl-efgcvt.c

Ping.  This patch
<https://sourceware.org/ml/libc-alpha/2018-11/msg00210.html> is pending
review.

Ping^2.  This patch
<https://sourceware.org/ml/libc-alpha/2018-11/msg00210.html> is
still pending review.

* Gabriel F. T. Gomes:

> 	* misc/tst-efgcvt-template.c: Renamed from misc/tst-efgcvt.c.

I think you should add a file comment with a brief list of all the
customization macros.

Do you expect NAME to be customizable in the future?

Thanks,
Florian

On Tue, 11 Dec 2018, Florian Weimer wrote:

>* Gabriel F. T. Gomes:
>
>> 	* misc/tst-efgcvt-template.c: Renamed from misc/tst-efgcvt.c.  
>
>I think you should add a file comment with a brief list of all the
>customization macros.

Is the following text all right?

/* This template provides testing for the *cvt family of functions,
   which deal with double or long double types.  In order to use the
   template, the following macros must be defined before inclusion of
   this template:

     FLOAT: The floating-point type, i.e. double or long double.

     ECVT: Appropriate *ecvt function for FLOAT, i.e. ecvt or qecvt.
     FCVT: Likewise for *fcvt, i.e. fcvt or qfcvt.
     ECVT_R: Likewise for *ecvt_r, i.e. ecvt_r or qecvt_r.
     FCVT_R: Likewise for *fcvt_r, i.e. fcvt_r or qfcvt_r.

     NAME: Macro to stringify the name of the function, e.g. ecvt,
     possibly adding prefixes or suffixes to it, e.g. qecvt.

     PRINTF_CONVERSION: The appropriate printf conversion specifier with
     length modifier for FLOAT, i.e. "%f" or "%Lf".

     EXTRA_ECVT_TESTS: Additional tests for the ecvt or qecvt function
     that are only relevant to a particular floating-point type and
     cannot be represented generically.  */

>Do you expect NAME to be customizable in the future?

No more than what is already implemented, i.e. when FLOAT is double,
errors print the function name as ecvt or fcvt, but when FLOAT is long
double, you get qecvt, qfcvt, etc.  For the long double transition on
powerpc64le, I do *not* plan to make this test print the internal name of
the functions, e.g. __qecvtieee128.

* Gabriel F. T. Gomes:

> On Tue, 11 Dec 2018, Florian Weimer wrote:
>
>>* Gabriel F. T. Gomes:
>>
>>> 	* misc/tst-efgcvt-template.c: Renamed from misc/tst-efgcvt.c.  
>>
>>I think you should add a file comment with a brief list of all the
>>customization macros.
>
> Is the following text all right?
>
> /* This template provides testing for the *cvt family of functions,
>    which deal with double or long double types.  In order to use the
>    template, the following macros must be defined before inclusion of
>    this template:
>
>      FLOAT: The floating-point type, i.e. double or long double.
>
>      ECVT: Appropriate *ecvt function for FLOAT, i.e. ecvt or qecvt.
>      FCVT: Likewise for *fcvt, i.e. fcvt or qfcvt.
>      ECVT_R: Likewise for *ecvt_r, i.e. ecvt_r or qecvt_r.
>      FCVT_R: Likewise for *fcvt_r, i.e. fcvt_r or qfcvt_r.
>
>      NAME: Macro to stringify the name of the function, e.g. ecvt,
>      possibly adding prefixes or suffixes to it, e.g. qecvt.
>
>      PRINTF_CONVERSION: The appropriate printf conversion specifier with
>      length modifier for FLOAT, i.e. "%f" or "%Lf".
>
>      EXTRA_ECVT_TESTS: Additional tests for the ecvt or qecvt function
>      that are only relevant to a particular floating-point type and
>      cannot be represented generically.  */

Looks fine to me.

>>Do you expect NAME to be customizable in the future?
>
> No more than what is already implemented, i.e. when FLOAT is double,
> errors print the function name as ecvt or fcvt, but when FLOAT is long
> double, you get qecvt, qfcvt, etc.  For the long double transition on
> powerpc64le, I do *not* plan to make this test print the internal name of
> the functions, e.g. __qecvtieee128.

Can you move NAME into the template file, then?  Does it really differ
across test variants?

Thanks,
Florian

On Wed, 12 Dec 2018, Florian Weimer wrote:

>* Gabriel F. T. Gomes:
>
>> On Tue, 11 Dec 2018, Florian Weimer wrote:
>>  
>>>Do you expect NAME to be customizable in the future?  
>>
>> No more than what is already implemented, i.e. when FLOAT is double,
>> errors print the function name as ecvt or fcvt, but when FLOAT is long
>> double, you get qecvt, qfcvt, etc.  For the long double transition on
>> powerpc64le, I do *not* plan to make this test print the internal name of
>> the functions, e.g. __qecvtieee128.  
>
>Can you move NAME into the template file, then?

Ah, yes, I can.  Does the following patch look better?

From dd8545e8b3a9da1977777591ec356fca52bebe99 Mon Sep 17 00:00:00 2001
From: "Gabriel F. T. Gomes" <gabriel@inconstante.eti.br>
Date: Tue, 30 Oct 2018 15:04:18 -0300
Subject: [PATCH v4] Add tests for the long double version of ecvt and fcvt

Changes since v3:

  - Added description for each of the macros used for templatization.
  - Moved NAME into the template file.

Changes since v2:

  - Remove the macro L and its uses.  Let the compiler handle the
    conversion.
  - Define EXTRA_ECVT_TESTS to contain the two additional tests for
    double and DBL_MANT_DIG == 53.  Otherwise, define it to nothing.

Changes since v1:

  - Rebased on top of the new patch that adapts the test to the new
    testing framework.

-- 8< --
The test case misc/tst-efgcvt.c only tests the double variants of the
Old-fashioned System V number-to-string functions: ecvt, fcvt, and their
re-entrant counterparts.  With a few macros, the code can be reused for
the long double versions of these functions.  A future patch will reuse
it again for IEEE long double on powerpc64le.

Tested for powerpc and powerpc64le.

	* misc/Makefile (tests): Remove tst-efgcvt.  Add tst-dbl-efgcvt
	and tst-ldbl-efgcvt.
	* misc/tst-efgcvt.c: Renamed to misc/tst-efgcvt-template.c.
	* misc/tst-efgcvt-template.c: Renamed from misc/tst-efgcvt.c.
	(struct testcase, efcvt_func, efcvt_r_func, ecvt_tests)
	(fcvt_tests, output_error, output_r_error, do_test): Use the
	macros defined in tst-dbl-efgcvt.c and tst-ldbl-efgcvt.c to:
	select the type of floating-point variables and arguments; to
	produce extra tests for double and conversion specifiers for
	printf; and to set the names of called functions.
	* misc/tst-dbl-efgcvt.c: New file that defines the macros used
	in tst-efgcvt-template.c.
	* misc/tst-ldbl-efgcvt.c: Likewise.
---
 misc/Makefile                                |  4 +-
 misc/tst-dbl-efgcvt.c                        | 36 ++++++++++++++
 misc/{tst-efgcvt.c => tst-efgcvt-template.c} | 72 ++++++++++++++++++----------
 misc/tst-ldbl-efgcvt.c                       | 28 +++++++++++
 4 files changed, 112 insertions(+), 28 deletions(-)
 create mode 100644 misc/tst-dbl-efgcvt.c
 rename misc/{tst-efgcvt.c => tst-efgcvt-template.c} (68%)
 create mode 100644 misc/tst-ldbl-efgcvt.c

diff --git a/misc/Makefile b/misc/Makefile
index c2c9994d17..cf3726b637 100644
--- a/misc/Makefile
+++ b/misc/Makefile
@@ -80,12 +80,12 @@ aux := init-misc
 install-lib := libg.a
 gpl2lgpl := error.c error.h
 
-tests := tst-dirname tst-tsearch tst-fdset tst-efgcvt tst-mntent tst-hsearch \
+tests := tst-dirname tst-tsearch tst-fdset tst-mntent tst-hsearch \
 	 tst-error1 tst-pselect tst-insremque tst-mntent2 bug-hsearch1 \
 	 tst-mntent-blank-corrupt tst-mntent-blank-passno bug18240 \
 	 tst-preadvwritev tst-preadvwritev64 tst-makedev tst-empty \
 	 tst-preadvwritev2 tst-preadvwritev64v2 tst-warn-wide \
-	 tst-ldbl-warn tst-ldbl-error
+	 tst-ldbl-warn tst-ldbl-error tst-dbl-efgcvt tst-ldbl-efgcvt
 
 # Tests which need libdl.
 ifeq (yes,$(build-shared))
diff --git a/misc/tst-dbl-efgcvt.c b/misc/tst-dbl-efgcvt.c
new file mode 100644
index 0000000000..78accaddc7
--- /dev/null
+++ b/misc/tst-dbl-efgcvt.c
@@ -0,0 +1,36 @@
+/* Tests for *cvt function, double version.
+   Copyright (C) 1998-2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <float.h>
+
+#define ECVT ecvt
+#define FCVT fcvt
+#define ECVT_R ecvt_r
+#define FCVT_R fcvt_r
+#define FLOAT double
+#define PRINTF_CONVERSION "%f"
+
+#if DBL_MANT_DIG == 53
+# define EXTRA_ECVT_TESTS \
+  { 0x1p-1074, 3, -323, "494" }, \
+  { -0x1p-1074, 3, -323, "494" },
+#else
+# define EXTRA_ECVT_TESTS
+#endif
+
+#include <tst-efgcvt-template.c>
diff --git a/misc/tst-efgcvt.c b/misc/tst-efgcvt-template.c
similarity index 68%
rename from misc/tst-efgcvt.c
rename to misc/tst-efgcvt-template.c
index 5a598f49e3..cba964a842 100644
--- a/misc/tst-efgcvt.c
+++ b/misc/tst-efgcvt-template.c
@@ -15,11 +15,29 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+/* This template provides testing for the *cvt family of functions,
+   which deal with double or long double types.  In order to use the
+   template, the following macros must be defined before inclusion of
+   this template:
+
+     FLOAT: The floating-point type, i.e. double or long double.
+
+     ECVT: Appropriate *ecvt function for FLOAT, i.e. ecvt or qecvt.
+     FCVT: Likewise for *fcvt, i.e. fcvt or qfcvt.
+     ECVT_R: Likewise for *ecvt_r, i.e. ecvt_r or qecvt_r.
+     FCVT_R: Likewise for *fcvt_r, i.e. fcvt_r or qfcvt_r.
+
+     PRINTF_CONVERSION: The appropriate printf conversion specifier with
+     length modifier for FLOAT, i.e. "%f" or "%Lf".
+
+     EXTRA_ECVT_TESTS: Additional tests for the ecvt or qecvt function
+     that are only relevant to a particular floating-point type and
+     cannot be represented generically.  */
+
 #ifndef _GNU_SOURCE
 # define _GNU_SOURCE	1
 #endif
 
-#include <float.h>
 #include <math.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -27,17 +45,20 @@
 
 #include <support/check.h>
 
+#define NAME(x) NAMEX(x)
+#define NAMEX(x) #x
+
 typedef struct
 {
-  double value;
+  FLOAT value;
   int ndigit;
   int decpt;
   char result[30];
 } testcase;
 
-typedef char * ((*efcvt_func) (double, int, int *, int *));
+typedef char * ((*efcvt_func) (FLOAT, int, int *, int *));
 
-typedef int ((*efcvt_r_func) (double, int, int *, int *, char *, size_t));
+typedef int ((*efcvt_r_func) (FLOAT, int, int *, int *, char *, size_t));
 
 
 static testcase ecvt_tests[] =
@@ -59,10 +80,7 @@ static testcase ecvt_tests[] =
   { 123.01, -4, 3, "" },
   { 126.71, -4, 3, "" },
   { 0.0, 4, 1, "0000" },
-#if DBL_MANT_DIG == 53
-  { 0x1p-1074, 3, -323, "494" },
-  { -0x1p-1074, 3, -323, "494" },
-#endif
+  EXTRA_ECVT_TESTS
   /* -1.0 is end marker.  */
   { -1.0, 0, 0, "" }
 };
@@ -91,11 +109,12 @@ static testcase fcvt_tests[] =
 };
 
 static void
-output_error (const char *name, double value, int ndigit,
+output_error (const char *name, FLOAT value, int ndigit,
 	      const char *exp_p, int exp_decpt, int exp_sign,
 	      char *res_p, int res_decpt, int res_sign)
 {
-  printf ("%s returned wrong result for value: %f, ndigits: %d\n",
+  printf ("%s returned wrong result for value: " PRINTF_CONVERSION
+	  ", ndigits: %d\n",
 	  name, value, ndigit);
   printf ("Result was p: \"%s\", decpt: %d, sign: %d\n",
 	  res_p, res_decpt, res_sign);
@@ -106,11 +125,12 @@ output_error (const char *name, double value, int ndigit,
 
 
 static void
-output_r_error (const char *name, double value, int ndigit,
+output_r_error (const char *name, FLOAT value, int ndigit,
 		const char *exp_p, int exp_decpt, int exp_sign, int exp_return,
 		char *res_p, int res_decpt, int res_sign, int res_return)
 {
-  printf ("%s returned wrong result for value: %f, ndigits: %d\n",
+  printf ("%s returned wrong result for value: " PRINTF_CONVERSION
+	  ", ndigits: %d\n",
 	  name, value, ndigit);
   printf ("Result was buf: \"%s\", decpt: %d, sign: %d return value: %d\n",
 	  res_p, res_decpt, res_sign, res_return);
@@ -171,30 +191,30 @@ special (void)
   char *p;
   char buf [1024];
 
-  p = ecvt (NAN, 10, &decpt, &sign);
+  p = ECVT (NAN, 10, &decpt, &sign);
   if (sign != 0 || strcmp (p, "nan") != 0)
-    output_error ("ecvt", NAN, 10, "nan", 0, 0, p, decpt, sign);
+    output_error (NAME (ECVT), NAN, 10, "nan", 0, 0, p, decpt, sign);
 
-  p = ecvt (INFINITY, 10, &decpt, &sign);
+  p = ECVT (INFINITY, 10, &decpt, &sign);
   if (sign != 0 || strcmp (p, "inf") != 0)
-    output_error ("ecvt", INFINITY, 10, "inf", 0, 0, p, decpt, sign);
+    output_error (NAME (ECVT), INFINITY, 10, "inf", 0, 0, p, decpt, sign);
 
   /* Simply make sure these calls with large NDIGITs don't crash.  */
-  (void) ecvt (123.456, 10000, &decpt, &sign);
-  (void) fcvt (123.456, 10000, &decpt, &sign);
+  (void) ECVT (123.456, 10000, &decpt, &sign);
+  (void) FCVT (123.456, 10000, &decpt, &sign);
 
   /* Some tests for the reentrant functions.  */
   /* Use a too small buffer.  */
-  res = ecvt_r (123.456, 10, &decpt, &sign, buf, 1);
+  res = ECVT_R (123.456, 10, &decpt, &sign, buf, 1);
   if (res == 0)
     {
-      printf ("ecvt_r with a too small buffer was succesful.\n");
+      printf (NAME (ECVT_R) " with a too small buffer was succesful.\n");
       support_record_failure ();
     }
-  res = fcvt_r (123.456, 10, &decpt, &sign, buf, 1);
+  res = FCVT_R (123.456, 10, &decpt, &sign, buf, 1);
   if (res == 0)
     {
-      printf ("fcvt_r with a too small buffer was succesful.\n");
+      printf (NAME (FCVT_R) " with a too small buffer was succesful.\n");
       support_record_failure ();
     }
 }
@@ -203,10 +223,10 @@ special (void)
 static int
 do_test (void)
 {
-  test (ecvt_tests, ecvt, "ecvt");
-  test (fcvt_tests, fcvt, "fcvt");
-  test_r (ecvt_tests, ecvt_r, "ecvt_r");
-  test_r (fcvt_tests, fcvt_r, "fcvt_r");
+  test (ecvt_tests, ECVT, NAME (ECVT));
+  test (fcvt_tests, FCVT, NAME (FCVT));
+  test_r (ecvt_tests, ECVT_R, NAME (ECVT_R));
+  test_r (fcvt_tests, FCVT_R, NAME (FCVT_R));
   special ();
 
   return 0;
diff --git a/misc/tst-ldbl-efgcvt.c b/misc/tst-ldbl-efgcvt.c
new file mode 100644
index 0000000000..35d24ae3ad
--- /dev/null
+++ b/misc/tst-ldbl-efgcvt.c
@@ -0,0 +1,28 @@
+/* Tests for *cvt function, long double version.
+   Copyright (C) 1998-2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define ECVT qecvt
+#define FCVT qfcvt
+#define ECVT_R qecvt_r
+#define FCVT_R qfcvt_r
+#define FLOAT long double
+#define PRINTF_CONVERSION "%Lf"
+
+#define EXTRA_ECVT_TESTS
+
+#include <tst-efgcvt-template.c>

* Gabriel F. T. Gomes:

> 	* misc/Makefile (tests): Remove tst-efgcvt.  Add tst-dbl-efgcvt
> 	and tst-ldbl-efgcvt.
> 	* misc/tst-efgcvt.c: Renamed to misc/tst-efgcvt-template.c.
> 	* misc/tst-efgcvt-template.c: Renamed from misc/tst-efgcvt.c.
> 	(struct testcase, efcvt_func, efcvt_r_func, ecvt_tests)
> 	(fcvt_tests, output_error, output_r_error, do_test): Use the
> 	macros defined in tst-dbl-efgcvt.c and tst-ldbl-efgcvt.c to:
> 	select the type of floating-point variables and arguments; to
> 	produce extra tests for double and conversion specifiers for
> 	printf; and to set the names of called functions.
> 	* misc/tst-dbl-efgcvt.c: New file that defines the macros used
> 	in tst-efgcvt-template.c.
> 	* misc/tst-ldbl-efgcvt.c: Likewise.

This version looks okay to me.  I'm not a math reviewer, but please feel
free to commit this.

Thanks,
Florian

On Thu, 13 Dec 2018, Florian Weimer wrote:

>This version looks okay to me.  I'm not a math reviewer, but please feel
>free to commit this.

Committed.  Thanks.