bug-coreutils
[Top][All Lists]
Advanced

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

[PATCH v2] Is od broken?


From: Eric Blake
Subject: [PATCH v2] Is od broken?
Date: Wed, 11 Jun 2008 22:36:56 +0000 (UTC)
User-agent: Loom/3.14 (http://gmane.org/)

Jim Meyering <jim <at> meyering.net> writes:

> > Here's my attempt at a series to address this:
> >
> > Eric Blake (3):
> >   od defaults to -toS, not -td2.
> >   Align multiple od -t specs.
> >   Simplify long double support.
> 
> Thanks a lot!
> This looks like a fine improvement.
> I'll review the code tomorrow or Friday.
> 

Review this series instead (changes from v1: resequence the series, use xprintf 
so that invalid long doubles print as nan instead of garbage, use variable-
width padding to minimize whitespace, omit printing fields on the last line if 
the field consists entirely of the padding used to acheive the lcm width).

Eric Blake (4):
  od defaults to -toS, not -td2.
  simplify long double support
  use gnulib printf replacement in od as necessary
  align multiple od -t specs

 NEWS                     |    8 ++
 THANKS                   |    1 +
 m4/jm-macros.m4          |    1 -
 src/od.c                 |  230 +++++++++++++++++++---------------------------
 tests/Makefile.am        |    1 +
 tests/misc/od-multiple-t |   47 ++++++++++
 6 files changed, 153 insertions(+), 135 deletions(-)
 create mode 100755 tests/misc/od-multiple-t

For an example of the improved output:

old:

$ od blah -tfLz -txCz -toSz
0000000   1.000000000000000000e+00  >.........?..<
        00 00 00 00 00 00 00 80 ff 3f 00 00  >.........?..<
        000000 000000 000000 100000 037777 000000  >.........?..<
0000014 0.000000000000000000e+9999  >this is a te<
        74 68 69 73 20 69 73 20 61 20 74 65  >this is a te<
        064164 071551 064440 020163 020141 062564  >this is a te<
0000030 2.497585008459447006e-4945  >st..........<
        73 74 0a 00 00 00 00 00 00 00 00 00  >st..........<
        072163 000012 000000 000000 000000 000000  >st..........<
0000033

new:

$ src/od blah -tfLz -txCz -toSz
0000000                  1.000000000000000000e+00  >.........?..<
         00 00  00 00  00 00  00 80  ff 3f  00 00  >.........?..<
        000000 000000 000000 100000 037777 000000  >.........?..<
0000014                                       nan  >this is a te<
         74 68  69 73  20 69  73 20  61 20  74 65  >this is a te<
        064164 071551 064440 020163 020141 062564  >this is a te<
0000030                2.497585008459447006e-4945  >st.<
         73 74  0a                                 >st.<
        072163 000012                              >st.<
0000033

Again, if gmane botches this, use:
http://home.comcast.net/~ericblake/coreutils.patch8

>From 8b21806ef2f7676348b7807fd1b38caee2eb45b0 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 11 Jun 2008 08:01:31 -0600
Subject: [PATCH] od defaults to -toS, not -td2.

* src/od.c (usage): Correct description of default.

Signed-off-by: Eric Blake <address@hidden>
---
 src/od.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/od.c b/src/od.c
index 4df8e7d..97e43ea 100644
--- a/src/od.c
+++ b/src/od.c
@@ -382,7 +382,7 @@ output line.  \
 "), stdout);
       fputs (_("\
 --string without a number implies 3.  --width without a number\n\
-implies 32.  By default, od uses -A o -t d2 -w16.\n\
+implies 32.  By default, od uses -A o -t oS -w16.\n\
 "), stdout);
       emit_bug_reporting_address ();
     }
-- 
1.5.5.1


>From a96bfe0eb8930e4ec3b93acb176e70569ba13d16 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 11 Jun 2008 11:45:16 -0600
Subject: [PATCH] simplify long double support

* m4/jm-macros.m4 (gl_CHECK_ALL_TYPES): Remove obsolete check for
AC_C_LONG_DOUBLE.
* src/od.c (LONG_DOUBLE): Delete.
(width_bytes, MAX_FP_TYPE_SIZE, decode_one_format, main): Just use
'long double' directly.
(print_long_double): No longer protect by HAVE_LONG_DOUBLE.

Signed-off-by: Eric Blake <address@hidden>
---
 m4/jm-macros.m4 |    1 -
 src/od.c        |   24 +++++++-----------------
 2 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4
index cf1f2f0..9680e95 100644
--- a/m4/jm-macros.m4
+++ b/m4/jm-macros.m4
@@ -140,7 +140,6 @@ AC_DEFUN([gl_CHECK_ALL_TYPES],
   AC_REQUIRE([AC_C_BIGENDIAN])
   AC_REQUIRE([AC_C_VOLATILE])
   AC_REQUIRE([AC_C_INLINE])
-  AC_REQUIRE([AC_C_LONG_DOUBLE])
   AC_REQUIRE([AC_TYPE_UNSIGNED_LONG_LONG_INT])
 
   AC_REQUIRE([gl_CHECK_ALL_HEADERS])
diff --git a/src/od.c b/src/od.c
index 97e43ea..830f2ab 100644
--- a/src/od.c
+++ b/src/od.c
@@ -34,12 +34,6 @@
 
 #include <float.h>
 
-#ifdef HAVE_LONG_DOUBLE
-typedef long double LONG_DOUBLE;
-#else
-typedef double LONG_DOUBLE;
-#endif
-
 /* The default number of input bytes per output line.  */
 #define DEFAULT_BYTES_PER_BLOCK 16
 
@@ -159,7 +153,7 @@ static const int width_bytes[] =
   sizeof (unsigned_long_long_int),
   sizeof (float),
   sizeof (double),
-  sizeof (LONG_DOUBLE)
+  sizeof (long double)
 };
 
 /* Ensure that for each member of `enum size_spec' there is an
@@ -259,7 +253,7 @@ static bool have_read_stdin;
 /* Map the size in bytes to a type identifier.  */
 static enum size_spec integral_type_size[MAX_INTEGRAL_TYPE_SIZE + 1];
 
-#define MAX_FP_TYPE_SIZE sizeof (LONG_DOUBLE)
+#define MAX_FP_TYPE_SIZE sizeof (long double)
 static enum size_spec fp_type_size[MAX_FP_TYPE_SIZE + 1];
 
 static char const short_options[] = "A:aBbcDdeFfHhIij:LlN:OoS:st:vw::Xx";
@@ -472,7 +466,6 @@ print_double (size_t n_bytes, void const *block,
     printf (fmt_string, *p++);
 }
 
-#ifdef HAVE_LONG_DOUBLE
 static void
 print_long_double (size_t n_bytes, void const *block, char const *fmt_string)
 {
@@ -481,7 +474,6 @@ print_long_double (size_t n_bytes, void const *block,
   for (i = n_bytes / sizeof *p; i != 0; i--)
     printf (fmt_string, *p++);
 }
-#endif
 
 static void
 dump_hexl_mode_trailer (size_t n_bytes, const char *block)
@@ -782,7 +774,7 @@ this system doesn't provide a %lu-byte integral
 
        case 'L':
          ++s;
-         size = sizeof (LONG_DOUBLE);
+         size = sizeof (long double);
          break;
 
        default:
@@ -827,13 +819,11 @@ this system doesn't provide a %lu-byte floating
          precision = DBL_DIG;
          break;
 
-#ifdef HAVE_LONG_DOUBLE
        case FLOAT_LONG_DOUBLE:
          print_function = print_long_double;
          pre_fmt_string = " %%%d.%dLe";
          precision = LDBL_DIG;
          break;
-#endif
 
        default:
          abort ();
@@ -1586,10 +1576,10 @@ main (int argc, char **argv)
     fp_type_size[i] = NO_SIZE;
 
   fp_type_size[sizeof (float)] = FLOAT_SINGLE;
-  /* The array entry for `double' is filled in after that for LONG_DOUBLE
-     so that if `long double' is the same type or if long double isn't
-     supported FLOAT_LONG_DOUBLE will never be used.  */
-  fp_type_size[sizeof (LONG_DOUBLE)] = FLOAT_LONG_DOUBLE;
+  /* The array entry for `double' is filled in after that for `long double'
+     so that if they are the same size, we avoid any overhead of
+     long double computation in libc.  */
+  fp_type_size[sizeof (long double)] = FLOAT_LONG_DOUBLE;
   fp_type_size[sizeof (double)] = FLOAT_DOUBLE;
 
   n_specs = 0;
-- 
1.5.5.1


>From 46d535eacf669defcec3df046dd5ad6e0b9b9e21 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 11 Jun 2008 15:02:20 -0600
Subject: [PATCH] use gnulib printf replacement in od as necessary

* src/od.c (includes): Add xprintf.h.
(PRINT_TYPE): New macro, using xprintf instead of printf.
(print_s_char, print_char, print_s_short, print_short, print_int)
(print_long, print_long_long, print_float, print_double)
(print_long_double): Factor into PRINT_TYPE macro.
(print_named_ascii, print_ascii): Use xprintf.
* NEWS: Mention this as a bug fix.

Signed-off-by: Eric Blake <address@hidden>
---
 NEWS     |    5 +++
 src/od.c |  110 ++++++++++++-------------------------------------------------
 2 files changed, 27 insertions(+), 88 deletions(-)

diff --git a/NEWS b/NEWS
index 97f3162..056d6e8 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,11 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   md5sum now accepts the new option, --quiet, to suppress the printing of
   'OK' messages.  sha1sum, sha224sum, sha384sum, and sha512sum accept it, too.
 
+** Bug fixes
+
+  od no longer suffers from platform bugs in printf(2).  This is
+  probably most noticeable when using 'od -tfL' to print long doubles.
+
 ** Improvements
 
   Improved support for access control lists (ACLs): On MacOS X, Solaris 7..10,
diff --git a/src/od.c b/src/od.c
index 830f2ab..0c95322 100644
--- a/src/od.c
+++ b/src/od.c
@@ -25,6 +25,7 @@
 #include "system.h"
 #include "error.h"
 #include "quote.h"
+#include "xprintf.h"
 #include "xstrtol.h"
 
 /* The official name of this program (e.g., no `g' prefix).  */
@@ -385,95 +386,28 @@ implies 32.  By default, od uses -A o -t oS -w16.\n\
 
 /* Define the print functions.  */
 
-static void
-print_s_char (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  signed char const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
-
-static void
-print_char (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  unsigned char const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
-
-static void
-print_s_short (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  short int const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
-
-static void
-print_short (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  unsigned short int const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
+#define PRINT_TYPE(N, T)                                                \
+static void                                                             \
+N (size_t n_bytes, void const *block, char const *fmt_string)           \
+{                                                                       \
+  T const *p = block;                                                   \
+  size_t i;                                                             \
+  for (i = n_bytes / sizeof *p; i != 0; i--)                            \
+    xprintf (fmt_string, *p++);                                         \
 }
 
-static void
-print_int (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  unsigned int const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
+PRINT_TYPE (print_s_char, signed char)
+PRINT_TYPE (print_char, unsigned char)
+PRINT_TYPE (print_s_short, short int)
+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)
 
-static void
-print_long (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  unsigned long int const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
-
-static void
-print_long_long (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  unsigned_long_long_int const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
-
-static void
-print_float (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  float const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
-
-static void
-print_double (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  double const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
-
-static void
-print_long_double (size_t n_bytes, void const *block, char const *fmt_string)
-{
-  long double const *p = block;
-  size_t i;
-  for (i = n_bytes / sizeof *p; i != 0; i--)
-    printf (fmt_string, *p++);
-}
+#undef PRINT_TYPE
 
 static void
 dump_hexl_mode_trailer (size_t n_bytes, const char *block)
@@ -511,7 +445,7 @@ print_named_ascii (size_t n_bytes, void const *block,
          s = buf;
        }
 
-      printf (" %3s", s);
+      xprintf (" %3s", s);
     }
 }
 
@@ -566,7 +500,7 @@ print_ascii (size_t n_bytes, void const *block,
          s = buf;
        }
 
-      printf (" %3s", s);
+      xprintf (" %3s", s);
     }
 }
 
-- 
1.5.5.1


>From 414caf876c25c0e9a3a3d029b545a8f233a19f0d Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Wed, 11 Jun 2008 09:14:26 -0600
Subject: [PATCH] align multiple od -t specs

* src/od.c (struct tspec): Add pad_width field, and adjust
print_function prototype.
(decode_one_format): Rewrite all fmt_string values to account for
pad width.
(FMT_BYTES_ALLOCATED): Adjust to new format style.
(main): Compute pad width per spec.
(write_block): Account for pad width.
(dump): Don't print padding-only fields.
(PRINT_TYPE, print_named_ascii, print_ascii): All print functions
adjusted to use variable pad width.
* tests/Makefile.am (TESTS): Add test.
* tests/misc/od-multiple-t: New file.
* THANKS: Update.
* NEWS: Mention the improvement.
Reported by Gary Johnson.

Signed-off-by: Eric Blake <address@hidden>
---
 NEWS                     |    3 +
 THANKS                   |    1 +
 src/od.c                 |  104 +++++++++++++++++++++++++++++++--------------
 tests/Makefile.am        |    1 +
 tests/misc/od-multiple-t |   47 +++++++++++++++++++++
 5 files changed, 123 insertions(+), 33 deletions(-)
 create mode 100755 tests/misc/od-multiple-t

diff --git a/NEWS b/NEWS
index 056d6e8..ff107c0 100644
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,9 @@ GNU coreutils NEWS
   HP-UX 11, Tru64, AIX, IRIX 6.5, and Cygwin, "ls -l" now displays the presence
   of an ACL on a file via a '+' sign after the mode, and "cp -p" copies ACLs.
 
+  od now aligns fields across lines when printing multiple -t
+  specifiers, and no longer prints fields that resulted entirely from
+  padding the input out to the least common multiple width.
 
 * Noteworthy changes in release 6.12 (2008-05-31) [stable]
 
diff --git a/THANKS b/THANKS
index cb9b098..a9bb12f 100644
--- a/THANKS
+++ b/THANKS
@@ -182,6 +182,7 @@ Gabor Z. Papp                       address@hidden
 Gaël Quéri                          address@hidden
 Galen Hazelwood                     address@hidden
 Gary Anderson                       address@hidden
+Gary Johnson                        address@hidden
 Gary V. Vaughan                     address@hidden
 Gaute Hvoslef Kvalnes               address@hidden
 Geoff Collyer                       geoff at collyer.net
diff --git a/src/od.c b/src/od.c
index 0c95322..576de3e 100644
--- a/src/od.c
+++ b/src/od.c
@@ -87,13 +87,13 @@ enum output_format
 enum
   {
     FMT_BYTES_ALLOCATED =
-      MAX ((sizeof " %0" - 1 + INT_STRLEN_BOUND (int)
+      MAX ((sizeof "%*s%0" - 1 + INT_STRLEN_BOUND (int)
            + MAX (sizeof "ld",
                   MAX (sizeof PRIdMAX,
                        MAX (sizeof PRIoMAX,
                             MAX (sizeof PRIuMAX,
                                  sizeof PRIxMAX))))),
-          sizeof " %.Le" + 2 * INT_STRLEN_BOUND (int))
+          sizeof "%*s%.Le" + 2 * INT_STRLEN_BOUND (int))
   };
 
 /* Each output format specification (from `-t spec' or from
@@ -102,10 +102,11 @@ struct tspec
   {
     enum output_format fmt;
     enum size_spec size;
-    void (*print_function) (size_t, void const *, char const *);
+    void (*print_function) (size_t, size_t, void const *, char const *, int);
     char fmt_string[FMT_BYTES_ALLOCATED];
     bool hexl_mode_trailer;
     int field_width;
+    int pad_width;
   };
 
 /* Convert the number of 8-bit bytes of a binary representation to
@@ -388,12 +389,17 @@ implies 32.  By default, od uses -A o -t oS -w16.\n\
 
 #define PRINT_TYPE(N, T)                                                \
 static void                                                             \
-N (size_t n_bytes, void const *block, char const *fmt_string)           \
+N (size_t fields, size_t limit, void const *block,                      \
+   char const *fmt_string, int pad)                                     \
 {                                                                       \
   T const *p = block;                                                   \
   size_t i;                                                             \
-  for (i = n_bytes / sizeof *p; i != 0; i--)                            \
-    xprintf (fmt_string, *p++);                                         \
+  for (i = fields; limit < i; i--)                                      \
+    {                                                                   \
+      int local_pad = (pad + i / 2) / i;                                \
+      xprintf (fmt_string, local_pad, "", *p++);                        \
+      pad -= local_pad;                                                 \
+    }                                                                   \
 }
 
 PRINT_TYPE (print_s_char, signed char)
@@ -424,13 +430,14 @@ dump_hexl_mode_trailer (size_t n_bytes, const
 }
 
 static void
-print_named_ascii (size_t n_bytes, void const *block,
-                  const char *unused_fmt_string ATTRIBUTE_UNUSED)
+print_named_ascii (size_t fields, size_t limit, void const *block,
+                  const char *unused_fmt_string ATTRIBUTE_UNUSED, int pad)
 {
   unsigned char const *p = block;
   size_t i;
-  for (i = n_bytes; i > 0; i--)
+  for (i = fields; limit < i; i--)
     {
+      int local_pad = (pad + i / 2) / i;
       int masked_c = *p++ & 0x7f;
       const char *s;
       char buf[5];
@@ -445,18 +452,20 @@ print_named_ascii (size_t n_bytes, void const *block,
          s = buf;
        }
 
-      xprintf (" %3s", s);
+      xprintf ("%*s%3s", local_pad, "", s);
+      pad -= local_pad;
     }
 }
 
 static void
-print_ascii (size_t n_bytes, void const *block,
-            const char *unused_fmt_string ATTRIBUTE_UNUSED)
+print_ascii (size_t fields, size_t limit, void const *block,
+            const char *unused_fmt_string ATTRIBUTE_UNUSED, int pad)
 {
   unsigned char const *p = block;
   size_t i;
-  for (i = n_bytes; i > 0; i--)
+  for (i = fields; limit < i; i--)
     {
+      int local_pad = (pad + i / 2) / i;
       unsigned char c = *p++;
       const char *s;
       char buf[5];
@@ -500,7 +509,8 @@ print_ascii (size_t n_bytes, void const *block,
          s = buf;
        }
 
-      xprintf (" %3s", s);
+      xprintf ("%*s%3s", local_pad, "", s);
+      pad -= local_pad;
     }
 }
 
@@ -540,8 +550,9 @@ simple_strtoul (const char *s, const char **p,
        fmt = SIGNED_DECIMAL;
        size = INT or LONG; (whichever integral_type_size[4] resolves to)
        print_function = print_int; (assuming size == INT)
-       fmt_string = "%011d%c";
+       fmt_string = "%*s%011d";
       }
+   pad_width is determined later, but is at least 1
    S_ORIG is solely for reporting errors.  It should be the full format
    string argument.
    */
@@ -554,7 +565,7 @@ decode_one_format (const char *s_orig, const char *s,
   unsigned long int size;
   enum output_format fmt;
   const char *pre_fmt_string;
-  void (*print_function) (size_t, void const *, char const *);
+  void (*print_function) (size_t, size_t, void const *, char const *, int);
   const char *p;
   char c;
   int field_width;
@@ -628,28 +639,28 @@ this system doesn't provide a %lu-byte integral
        {
        case 'd':
          fmt = SIGNED_DECIMAL;
-         sprintf (tspec->fmt_string, " %%%d%s",
+         sprintf (tspec->fmt_string, "%%*s%%%d%s",
                   (field_width = bytes_to_signed_dec_digits[size]),
                   ISPEC_TO_FORMAT (size_spec, "d", "ld", PRIdMAX));
          break;
 
        case 'o':
          fmt = OCTAL;
-         sprintf (tspec->fmt_string, " %%0%d%s",
+         sprintf (tspec->fmt_string, "%%*s%%0%d%s",
                   (field_width = bytes_to_oct_digits[size]),
                   ISPEC_TO_FORMAT (size_spec, "o", "lo", PRIoMAX));
          break;
 
        case 'u':
          fmt = UNSIGNED_DECIMAL;
-         sprintf (tspec->fmt_string, " %%%d%s",
+         sprintf (tspec->fmt_string, "%%*s%%%d%s",
                   (field_width = bytes_to_unsigned_dec_digits[size]),
                   ISPEC_TO_FORMAT (size_spec, "u", "lu", PRIuMAX));
          break;
 
        case 'x':
          fmt = HEXADECIMAL;
-         sprintf (tspec->fmt_string, " %%0%d%s",
+         sprintf (tspec->fmt_string, "%%*s%%0%d%s",
                   (field_width = bytes_to_hex_digits[size]),
                   ISPEC_TO_FORMAT (size_spec, "x", "lx", PRIxMAX));
          break;
@@ -743,19 +754,19 @@ this system doesn't provide a %lu-byte floating
        case FLOAT_SINGLE:
          print_function = print_float;
          /* Don't use %#e; not all systems support it.  */
-         pre_fmt_string = " %%%d.%de";
+         pre_fmt_string = "%%*s%%%d.%de";
          precision = FLT_DIG;
          break;
 
        case FLOAT_DOUBLE:
          print_function = print_double;
-         pre_fmt_string = " %%%d.%de";
+         pre_fmt_string = "%%*s%%%d.%de";
          precision = DBL_DIG;
          break;
 
        case FLOAT_LONG_DOUBLE:
          print_function = print_long_double;
-         pre_fmt_string = " %%%d.%dLe";
+         pre_fmt_string = "%%*s%%%d.%dLe";
          precision = LDBL_DIG;
          break;
 
@@ -1118,18 +1129,23 @@ write_block (uintmax_t current_offset, size_t
       prev_pair_equal = false;
       for (i = 0; i < n_specs; i++)
        {
+          int datum_width = width_bytes[spec[i].size];
+          int fields_per_block = bytes_per_block / datum_width;
+          int blank_fields = (bytes_per_block - n_bytes) / datum_width;
          if (i == 0)
            format_address (current_offset, '\0');
          else
            printf ("%*s", address_pad_len, "");
-         (*spec[i].print_function) (n_bytes, curr_block, spec[i].fmt_string);
+         (*spec[i].print_function) (fields_per_block, blank_fields,
+                                     curr_block, spec[i].fmt_string,
+                                     spec[i].pad_width);
          if (spec[i].hexl_mode_trailer)
            {
              /* space-pad out to full line width, then dump the trailer */
-             int datum_width = width_bytes[spec[i].size];
-             int blank_fields = (bytes_per_block - n_bytes) / datum_width;
-             int field_width = spec[i].field_width + 1;
-             printf ("%*s", blank_fields * field_width, "");
+             int field_width = spec[i].field_width;
+              int pad_width = (spec[i].pad_width * blank_fields
+                               / fields_per_block);
+             printf ("%*s", blank_fields * field_width + pad_width, "");
              dump_hexl_mode_trailer (n_bytes, curr_block);
            }
          putchar ('\n');
@@ -1333,13 +1349,12 @@ dump (void)
 
       l_c_m = get_lcm ();
 
-      /* Make bytes_to_write the smallest multiple of l_c_m that
+      /* Ensure zero-byte padding up to the smallest multiple of l_c_m that
         is at least as large as n_bytes_read.  */
       bytes_to_write = l_c_m * ((n_bytes_read + l_c_m - 1) / l_c_m);
 
       memset (block[idx] + n_bytes_read, 0, bytes_to_write - n_bytes_read);
-      write_block (current_offset, bytes_to_write,
-                  block[!idx], block[idx]);
+      write_block (current_offset, n_bytes_read, block[!idx], block[idx]);
       current_offset += n_bytes_read;
     }
 
@@ -1479,6 +1494,7 @@ main (int argc, char **argv)
   bool modern = false;
   bool width_specified = false;
   bool ok = true;
+  size_t width_per_block = 0;
   static char const multipliers[] = "bEGKkMmPTYZ0";
 
   /* The old-style `pseudo starting address' to be printed in parentheses
@@ -1839,11 +1855,31 @@ it must be one character from [doxn]"),
        bytes_per_block = l_c_m;
     }
 
+  /* Compute padding necessary to align output block.  */
+  for (i = 0; i < n_specs; i++)
+    {
+      int fields_per_block = bytes_per_block / width_bytes[spec[i].size];
+      int block_width = (spec[i].field_width + 1) * fields_per_block;
+      if (width_per_block < block_width)
+        width_per_block = block_width;
+    }
+  for (i = 0; i < n_specs; i++)
+    {
+      int fields_per_block = bytes_per_block / width_bytes[spec[i].size];
+      int block_width = spec[i].field_width * fields_per_block;
+      spec[i].pad_width = width_per_block - block_width;
+    }
+
 #ifdef DEBUG
+  printf (_("lcm=%d, width_per_block=%zu\n"), l_c_m, width_per_block);
   for (i = 0; i < n_specs; i++)
     {
-      printf (_("%d: fmt=\"%s\" width=%d\n"),
-             i, spec[i].fmt_string, width_bytes[spec[i].size]);
+      int fields_per_block = bytes_per_block / width_bytes[spec[i].size];
+      assert (bytes_per_block % width_bytes[spec[i].size] == 0);
+      assert (1 <= spec[i].pad_width / fields_per_block);
+      printf (_("%d: fmt=\"%s\" in_width=%d out_width=%d pad=%d\n"),
+             i, spec[i].fmt_string, width_bytes[spec[i].size],
+              spec[i].field_width, spec[i].pad_width);
     }
 #endif
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index bc17299..d09e451 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -171,6 +171,7 @@ TESTS =                                             \
   misc/nl                                      \
   misc/nohup                                   \
   misc/od-N                                    \
+  misc/od-multiple-t                           \
   misc/od-x8                                   \
   misc/paste                                   \
   misc/pathchk1                                        \
diff --git a/tests/misc/od-multiple-t b/tests/misc/od-multiple-t
new file mode 100755
index 0000000..63fb7e4
--- /dev/null
+++ b/tests/misc/od-multiple-t
@@ -0,0 +1,47 @@
+#!/bin/sh
+# verify that multiple -t specifiers to od align well
+# This would fail before coreutils-6.13.
+
+# Copyright (C) 2008 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/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  od --version
+fi
+
+. $srcdir/test-lib.sh
+
+# Choose 48 bytes for the input, as that is lcm for 1, 2, 4, 8, 12, 16;
+# we don't anticipate any other native object size on modern hardware.
+seq 19 > in || framework_failure
+test `wc -c < in` -eq 48 || framework_failure
+
+fail=0
+
+list='a c dC dS dI dL oC oS oI oL uC uS uI uL xC xS xI xL fF fD fL'
+for format1 in $list; do
+  for format2 in $list; do
+    od -An -t${format1}z -t${format2}z in > out-raw || fail=1
+    linewidth=`head -n1 out-raw | wc -c`
+    linecount=`wc -l < out-raw`
+    echo $format1 $format2 `wc -c < out-raw` >> out
+    echo $format1 $format2 `expr $linewidth '*' $linecount` >> exp
+  done
+done
+
+compare out exp || fail=1
+
+(exit $fail); exit $fail
-- 
1.5.5.1







reply via email to

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