[PATCH] Fix ecvt to pass tests

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[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 <[hidden email]>
---
 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

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix ecvt to pass tests

Corinna Vinschen
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 <[hidden email]>
> ---
>  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

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix ecvt to pass tests

Keith Packard
Corinna Vinschen <[hidden email]> 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

signature.asc (847 bytes) Download Attachment