Subject: [PATCH] od: fix bugs in displaying floating-point values

From: Paul Eggert <[email-masked]>
Date: Thu, 18 Nov 2010 09:53:05 -0800

* NEWS: Describe patch.
* bootstrap.conf (gnulib_modules): Add ftoastr.
* src/od.c: Include ftoastr.h, not float.h.
(FLT_DIG, DBL_DIG): Remove.  No need to verify LDBL_DIG.
(FMT_BYTES_ALLOCATED): No need to worry about floating point now,
since this format is no longer used for floating point.
(PRINT_FIELDS): New macro, with most of the guts of the old PRINT_TYPE.
(PRINT_TYPE): Rewrite to use PRINT_FIELDS.
(PRINT_FLOATTYPE): New macro.  This uses the new functions from
ftoastr.
(print_float, print_double, print_long_double): Reimplement
using PRINT_FLOATTYPE.
(decode_one_format): Calculate field widths based on ftoastr-supplied
macros.
* tests/Makefile.am (TESTS): Add misc/od-float.
* tests/misc/od-float: New file.
---
 NEWS                |    4 +++
 bootstrap.conf      |    1 +
 src/od.c            |   63 +++++++++++++++++++++++---------------------------
 tests/Makefile.am   |    1 +
 tests/misc/od-float |   58 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 93 insertions(+), 34 deletions(-)
 create mode 100755 tests/misc/od-float

diff --git a/NEWS b/NEWS
index 1da7db2..ca6facb 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ GNU coreutils NEWS                                    -*- outline -*-
 
 * Noteworthy changes in release ?.? (????-??-??) [?]
 
+** Bug fixes
+
+  od now prints floating-point numbers without losing information, and
+  it no longer omits spaces between floating-point columns in some cases.
 
 * Noteworthy changes in release 8.7 (2010-11-13) [stable]
 
diff --git a/bootstrap.conf b/bootstrap.conf
index 2da0883..dc4b5b2 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -88,6 +88,7 @@ gnulib_modules="
   fsusage
   fsync
   ftello
+  ftoastr
   fts
   getgroups
   gethrxtime
diff --git a/src/od.c b/src/od.c
index 1d106b1..2f7ef00 100644
--- a/src/od.c
+++ b/src/od.c
@@ -24,6 +24,7 @@
 #include <sys/types.h>
 #include "system.h"
 #include "error.h"
+#include "ftoastr.h"
 #include "quote.h"
 #include "xfreopen.h"
 #include "xprintf.h"
@@ -34,21 +35,9 @@
 
 #define AUTHORS proper_name ("Jim Meyering")
 
-#include <float.h>
-
 /* The default number of input bytes per output line.  */
 #define DEFAULT_BYTES_PER_BLOCK 16
 
-/* The number of decimal digits of precision in a float.  */
-#ifndef FLT_DIG
-# define FLT_DIG 7
-#endif
-
-/* The number of decimal digits of precision in a double.  */
-#ifndef DBL_DIG
-# define DBL_DIG 15
-#endif
-
 #if HAVE_UNSIGNED_LONG_LONG_INT
 typedef unsigned long long int unsigned_long_long_int;
 #else
@@ -92,17 +81,15 @@ enum output_format
 enum
   {
     FMT_BYTES_ALLOCATED =
-      MAX ((sizeof "%*.99" - 1
+           (sizeof "%*.99" - 1
             + MAX (sizeof "ld",
                    MAX (sizeof PRIdMAX,
                         MAX (sizeof PRIoMAX,
                              MAX (sizeof PRIuMAX,
-                                  sizeof PRIxMAX))))),
-           sizeof "%*.99Le")
+                                  sizeof PRIxMAX)))))
   };
 
 /* Ensure that our choice for FMT_BYTES_ALLOCATED is reasonable.  */
-verify (LDBL_DIG <= 99);
 verify (MAX_INTEGRAL_TYPE_SIZE * CHAR_BIT / 3 <= 99);
 
 /* Each output format specification (from `-t spec' or from
@@ -401,10 +388,10 @@ implies 32.  By default, od uses -A o -t oS -w16.\n\
 
 /* Define the print functions.  */
 
-#define PRINT_TYPE(N, T)                                                \
+#define PRINT_FIELDS(N, T, FMT_STRING, ACTION)                          \
 static void                                                             \
 N (size_t fields, size_t blank, void const *block,                      \
-   char const *fmt_string, int width, int pad)                          \
+   char const *FMT_STRING, int width, int pad)                          \
 {                                                                       \
   T const *p = block;                                                   \
   size_t i;                                                             \
@@ -412,11 +399,22 @@ N (size_t fields, size_t blank, void const *block,                      \
   for (i = fields; blank < i; i--)                                      \
     {                                                                   \
       int next_pad = pad * (i - 1) / fields;                            \
-      xprintf (fmt_string, pad_remaining - next_pad + width, *p++);     \
+      int adjusted_width = pad_remaining - next_pad + width;            \
+      T x = *p++;                                                       \
+      ACTION;                                                           \
       pad_remaining = next_pad;                                         \
     }                                                                   \
 }
 
+#define PRINT_TYPE(N, T)                                                \
+  PRINT_FIELDS (N, T, fmt_string, xprintf (fmt_string, adjusted_width, x))
+
+#define PRINT_FLOATTYPE(N, T, FTOASTR, BUFSIZE)                         \
+  PRINT_FIELDS (N, T, fmt_string ATTRIBUTE_UNUSED,                      \
+                char buf[BUFSIZE];                                      \
+                FTOASTR (buf, sizeof buf, 0, 0, x);                     \
+                xprintf ("%*s", adjusted_width, buf))
+
 PRINT_TYPE (print_s_char, signed char)
 PRINT_TYPE (print_char, unsigned char)
 PRINT_TYPE (print_s_short, short int)
@@ -424,11 +422,13 @@ PRINT_TYPE (print_short, unsigned short int)
 PRINT_TYPE (print_int, unsigned int)
 PRINT_TYPE (print_long, unsigned long int)
 PRINT_TYPE (print_long_long, unsigned_long_long_int)
-PRINT_TYPE (print_float, float)
-PRINT_TYPE (print_double, double)
-PRINT_TYPE (print_long_double, long double)
+
+PRINT_FLOATTYPE (print_float, float, ftoastr, FLT_BUFSIZE_BOUND)
+PRINT_FLOATTYPE (print_double, double, dtoastr, DBL_BUFSIZE_BOUND)
+PRINT_FLOATTYPE (print_long_double, long double, ldtoastr, LDBL_BUFSIZE_BOUND)
 
 #undef PRINT_TYPE
+#undef PRINT_FLOATTYPE
 
 static void
 dump_hexl_mode_trailer (size_t n_bytes, const char *block)
@@ -586,13 +586,11 @@ decode_one_format (const char *s_orig, const char *s, const char **next,
   enum size_spec size_spec;
   unsigned long int size;
   enum output_format fmt;
-  const char *pre_fmt_string;
   void (*print_function) (size_t, size_t, void const *, char const *,
                           int, int);
   const char *p;
   char c;
   int field_width;
-  int precision;
 
   assert (tspec != NULL);
 
@@ -772,34 +770,31 @@ this system doesn't provide a %lu-byte floating point type"),
         }
       size_spec = fp_type_size[size];
 
+      struct lconv const *locale = localeconv ();
+      size_t decimal_point_len =
+        (locale->decimal_point[0] ? strlen (locale->decimal_point) : 1);
+
       switch (size_spec)
         {
         case FLOAT_SINGLE:
           print_function = print_float;
-          /* FIXME - should we use %g instead of %e?  */
-          pre_fmt_string = "%%*.%de";
-          precision = FLT_DIG;
+          field_width = FLT_STRLEN_BOUND_L (decimal_point_len);
           break;
 
         case FLOAT_DOUBLE:
           print_function = print_double;
-          pre_fmt_string = "%%*.%de";
-          precision = DBL_DIG;
+          field_width = DBL_STRLEN_BOUND_L (decimal_point_len);
           break;
 
         case FLOAT_LONG_DOUBLE:
           print_function = print_long_double;
-          pre_fmt_string = "%%*.%dLe";
-          precision = LDBL_DIG;
+          field_width = LDBL_STRLEN_BOUND_L (decimal_point_len);
           break;
 
         default:
           abort ();
         }
 
-      field_width = precision + 8;
-      sprintf (tspec->fmt_string, pre_fmt_string, precision);
-      assert (strlen (tspec->fmt_string) < FMT_BYTES_ALLOCATED);
       break;
 
     case 'a':
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 3bd7ad1..971f427 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -149,6 +149,7 @@ TESTS =						\
   misc/xstrtol					\
   tail-2/pid					\
   misc/od					\
+  misc/od-float					\
   misc/mktemp					\
   misc/arch					\
   misc/pr					\
diff --git a/tests/misc/od-float b/tests/misc/od-float
new file mode 100755
index 0000000..fb9fb42
--- /dev/null
+++ b/tests/misc/od-float
@@ -0,0 +1,58 @@
+#!/bin/sh
+# Test od on floating-point values.
+
+# Copyright (C) 2010 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ od
+
+export LC_ALL=C
+
+# Test for a bug in coreutils up through 8.7: od was losing
+# information when asked to parse floating-point values.  The numeric
+# tests are valid only on Intel-like hosts, but that should be good
+# enough to detect regressions, as they are designed to succeed on
+# non-Intel-like hosts.  Also, test for another bug in coreutils 8.7
+# on x86: sometimes there was no space between the columns.
+
+set x $(echo aaaabaaa | tr ab '\376\377' | od -t fF) ||
+  framework_failure
+case "$*" in
+*0-*) fail=1;;
+esac
+case $3,$4 in
+-1.694740e+38,-1.694740e+38) fail=1;;
+esac
+
+set x $(echo aaaaaaaabaaaaaaa | tr ab '\376\377' | od -t fD) ||
+  framework_failure
+case "$*" in
+*0-*) fail=1;;
+esac
+case $3,$4 in
+-5.314010372517808e+303,-5.314010372517808e+303) fail=1;;
+esac
+
+set x $(echo aaaaaaaaaaaaaaaabaaaaaaaaaaaaaaa | tr ab '\376\377' | od -t fL) ||
+  framework_failure
+case "$*" in
+*0-*) fail=1;;
+esac
+case $3,$4 in
+-1.023442870282055988e+4855,-1.023442870282055988e+4855) fail=1;;
+esac
+
+Exit $fail
-- 
1.7.2