Subject: Re: bug#10016: ls -lk is wrong

From: Paul Eggert <[email-masked]>
Date: Fri, 11 Nov 2011 23:40:36 -0800

On 11/11/11 13:10, Jim Meyering wrote:
> Do you feel like writing the patch?

Sure.  I wrote and pushed the following.
The test case isn't elegant, but at least it catches the bug.

ls: -k no longer affects -l's file sizes
This fixes an incompatibility with POSIX 2008 and with BSD.
Problem reported by Abdallah Clark (Bug#9939)
via Alan Curry (Bug#10016).
* NEWS: Document this.
* doc/coreutils.texi (General output formatting): Document the
new -k behavior, and --kibibytes.
* src/ls.c (file_human_output_opts): New static var.
(long_options, usage): Add --kibibytes.
(decode_switches, gobble_file, print_long_format):
Implement the new -k behavior.
* tests/ls/block-size: New file.
* tests/Makefile.am (TESTS): Add it.
diff --git a/NEWS b/NEWS
index 081989d..1b0f2f5 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,13 @@ GNU coreutils NEWS                                    -*- outline -*-

 ** Bug fixes

+  ls's -k option no longer affects how ls -l outputs file sizes.
+  It now affects only the per-directory block counts written by -l,
+  and the sizes written by -s.  This is for compatibility with BSD
+  and with POSIX 2008.  Because -k is no longer equivalent to
+  --block-size=1KiB, a new long option --kibibyte stands for -k.
+  [bug introduced in coreutils-4.5.4]
+
   rm -rf DIR would fail with "Device or resource busy" on Cygwin with NWFS
   and NcFsd file systems.  This did not affect Unix/Linux-based kernels.
   [bug introduced in coreutils-8.0, when rm began using fts]
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 2c33fe8..4531440 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -7127,10 +7127,19 @@ Append @samp{*} for executable regular files, otherwise behave as for
 @end table

 @item -k
+@itemx --kibibytes
 @opindex -k
-Print file sizes in 1024-byte blocks, overriding the default block
-size (@pxref{Block size}).
-This option is equivalent to @option{--block-size=1K}.
+@opindex --kibibytes
+Set the default block size to its normal value of 1024 bytes,
+overriding any contrary specification in environment variables
+(@pxref{Block size}).  This option is in turn overridden by the
+@option{--block-size}, @option{-h} or @option{--human-readable}, and
+@option{--si} options.
+
+The @option{-k} or @option{--kibibytes} option affects the
+per-directory block count written by the @option{-l} and similar
+options, and the size written by the @option{-s} or @option{--size}
+option.  It does not affect the file size written by @option{-l}.

 @item -m
 @itemx --format=commas
diff --git a/src/ls.c b/src/ls.c
index 1b0c250..b8a09b3 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -479,13 +479,14 @@ static bool numeric_ids;

 static bool print_block_size;

-/* Human-readable options for output.  */
+/* Human-readable options for output, when printing block counts.  */
 static int human_output_opts;

-/* The units to use when printing sizes other than file sizes.  */
+/* The units to use when printing block counts.  */
 static uintmax_t output_block_size;

 /* Likewise, but for file sizes.  */
+static int file_human_output_opts;
 static uintmax_t file_output_block_size = 1;

 /* Follow the output with a special string.  Using this format,
@@ -809,6 +810,7 @@ static struct option const long_options[] =
    GROUP_DIRECTORIES_FIRST_OPTION},
   {"human-readable", no_argument, NULL, 'h'},
   {"inode", no_argument, NULL, 'i'},
+  {"kibibytes", no_argument, NULL, 'k'},
   {"numeric-uid-gid", no_argument, NULL, 'n'},
   {"no-group", no_argument, NULL, 'G'},
   {"hide-control-chars", no_argument, NULL, 'q'},
@@ -1512,8 +1514,8 @@ decode_switches (int argc, char **argv)
 {
   char *time_style_option = NULL;

-  /* Record whether there is an option specifying sort type.  */
   bool sort_type_specified = false;
+  bool kibibytes_specified = false;

   qmark_funny_chars = false;

@@ -1582,14 +1584,6 @@ decode_switches (int argc, char **argv)
       }
   }

-  {
-    char const *ls_block_size = getenv ("LS_BLOCK_SIZE");
-    human_options (ls_block_size,
-                   &human_output_opts, &output_block_size);
-    if (ls_block_size || getenv ("BLOCK_SIZE"))
-      file_output_block_size = output_block_size;
-  }
-
   line_length = 80;
   {
     char const *p = getenv ("COLUMNS");
@@ -1689,7 +1683,8 @@ decode_switches (int argc, char **argv)
           break;

         case 'h':
-          human_output_opts = human_autoscale | human_SI | human_base_1024;
+          file_human_output_opts = human_output_opts =
+            human_autoscale | human_SI | human_base_1024;
           file_output_block_size = output_block_size = 1;
           break;

@@ -1698,8 +1693,7 @@ decode_switches (int argc, char **argv)
           break;

         case 'k':
-          human_output_opts = 0;
-          file_output_block_size = output_block_size = 1024;
+          kibibytes_specified = true;
           break;

         case 'l':
@@ -1937,12 +1931,14 @@ decode_switches (int argc, char **argv)
                                                  &output_block_size);
             if (e != LONGINT_OK)
               xstrtol_fatal (e, oi, 0, long_options, optarg);
+            file_human_output_opts = human_output_opts;
             file_output_block_size = output_block_size;
           }
           break;

         case SI_OPTION:
-          human_output_opts = human_autoscale | human_SI;
+          file_human_output_opts = human_output_opts =
+            human_autoscale | human_SI;
           file_output_block_size = output_block_size = 1;
           break;

@@ -1959,6 +1955,23 @@ decode_switches (int argc, char **argv)
         }
     }

+  if (! output_block_size)
+    {
+      char const *ls_block_size = getenv ("LS_BLOCK_SIZE");
+      human_options (ls_block_size,
+                     &human_output_opts, &output_block_size);
+      if (ls_block_size || getenv ("BLOCK_SIZE"))
+        {
+          file_human_output_opts = human_output_opts;
+          file_output_block_size = output_block_size;
+        }
+      if (kibibytes_specified)
+        {
+          human_output_opts = 0;
+          output_block_size = 1024;
+        }
+    }
+
   max_idx = MAX (1, line_length / MIN_COLUMN_WIDTH);

   filename_quoting_options = clone_quoting_options (NULL);
@@ -3025,7 +3038,8 @@ gobble_file (char const *name, enum filetype type, ino_t inode,
             {
               char buf[LONGEST_HUMAN_READABLE + 1];
               uintmax_t size = unsigned_file_size (f->stat.st_size);
-              int len = mbswidth (human_readable (size, buf, human_output_opts,
+              int len = mbswidth (human_readable (size, buf,
+                                                  file_human_output_opts,
                                                   1, file_output_block_size),
                                   0);
               if (file_size_width < len)
@@ -3767,7 +3781,8 @@ print_long_format (const struct fileinfo *f)
         (! f->stat_ok
          ? "?"
          : human_readable (unsigned_file_size (f->stat.st_size),
-                           hbuf, human_output_opts, 1, file_output_block_size));
+                           hbuf, file_human_output_opts, 1,
+                           file_output_block_size));
       int pad;
       for (pad = file_size_width - mbswidth (size, 0); 0 < pad; pad--)
         *p++ = ' ';
@@ -4672,7 +4687,7 @@ Mandatory arguments to long options are mandatory for short options too.\n\
   -i, --inode                print the index number of each file\n\
   -I, --ignore=PATTERN       do not list implied entries matching shell PATTERN\
 \n\
-  -k                         like --block-size=1K\n\
+  -k, --kibibytes            use 1024-byte blocks\n\
 "), stdout);
       fputs (_("\
   -l                         use a long listing format\n\
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5021c18..64366a4 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -415,6 +415,7 @@ TESTS =						\
   ln/slash-decorated-nonexistent-dest		\
   ln/target-1					\
   ls/abmon-align				\
+  ls/block-size					\
   ls/color-clear-to-eol				\
   ls/color-dtype-dir				\
   ls/color-norm					\
diff --git a/tests/ls/block-size b/tests/ls/block-size
new file mode 100644
index 0000000..16ede04
--- /dev/null
+++ b/tests/ls/block-size
@@ -0,0 +1,173 @@
+#!/bin/sh
+# Exercise ls --block-size and related options.
+
+# Copyright (C) 2011 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_ ls
+
+TZ=UTC0
+export TZ
+
+mkdir sub
+cd sub
+
+for size in 1024 4096 262144; do
+  echo foo | dd conv=sync bs=$size >file$size || fail=1
+done
+touch -d '2001-01-01 00:00' file* || fail=1
+
+size_etc='s/[^ ]* *[^ ]* *[^ ]* *[^ ]* *//'
+
+ls -l * | sed "$size_etc" >../out || fail=1
+POSIXLY_CORRECT=1 ls -l * | sed "$size_etc" >>../out || fail=1
+POSIXLY_CORRECT=1 ls -k -l * | sed "$size_etc" >>../out || fail=1
+
+for var in BLOCKSIZE BLOCK_SIZE LS_BLOCK_SIZE; do
+  for blocksize in 1 512 1K 1KiB; do
+    (eval $var=$blocksize && export $var &&
+     ls -l * &&
+     ls -l -k * &&
+     ls -l -k --block-size=$blocksize *
+    ) | sed "$size_etc" >>../out || fail=1
+  done
+done
+
+cd ..
+
+cat >exp <<'EOF'
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+2 Jan  1  2001 file1024
+512 Jan  1  2001 file262144
+8 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+2 Jan  1  2001 file1024
+512 Jan  1  2001 file262144
+8 Jan  1  2001 file4096
+2 Jan  1  2001 file1024
+512 Jan  1  2001 file262144
+8 Jan  1  2001 file4096
+2 Jan  1  2001 file1024
+512 Jan  1  2001 file262144
+8 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+1024 Jan  1  2001 file1024
+262144 Jan  1  2001 file262144
+4096 Jan  1  2001 file4096
+2 Jan  1  2001 file1024
+512 Jan  1  2001 file262144
+8 Jan  1  2001 file4096
+2 Jan  1  2001 file1024
+512 Jan  1  2001 file262144
+8 Jan  1  2001 file4096
+2 Jan  1  2001 file1024
+512 Jan  1  2001 file262144
+8 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+1 Jan  1  2001 file1024
+256 Jan  1  2001 file262144
+4 Jan  1  2001 file4096
+EOF
+
+compare out exp || fail=1
+
+Exit $fail