bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] ftoastr: new module, for lossless conversion of floats to sh


From: Paul Eggert
Subject: Re: [PATCH] ftoastr: new module, for lossless conversion of floats to short strings
Date: Thu, 18 Nov 2010 13:59:29 -0800
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.15) Gecko/20101027 Thunderbird/3.0.10

On 11/18/10 12:46, Bruno Haible wrote:
> 
> * The code uses the snprintf() function but the module does not depend on the
>   'snprintf' or 'snprintf-posix' module. Do you intend to ignore these
>   portability problems?

Thanks for your thoughtful review.  This is a tricky business.  On the
one hand, I want ftoastr to work portably.  On the other, I also want
it to work for packages that want to assume a working snprintf and
don't want to drag in that other stuff.

It'd be nice for ftoastr to use snprintf if it works, and use sprintf
otherwise.  For this particular case this can be done safely, without
fear of buffer overrun, although it'll be a bit slower when it uses
sprintf, since it'll have to run sprintf one more time in some cases.

Is there an easy way for code to ask whether snprintf is available in
the presence of gnulib modules, regardless of whether it's builtin or
supplied by gnulib?  I'd like something like this

  #if HAVE_SNPRINTF
    (use snprintf)
  #else
    (use sprintf)
  #fi

except it's not obvious from stdio.in.h what #if test to use here.

> * The URL to the paper that you gave is a closed-content one: it requires
>   payment. Fortunately, the author of the paper makes it also available 
> online:
>   <http://florian.loitsch.com/tmp/article.pdf>

Yes, I considered putting that URL in the comment, but its "/tmp/"
scared me away.  I see no real problem in adding it, if you prefer.

>   His algorithm depends on the existence of a precomputed table of powers of 
> 10
>   whose size depends on the exponent range.

Yes, and for that reason, I don't think it's practical for long
double, as you mentioned.  The idea would be to use his algorithm when
it's a win, and to fall back on the current code, or on the older
algorithms that he mentions, for cases like 'long double' when it's
not a win.  I expect that for 'double' and 'float' it will be a win on
the vast majority of gnulib targets these days.

> * This module uses the function strtof(), strtod(), strtold(), which also have
>   known portability problems....
>   In particular, when the gnulib strtod() function is in use, which has known
>   rounding errors, this will lead to wrong results of the function dtoastr(),
>   no?

Yes, if the gnulib function is buggy, dtoastr will also be buggy.
The best way to fix that, presumably, is to fix the gnulib function.

Of the portability errors mentioned, I think the only ones that will
break ftoastr etc. (in the sense that they'll return worse answers
than what you'd get with printf with a precision of DBL_DIG + 2 etc.),
are the platforms that lack strtof and strtold entirely.  Something
like the following patch should address that issue, I expect.  I'll
try it out.

diff --git a/lib/ftoastr.c b/lib/ftoastr.c
index 25e0705..959070d 100644
--- a/lib/ftoastr.c
+++ b/lib/ftoastr.c
@@ -30,14 +30,15 @@
 # define FLOAT_MIN LDBL_MIN
 # define FLOAT_PREC_BOUND _GL_LDBL_PREC_BOUND
 # define FTOASTR ldtoastr
-# define STRTOF strtold
+# if HAVE_C99_STRTOLD
+#  define STRTOF strtold
+# endif
 #elif LENGTH == 2
 # define FLOAT double
 # define FLOAT_DIG DBL_DIG
 # define FLOAT_MIN DBL_MIN
 # define FLOAT_PREC_BOUND _GL_DBL_PREC_BOUND
 # define FTOASTR dtoastr
-# define STRTOF strtod
 #else
 # define LENGTH 1
 # define FLOAT float
@@ -45,7 +46,13 @@
 # define FLOAT_MIN FLT_MIN
 # define FLOAT_PREC_BOUND _GL_FLT_PREC_BOUND
 # define FTOASTR ftoastr
-# define STRTOF strtof
+# if HAVE_C99_STRTOLD
+#  define STRTOF strtof
+# endif
+#endif
+
+#ifndef STRTOF
+# define STRTOF strtod
 #endif
 
 int
diff --git a/modules/ftoastr b/modules/ftoastr
index fbc0cba..64d0a77 100644
--- a/modules/ftoastr
+++ b/modules/ftoastr
@@ -6,11 +6,13 @@ lib/ftoastr.h
 lib/ftoastr.c
 lib/dtoastr.c
 lib/ldtoastr.c
+m4/c-strtod.m4
 
 Depends-on:
 intprops
 
 configure.ac:
+AC_REQUIRE([gl_C99_STRTOLD])
 
 Makefile.am:
 lib_SOURCES += ftoastr.h ftoastr.c dtoastr.c ldtoastr.c




reply via email to

[Prev in Thread] Current Thread [Next in Thread]