[PATCH] Fix ecvt to pass tests

[PATCH] Fix ecvt to pass tests

[Keith Packard]

Elide decimal point when no digits are right of that. Fix computation
of trailing zero length.

Signed-off-by: Keith Packard <keithp@keithp.com>
---
 newlib/libc/stdlib/ecvtbuf.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/newlib/libc/stdlib/ecvtbuf.c b/newlib/libc/stdlib/ecvtbuf.c
index e3d7b55d8..d2ba6359d 100644
--- a/newlib/libc/stdlib/ecvtbuf.c
+++ b/newlib/libc/stdlib/ecvtbuf.c
@@ -93,7 +93,8 @@ print_f (struct _reent *ptr,
     {
       if (p == start)
 	*buf++ = '0';
-      *buf++ = '.';
+      if (decpt < 0 && ndigit > 0)
+	*buf++ = '.';
       while (decpt < 0 && ndigit > 0)
 	{
 	  *buf++ = '0';
@@ -148,11 +149,15 @@ print_e (struct _reent *ptr,
     }
 
   *buf++ = *p++;
-  if (dot || ndigit != 0)
-    *buf++ = '.';
+  if (ndigit > 0)
+    dot = 1;
 
   while (*p && ndigit > 0)
     {
+      if (dot) {
+	*buf++ = '.';
+	dot = 0;
+      }
       *buf++ = *p++;
       ndigit--;
     }
@@ -168,6 +173,10 @@ print_e (struct _reent *ptr,
     {
       while (ndigit > 0)
 	{
+	  if  (dot) {
+	    *buf++ = '.';
+	    dot = 0;
+	  }
 	  *buf++ = '0';
 	  ndigit--;
 	}
@@ -246,7 +255,7 @@ fcvtbuf (double invalue,
 
   /* Now copy */
 
-  done = -*decpt;
+  done = 0;
   while (p < end)
     {
       *fcvt_buf++ = *p++;
-- 
2.24.0

Re: [PATCH] Fix ecvt to pass tests

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]

Hi Keith,

On Dec 16 13:54, Keith Packard wrote:
> Elide decimal point when no digits are right of that. Fix computation
> of trailing zero length.
> 
> Signed-off-by: Keith Packard <keithp@keithp.com>
> ---
>  newlib/libc/stdlib/ecvtbuf.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/newlib/libc/stdlib/ecvtbuf.c b/newlib/libc/stdlib/ecvtbuf.c
> index e3d7b55d8..d2ba6359d 100644
> --- a/newlib/libc/stdlib/ecvtbuf.c
> +++ b/newlib/libc/stdlib/ecvtbuf.c
> @@ -93,7 +93,8 @@ print_f (struct _reent *ptr,
>      {
>        if (p == start)
>  	*buf++ = '0';
> -      *buf++ = '.';
> +      if (decpt < 0 && ndigit > 0)
> +	*buf++ = '.';
>        while (decpt < 0 && ndigit > 0)
>  	{
>  	  *buf++ = '0';
> @@ -148,11 +149,15 @@ print_e (struct _reent *ptr,
>      }
>  
>    *buf++ = *p++;
> -  if (dot || ndigit != 0)
> -    *buf++ = '.';
> +  if (ndigit > 0)
> +    dot = 1;
>  
>    while (*p && ndigit > 0)
>      {
> +      if (dot) {
> +	*buf++ = '.';
> +	dot = 0;
> +      }
>        *buf++ = *p++;
>        ndigit--;
>      }
> @@ -168,6 +173,10 @@ print_e (struct _reent *ptr,
>      {
>        while (ndigit > 0)
>  	{
> +	  if  (dot) {
> +	    *buf++ = '.';
> +	    dot = 0;
> +	  }
>  	  *buf++ = '0';
>  	  ndigit--;
>  	}
> @@ -246,7 +255,7 @@ fcvtbuf (double invalue,
>  
>    /* Now copy */
>  
> -  done = -*decpt;
> +  done = 0;
>    while (p < end)
>      {
>        *fcvt_buf++ = *p++;

That last hunk is not immediately clear to me.  Can you explain this a
bit or even add some more text to the commit message?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Fix ecvt to pass tests

[Keith Packard]

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

Corinna Vinschen <vinschen@redhat.com> writes:

> That last hunk is not immediately clear to me.  Can you explain this a
> bit or even add some more text to the commit message?

Good catch. Not only was it 'unclear', it was wrong.

I've re-generated all of the test vectors for this code using glibc as a
model, then fixed the code to pass those tests *and* match my reading of
the POSIX manual for fcvt, ecvt and gcvt.

I then split the fixes into three patches:

 1) Fix fcvt. Fcvt is defined to only show a limited number of digits
    past the radix marker/decimal point. The unfixed code had a special
    case for numbers < 1.0 so that it would display the specified
    number of digits, even if there would need to be a number of leading
    zeros before those. The fixed code will limit itself to the
    specified number of digits past the decimal point, even if that
    means returning the empty string.

 2) Fix gcvt. Gcvt is always supposed to return the specified number of
    digits of precision. For numbers < 1.0, gcvt may insert leading
    zeros which are supposed to be part of this count.

It's interesting to note that both of these cases actually removed
conditionals around the calls to _dtoa_r as that function already did
exactly what was needed.

 3) Make sure _dcvt doesn't display a trailing decimal point

I'll send these three patches to the list shortly. Thanks much for your
review, and for asking a really good question. I got to spend quite a
few hours sorting this out.

-- 
-keith

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]