bug#14530: [PATCH] od: -wN, N>64K, avoid misbehavior on systems with 32-bit size_t

[Jim Meyering]

Rich reported this bug privately.

Here's a proposed patch.
I could have fixed the underlying problem with
explicit casts to a wider type (including floating point types),
but this seemed cleanest, if perhaps a little too subtle.
I'd rather not write a comment in each of three places,
and didn't really want to factor out a 3-input function
just to compute each next_pad value.

Along the way, I noticed that an unreasonably large width
caused trouble, but I'm not inclined to worry about it:

  $ : | od -An -a -w$(echo 2^32-1|bc)
  od: memory exhausted

>From 19a4227793820f74bd6ca503ded3fafe65425eff Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 1 Jun 2013 19:20:06 -0700
Subject: [PATCH] od: -wN, N>64K, avoid misbehavior on systems with 32-bit
 size_t

* src/od.c (PRINT_FIELDS): Declare "i" to be of type uintmax_t, so that
the numerator in the expression for "next_pad" does not overflow.
(print_named_ascii): Likewise.
(print_ascii): Likewise.
Bug introduced via commit v6.12-42-g20c0b87.
* tests/misc/od.pl: Exercise each of the three affected code paths.
* NEWS (Bug fixes): Mention it.
Reported by Rich Burridge.
---
 NEWS             | 4 ++++
 THANKS.in        | 1 +
 src/od.c         | 6 +++---
 tests/misc/od.pl | 7 +++++++
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/NEWS b/NEWS
index 721e05b..cad5eee 100644
--- a/NEWS
+++ b/NEWS
@@ -16,10 +16,14 @@ GNU coreutils NEWS                                    -*- 
outline -*-
   mkdir, mkfifo, and mknod now work better when creating a file in a directory
   with a default ACL whose umask disagrees with the process's umask, on a
   system such as GNU/Linux where directory ACL umasks override process umasks.
   [bug introduced in coreutils-6.0]

+  od -wN with N larger than 64K on a system with 32-bit size_t would
+  print approximately 2*N bytes of extraneous padding.
+  [Bug introduced in coreutils-7.0]
+
   tail --retry -f now waits for the files specified to appear.  Before, tail
   would immediately exit when such a file is inaccessible during the initial
   open.
   [This bug was introduced when inotify support was added in coreutils-7.5]

diff --git a/THANKS.in b/THANKS.in
index 67b60b9..f399d77 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -519,10 +519,11 @@ Rainer Orth                         address@hidden
 Ralf W. Stephan                     address@hidden
 Ralph Loader                        address@hidden
 Rasmus Borup Hansen                 address@hidden
 Raul Miller                         address@hidden
 Raúl Núñez de Arenas Coronado       address@hidden
+Rich Burridge                       address@hidden
 Richard A Downing                   address@hidden
 Richard Braakman                    address@hidden
 Richard Dawe                        address@hidden
 Richard J. Rauenzahn                address@hidden
 Richard Neill                       address@hidden
diff --git a/src/od.c b/src/od.c
index e8cab46..1c23401 100644
--- a/src/od.c
+++ b/src/od.c
@@ -398,11 +398,11 @@ and so on for G, T, P, E, Z, Y.\n\
 static void                                                             \
 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;                                                             \
+  uintmax_t i;                                                             \
   int pad_remaining = pad;                                              \
   for (i = fields; blank < i; i--)                                      \
     {                                                                   \
       int next_pad = pad * (i - 1) / fields;                            \
       int adjusted_width = pad_remaining - next_pad + width;            \
@@ -454,11 +454,11 @@ static void
 print_named_ascii (size_t fields, size_t blank, void const *block,
                    const char *unused_fmt_string _GL_UNUSED,
                    int width, int pad)
 {
   unsigned char const *p = block;
-  size_t i;
+  uintmax_t i;
   int pad_remaining = pad;
   for (i = fields; blank < i; i--)
     {
       int next_pad = pad * (i - 1) / fields;
       int masked_c = *p++ & 0x7f;
@@ -485,11 +485,11 @@ static void
 print_ascii (size_t fields, size_t blank, void const *block,
              const char *unused_fmt_string _GL_UNUSED, int width,
              int pad)
 {
   unsigned char const *p = block;
-  size_t i;
+  uintmax_t i;
   int pad_remaining = pad;
   for (i = fields; blank < i; i--)
     {
       int next_pad = pad * (i - 1) / fields;
       unsigned char c = *p++;
diff --git a/tests/misc/od.pl b/tests/misc/od.pl
index 0649b1c..fb579aa 100755
--- a/tests/misc/od.pl
+++ b/tests/misc/od.pl
@@ -55,10 +55,17 @@ my @Tests =

      # Ensure that od -j doesn't fseek across a nonempty file in /proc,
      # even if the kernel reports that the file has stat.st_size = 0.
      ['j-proc', "-An -c -j $proc_file_byte_count $proc_file",
                                {IN=>{f2=>'e'}}, {OUT=>"   e\n"}],
+
+     # Ensure that a large width does not cause trouble.
+     # From coreutils-7.0 through coreutils-8.21, these would print
+     # approximately 128KiB of padding.
+     ['wide-a',   '-a -w65537 -An', {IN=>{g=>'x'}}, {OUT=>"   x\n"}],
+     ['wide-c',   '-c -w65537 -An', {IN=>{g=>'x'}}, {OUT=>"   x\n"}],
+     ['wide-x', '-tx1 -w65537 -An', {IN=>{g=>'B'}}, {OUT=>" 42\n"}],
     );

 my $save_temps = $ENV{DEBUG};
 my $verbose = $ENV{VERBOSE};

--
1.8.3.101.g727a46b