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

[Paul Eggert]

On 11/19/10 04:40, Bruno Haible wrote:

> You would have to add a line
>   gl_MODULE_INDICATOR([snprintf])
> to the snprintf module, and then test
>    #if HAVE_SNPRINTF || GNULIB_SNPRINTF

Could we do that with gl_MODULE_INDICATOR([snprintf-posix]) and
the snprintf-posix module instead?

> the majority portability bugs of snprintf is also present in
> sprintf. (See the matrix at the end of m4/printf.m4.)

Yes, a lot of problems are listed there, but in practice, if printf
doesn't work on typical formats and sizes, a large amount of software
will break, and users don't expect packages to work around printf bugs
like that: they expect printf to get fixed.  The problems listed there
seem to be corner cases that won't seriously hurt the use of 'od' in
practice.

Also, I'd like ftoastr to be used with GNU Emacs; it already does something
like ftoastr, but assumes IEEE floating point, whereas ftoastr doesn't.
There, I expect that the Emacs maintainers will resist having to bring
in a lot of other stuff simply to get ftoastr to work, but that they
won't mind if ftoastr simply assumes that sprintf and strtod work (as
Emacs makes that assumption already).

After hacking around with it a bit I installed the following patch.
By default it does not use snprintf.  It assumes the
gl_MODULE_INDICATOR([snprintf-posix]) change mentioned above,
and uses snprintf only if the snprintf-posix module is also used.
This should work just fine for coreutils, even though coreutils doesn't
use the snprintf-posix module.  How does that sound?

>From dc9c838fbd1cfdd0bb0a88229ee61701c14617cd Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Fri, 19 Nov 2010 14:36:12 -0800
Subject: [PATCH] ftoastr: don't assume snprintf

* lib/ftoastr.c (snprintf) [! GNULIB_SNPRINTF_POSIX]:
Implement a subset of snprintf here, by using sprintf safely.
* modules/ftoastr (Depends-on): Remove snprintf.
---
 ChangeLog       |    7 +++++++
 lib/ftoastr.c   |   36 ++++++++++++++++++++++++++++++++++++
 modules/ftoastr |    1 -
 3 files changed, 43 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e4f534d..9dbc0c2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2010-11-19  Paul Eggert  <address@hidden>
+
+       ftoastr: don't assume snprintf
+       * lib/ftoastr.c (snprintf) [! GNULIB_SNPRINTF_POSIX]:
+       Implement a subset of snprintf here, by using sprintf safely.
+       * modules/ftoastr (Depends-on): Remove snprintf.
+
 2010-11-19  Jim Meyering  <address@hidden>
 
        test-rename.h: fix compilation failure
diff --git a/lib/ftoastr.c b/lib/ftoastr.c
index f747400..41b5aed 100644
--- a/lib/ftoastr.c
+++ b/lib/ftoastr.c
@@ -17,6 +17,14 @@
 
 /* Written by Paul Eggert.  */
 
+/* This code can misbehave on some buggy or older platforms, when
+   operating on arguments on floating types other than 'double', or
+   when given unusual combinations of options.  Gnulib's
+   snprintf-posix module works around many of these problems.
+
+   This code relies on sprintf, strtod, etc. operating accurately;
+   otherwise, the resulting strings could be inaccurate or too long.  */
+
 #include "ftoastr.h"
 
 #include "intprops.h"
@@ -56,6 +64,34 @@
 # define STRTOF strtod
 #endif
 
+/* On hosts where it's not known that snprintf works, use sprintf to
+   implement the subset needed here.  Typically BUFSIZE is big enough
+   and there's little performance hit.  */
+#if ! GNULIB_SNPRINTF_POSIX
+# undef snprintf
+# define snprintf ftoastr_snprintf
+static int
+ftoastr_snprintf (char *buf, size_t bufsize, char const *format,
+                  int width, int prec, FLOAT x)
+{
+  char width_0_buffer[LENGTH == 1 ? FLT_BUFSIZE_BOUND
+                      : LENGTH == 2 ? DBL_BUFSIZE_BOUND
+                      : LDBL_BUFSIZE_BOUND];
+  int n = width;
+  if (bufsize < sizeof width_0_buffer)
+    {
+      n = sprintf (width_0_buffer, format, 0, prec, x);
+      if (n < 0)
+        return n;
+      if (n < width)
+        n = width;
+    }
+  if (n < bufsize)
+    n = sprintf (buf, format, width, prec, x);
+  return n;
+}
+#endif
+
 int
 FTOASTR (char *buf, size_t bufsize, int flags, int width, FLOAT x)
 {
diff --git a/modules/ftoastr b/modules/ftoastr
index 862fb1a..64d0a77 100644
--- a/modules/ftoastr
+++ b/modules/ftoastr
@@ -10,7 +10,6 @@ m4/c-strtod.m4
 
 Depends-on:
 intprops
-snprintf
 
 configure.ac:
 AC_REQUIRE([gl_C99_STRTOLD])
-- 
1.7.2