From 46a811b9e7c0e9fefb75864d198f9a4f3fd0e6cf Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Wed, 11 Jun 2008 09:14:26 -0600 Subject: [PATCH] od: align multiple -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. --- NEWS | 3 ++ THANKS | 1 + src/od.c | 102 ++++++++++++++++++++++++++++++++--------------- tests/Makefile.am | 1 + tests/misc/od-multiple-t | 47 ++++++++++++++++++++++ 5 files changed, 121 insertions(+), 33 deletions(-) create mode 100755 tests/misc/od-multiple-t diff --git a/NEWS b/NEWS index 750b15e..3f2a8db 100644 --- a/NEWS +++ b/NEWS @@ -29,6 +29,9 @@ GNU coreutils NEWS -*- outline -*- 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 3e5443b..098f1d7 100644 --- a/THANKS +++ b/THANKS @@ -183,6 +183,7 @@ Gabor Z. Papp gzp@gzp.org.hu Gaël Quéri gqueri@mail.dotcom.fr Galen Hazelwood galenh@micron.net Gary Anderson ganderson@clark.net +Gary Johnson garyjohn@spk.agilent.com Gary V. Vaughan gary@gnu.org Gaute Hvoslef Kvalnes gaute@verdsveven.com Geoff Collyer geoff at collyer.net diff --git a/src/od.c b/src/od.c index 0c95322..5c0b1b7 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 char *block) } 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, 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 = "%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, const char **next, 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 type"), quote (s_orig), size); { 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 point type"), 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 n_bytes, 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 f07837b..91db7bc 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 . + +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 -- 2.7.4