Re: strtold does not set errno when it should

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

Re: strtold does not set errno when it should

Corinna Vinschen-2
Hi Bruno,

[moving this discussion to the newlib list since that's newlib code]

On Dec 12 07:38, Bruno Haible wrote:

> POSIX [1] makes it clear that when the value to be returned would cause
> underflow, it should set errno to ERANGE.
>
> [1] <https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html>
>
> This test case fails with error code 4 on Cygwin 2.9.
>
> ========================== foo.c ==========================
> #include <stdlib.h>
> #include <errno.h>
> #include <float.h>
> #include <math.h>
>
> int main ()
> {
>   const char input[] = "1E-100000";
>   char *ptr;
>   long double result;
>   errno = 0;
>   result = strtold (input, &ptr);
>   if (!(ptr == input + 9))
>     return 1;
>   if (!(0.0L <= result && result <= LDBL_MIN))
>     return 2;
>   if (signbit (result))
>     return 3;
>   if (result == 0.0L && errno != ERANGE)
>     return 4;
>   return 0;
> }
> ============================================================
> $ gcc -Wall foo.c
> $ ./a.exe
> $ echo $?
> 4
>
> The corresponding test case for strtod() / 'double' succeeds.
The code for strtold is almost verbatim taken from

  https://github.com/jwiegley/gdtoa.git

There haven't been any patches upstream.  I, for one, am not overly
fluent with FP math, so I'm glad for any help.

Looking into _strtodg_l (newlib/libc/stdlib/strtodg.c) I tried this
patch:

diff --git a/newlib/libc/stdlib/strtodg.c b/newlib/libc/stdlib/strtodg.c
index 013315946c1b..d6fb26ad3b45 100644
--- a/newlib/libc/stdlib/strtodg.c
+++ b/newlib/libc/stdlib/strtodg.c
@@ -1091,6 +1091,10 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
  irv |= STRTOG_Underflow;
  }
  }
+#ifndef NO_ERRNO
+ if (irv & STRTOG_Underflow)
+ errno = ERANGE;
+#endif
  if (se)
  *se = (char *)s;
  if (sign)

which seems to do the trick.  But, does it make sense?  If not, I'd
really appreciate a patch.


Thanks,
Corinna

--
Corinna Vinschen
Cygwin Maintainer

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

Re: strtold does not set errno when it should

Bruno Haible
Hi Corinna,

> The code for strtold is almost verbatim taken from
>
>   https://github.com/jwiegley/gdtoa.git
>
> There haven't been any patches upstream.

Probably that's because the code is good enough for ISO C compliance;
however, POSIX [1] adds the "ERANGE upon underflow" requirement,
whereas ISO C only has an "ERANGE upon overflow" requirement.

[1] <https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html>

> diff --git a/newlib/libc/stdlib/strtodg.c b/newlib/libc/stdlib/strtodg.c
> index 013315946c1b..d6fb26ad3b45 100644
> --- a/newlib/libc/stdlib/strtodg.c
> +++ b/newlib/libc/stdlib/strtodg.c
> @@ -1091,6 +1091,10 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
>   irv |= STRTOG_Underflow;
>   }
>   }
> +#ifndef NO_ERRNO
> + if (irv & STRTOG_Underflow)
> + errno = ERANGE;
> +#endif
>   if (se)
>   *se = (char *)s;
>   if (sign)
>
> which seems to do the trick.  But, does it make sense?  If not, I'd
> really appreciate a patch.
I think it goes in the right direction.

It can be improved a bit, though: The existing code for overflow is careful
to do the errno stuff only when STRTOG_Overflow is being set/added. I.e.
keep this overflow/underflow handling out of the main path, in order not
to slow down the 99.99% of the cases that don't need it.

My attempt to do this would look like the attached patch. Untested.

Bruno

strtodg.c.diff (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: strtold does not set errno when it should

Corinna Vinschen
Hi Bruno,

On Dec 16 11:05, Bruno Haible wrote:

> Hi Corinna,
>
> > The code for strtold is almost verbatim taken from
> >
> >   https://github.com/jwiegley/gdtoa.git
> >
> > There haven't been any patches upstream.
>
> Probably that's because the code is good enough for ISO C compliance;
> however, POSIX [1] adds the "ERANGE upon underflow" requirement,
> whereas ISO C only has an "ERANGE upon overflow" requirement.
>
> [1] <https://pubs.opengroup.org/onlinepubs/9699919799/functions/strtod.html>
>
> > diff --git a/newlib/libc/stdlib/strtodg.c b/newlib/libc/stdlib/strtodg.c
> > index 013315946c1b..d6fb26ad3b45 100644
> > --- a/newlib/libc/stdlib/strtodg.c
> > +++ b/newlib/libc/stdlib/strtodg.c
> > @@ -1091,6 +1091,10 @@ _strtodg_l (struct _reent *p, const char *s00, char **se, FPI *fpi, Long *exp,
> >   irv |= STRTOG_Underflow;
> >   }
> >   }
> > +#ifndef NO_ERRNO
> > + if (irv & STRTOG_Underflow)
> > + errno = ERANGE;
> > +#endif
> >   if (se)
> >   *se = (char *)s;
> >   if (sign)
> >
> > which seems to do the trick.  But, does it make sense?  If not, I'd
> > really appreciate a patch.
>
> I think it goes in the right direction.
>
> It can be improved a bit, though: The existing code for overflow is careful
> to do the errno stuff only when STRTOG_Overflow is being set/added. I.e.
> keep this overflow/underflow handling out of the main path, in order not
> to slow down the 99.99% of the cases that don't need it.
>
> My attempt to do this would look like the attached patch. Untested.
That looks good to me and the result is the expected one.

If nobody complains in the next few hours, I'll check this in.
I *think* as of yet Cygwin is the only user of this code anyway.

Thanks!


Corinna

--
Corinna Vinschen
Cygwin Maintainer
Red Hat

signature.asc (849 bytes) Download Attachment