]> git.ipfire.org Git - thirdparty/coreutils.git/commitdiff
od: improve handling of padding
authorEric Blake <ebb9@byu.net>
Thu, 12 Jun 2008 17:05:09 +0000 (11:05 -0600)
committerJim Meyering <meyering@redhat.com>
Fri, 13 Jun 2008 21:07:58 +0000 (23:07 +0200)
* src/od.c (decode_one_format): Alter the format, again.
(FMT_BYTES_ALLOCATED): Reduce size by adjusting to new format.
(MAX_INTEGRAL_TYPE_SIZE): Move earlier in the file.
(charname): Turn it into a 2D array, since there's no need for
pointers now.
(PRINT_TYPE, print_named_ascii, print_ascii): Add a width
parameter.
(write_block): Account for width parameter.
Using ideas from Paul Eggert.

src/od.c

index 5c0b1b7967001547bb466d88891080d2f7042161..5b4b7bd4a6b6c297aadadab07a827582a05dac17 100644 (file)
--- a/src/od.c
+++ b/src/od.c
@@ -82,31 +82,44 @@ enum output_format
     CHARACTER
   };
 
-/* The maximum number of bytes needed for a format string,
-   including the trailing null.  */
+#define MAX_INTEGRAL_TYPE_SIZE sizeof (unsigned_long_long_int)
+
+/* The maximum number of bytes needed for a format string, including
+   the trailing nul.  Each format string expects a variable amount of
+   padding (guaranteed to be at least 1 plus the field width), then an
+   element that will be formatted in the field.  */
 enum
   {
     FMT_BYTES_ALLOCATED =
-      MAX ((sizeof "%*s%0" - 1 + INT_STRLEN_BOUND (int)
+      MAX ((sizeof "%*.99" - 1
            + MAX (sizeof "ld",
                   MAX (sizeof PRIdMAX,
                        MAX (sizeof PRIoMAX,
                             MAX (sizeof PRIuMAX,
                                  sizeof PRIxMAX))))),
-          sizeof "%*s%.Le" + 2 * INT_STRLEN_BOUND (int))
+          sizeof "%*.99Le")
   };
 
+/* 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
    old-style options) is represented by one of these structures.  */
 struct tspec
   {
     enum output_format fmt;
-    enum size_spec size;
-    void (*print_function) (size_t, size_t, void const *, char const *, int);
-    char fmt_string[FMT_BYTES_ALLOCATED];
+    enum size_spec size; /* Type of input object.  */
+    /* FIELDS is the number of fields per line, BLANK is the number of
+       fields to leave blank.  WIDTH is width of one field, excluding
+       leading space, and PAD is total pad to divide among FIELDS.
+       PAD is at least as large as FIELDS.  */
+    void (*print_function) (size_t fields, size_t blank, void const *data,
+                           char const *fmt, int width, int pad);
+    char fmt_string[FMT_BYTES_ALLOCATED]; /* Of the style "%*d".  */
     bool hexl_mode_trailer;
-    int field_width;
-    int pad_width;
+    int field_width; /* Minimum width of a field, excluding leading space.  */
+    int pad_width; /* Total padding to be divided among fields.  */
   };
 
 /* Convert the number of 8-bit bytes of a binary representation to
@@ -131,8 +144,6 @@ static unsigned int const bytes_to_unsigned_dec_digits[] =
 static unsigned int const bytes_to_hex_digits[] =
 {0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, 28, 30, 32};
 
-#define MAX_INTEGRAL_TYPE_SIZE sizeof (unsigned_long_long_int)
-
 /* It'll be a while before we see integral types wider than 16 bytes,
    but if/when it happens, this check will catch it.  Without this check,
    a wider type would provoke a buffer overrun.  */
@@ -163,7 +174,7 @@ static const int width_bytes[] =
 verify (sizeof width_bytes / sizeof width_bytes[0] == N_SIZE_SPECS);
 
 /* Names for some non-printing characters.  */
-static const char *const charname[33] =
+static char const charname[33][4] =
 {
   "nul", "soh", "stx", "etx", "eot", "enq", "ack", "bel",
   "bs", "ht", "nl", "vt", "ff", "cr", "so", "si",
@@ -183,7 +194,10 @@ static int address_base;
 /* Width of a normal address.  */
 static int address_pad_len;
 
+/* Minimum length when detecting --strings.  */
 static size_t string_min;
+
+/* True when in --strings mode.  */
 static bool flag_dump_strings;
 
 /* True if we should recognize the older non-option arguments
@@ -389,16 +403,17 @@ implies 32.  By default, od uses -A o -t oS -w16.\n\
 
 #define PRINT_TYPE(N, T)                                                \
 static void                                                             \
-N (size_t fields, size_t limit, void const *block,                      \
-   char const *fmt_string, int pad)                                     \
+N (size_t fields, size_t blank, void const *block,                      \
+   char const *fmt_string, int width, int pad)                          \
 {                                                                       \
   T const *p = block;                                                   \
   size_t i;                                                             \
-  for (i = fields; limit < i; i--)                                      \
+  int pad_remaining = pad;                                              \
+  for (i = fields; blank < i; i--)                                      \
     {                                                                   \
-      int local_pad = (pad + i / 2) / i;                                \
-      xprintf (fmt_string, local_pad, "", *p++);                        \
-      pad -= local_pad;                                                 \
+      int next_pad = pad * (i - 1) / fields;                            \
+      xprintf (fmt_string, pad_remaining - next_pad + width, *p++);     \
+      pad_remaining = next_pad;                                         \
     }                                                                   \
 }
 
@@ -430,17 +445,19 @@ dump_hexl_mode_trailer (size_t n_bytes, const char *block)
 }
 
 static void
-print_named_ascii (size_t fields, size_t limit, void const *block,
-                  const char *unused_fmt_string ATTRIBUTE_UNUSED, int pad)
+print_named_ascii (size_t fields, size_t blank, void const *block,
+                  const char *unused_fmt_string ATTRIBUTE_UNUSED,
+                  int width, int pad)
 {
   unsigned char const *p = block;
   size_t i;
-  for (i = fields; limit < i; i--)
+  int pad_remaining = pad;
+  for (i = fields; blank < i; i--)
     {
-      int local_pad = (pad + i / 2) / i;
+      int next_pad = pad * (i - 1) / fields;
       int masked_c = *p++ & 0x7f;
       const char *s;
-      char buf[5];
+      char buf[2];
 
       if (masked_c == 127)
        s = "del";
@@ -448,69 +465,72 @@ print_named_ascii (size_t fields, size_t limit, void const *block,
        s = charname[masked_c];
       else
        {
-         sprintf (buf, "  %c", masked_c);
+         buf[0] = masked_c;
+         buf[1] = 0;
          s = buf;
        }
 
-      xprintf ("%*s%3s", local_pad, "", s);
-      pad -= local_pad;
+      xprintf ("%*s", pad_remaining - next_pad + width, s);
+      pad_remaining = next_pad;
     }
 }
 
 static void
-print_ascii (size_t fields, size_t limit, void const *block,
-            const char *unused_fmt_string ATTRIBUTE_UNUSED, int pad)
+print_ascii (size_t fields, size_t blank, void const *block,
+            const char *unused_fmt_string ATTRIBUTE_UNUSED, int width,
+            int pad)
 {
   unsigned char const *p = block;
   size_t i;
-  for (i = fields; limit < i; i--)
+  int pad_remaining = pad;
+  for (i = fields; blank < i; i--)
     {
-      int local_pad = (pad + i / 2) / i;
+      int next_pad = pad * (i - 1) / fields;
       unsigned char c = *p++;
       const char *s;
-      char buf[5];
+      char buf[4];
 
       switch (c)
        {
        case '\0':
-         s = " \\0";
+         s = "\\0";
          break;
 
        case '\a':
-         s = " \\a";
+         s = "\\a";
          break;
 
        case '\b':
-         s = " \\b";
+         s = "\\b";
          break;
 
        case '\f':
-         s = " \\f";
+         s = "\\f";
          break;
 
        case '\n':
-         s = " \\n";
+         s = "\\n";
          break;
 
        case '\r':
-         s = " \\r";
+         s = "\\r";
          break;
 
        case '\t':
-         s = " \\t";
+         s = "\\t";
          break;
 
        case '\v':
-         s = " \\v";
+         s = "\\v";
          break;
 
        default:
-         sprintf (buf, (isprint (c) ? "  %c" : "%03o"), c);
+         sprintf (buf, (isprint (c) ? "%c" : "%03o"), c);
          s = buf;
        }
 
-      xprintf ("%*s%3s", local_pad, "", s);
-      pad -= local_pad;
+      xprintf ("%*s", pad_remaining - next_pad + width, s);
+      pad_remaining = next_pad;
     }
 }
 
@@ -550,9 +570,11 @@ simple_strtoul (const char *s, const char **p, unsigned long int *val)
        fmt = SIGNED_DECIMAL;
        size = INT or LONG; (whichever integral_type_size[4] resolves to)
        print_function = print_int; (assuming size == INT)
-       fmt_string = "%*s%011d";
+       field_width = 11;
+       fmt_string = "%*d";
       }
-   pad_width is determined later, but is at least 1
+   pad_width is determined later, but is at least as large as the
+   number of fields printed per row.
    S_ORIG is solely for reporting errors.  It should be the full format
    string argument.
    */
@@ -565,7 +587,8 @@ decode_one_format (const char *s_orig, const char *s, const char **next,
   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);
+  void (*print_function) (size_t, size_t, void const *, char const *,
+                         int, int);
   const char *p;
   char c;
   int field_width;
@@ -639,28 +662,28 @@ this system doesn't provide a %lu-byte integral type"), quote (s_orig), size);
        {
        case 'd':
          fmt = SIGNED_DECIMAL;
-         sprintf (tspec->fmt_string, "%%*s%%%d%s",
-                  (field_width = bytes_to_signed_dec_digits[size]),
+         field_width = bytes_to_signed_dec_digits[size];
+         sprintf (tspec->fmt_string, "%%*%s",
                   ISPEC_TO_FORMAT (size_spec, "d", "ld", PRIdMAX));
          break;
 
        case 'o':
          fmt = OCTAL;
-         sprintf (tspec->fmt_string, "%%*s%%0%d%s",
+         sprintf (tspec->fmt_string, "%%*.%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, "%%*s%%%d%s",
-                  (field_width = bytes_to_unsigned_dec_digits[size]),
+         field_width = bytes_to_unsigned_dec_digits[size];
+         sprintf (tspec->fmt_string, "%%*%s",
                   ISPEC_TO_FORMAT (size_spec, "u", "lu", PRIuMAX));
          break;
 
        case 'x':
          fmt = HEXADECIMAL;
-         sprintf (tspec->fmt_string, "%%*s%%0%d%s",
+         sprintf (tspec->fmt_string, "%%*.%d%s",
                   (field_width = bytes_to_hex_digits[size]),
                   ISPEC_TO_FORMAT (size_spec, "x", "lx", PRIxMAX));
          break;
@@ -753,20 +776,20 @@ this system doesn't provide a %lu-byte floating point type"),
        {
        case FLOAT_SINGLE:
          print_function = print_float;
-         /* Don't use %#e; not all systems support it.  */
-         pre_fmt_string = "%%*s%%%d.%de";
+         /* FIXME - should we use %g instead of %e?  */
+         pre_fmt_string = "%%*.%de";
          precision = FLT_DIG;
          break;
 
        case FLOAT_DOUBLE:
          print_function = print_double;
-         pre_fmt_string = "%%*s%%%d.%de";
+         pre_fmt_string = "%%*.%de";
          precision = DBL_DIG;
          break;
 
        case FLOAT_LONG_DOUBLE:
          print_function = print_long_double;
-         pre_fmt_string = "%%*s%%%d.%dLe";
+         pre_fmt_string = "%%*.%dLe";
          precision = LDBL_DIG;
          break;
 
@@ -775,7 +798,8 @@ this system doesn't provide a %lu-byte floating point type"),
        }
 
       field_width = precision + 8;
-      sprintf (tspec->fmt_string, pre_fmt_string, field_width, precision);
+      sprintf (tspec->fmt_string, pre_fmt_string, precision);
+      assert (strlen (tspec->fmt_string) < FMT_BYTES_ALLOCATED);
       break;
 
     case 'a':
@@ -1095,8 +1119,7 @@ format_address_label (uintmax_t address, char c)
    for a sequence of identical input blocks is the output for the first
    block followed by an asterisk alone on a line.  It is valid to compare
    the blocks PREV_BLOCK and CURR_BLOCK only when N_BYTES == BYTES_PER_BLOCK.
-   That condition may be false only for the last input block -- and then
-   only when it has not been padded to length BYTES_PER_BLOCK.  */
+   That condition may be false only for the last input block.  */
 
 static void
 write_block (uintmax_t current_offset, size_t n_bytes,
@@ -1138,7 +1161,7 @@ write_block (uintmax_t current_offset, size_t n_bytes,
            printf ("%*s", address_pad_len, "");
          (*spec[i].print_function) (fields_per_block, blank_fields,
                                     curr_block, spec[i].fmt_string,
-                                    spec[i].pad_width);
+                                    spec[i].field_width, spec[i].pad_width);
          if (spec[i].hexl_mode_trailer)
            {
              /* space-pad out to full line width, then dump the trailer */