Re: coreutils 6.0: ls -CF output misaligned

[Jim Meyering]

Andreas Schwab <address@hidden> wrote:
> length_of_file_name_and_frills mishandles entries that have never been
> passed to stat.  This causes the output of ls -CF to be misaligned.
>
> $ /bin/ls -CF
> afros/       dejagnu/    gnus/           pack-dejagnu*  qemu/
> alsa-kernel/  diffutils/         grep/           pack-emacs*    sharutils/
> aranym/      do_update*  groff/          pack-erc*      src/
> arch/        elilo/      kerncvs/        pack-gcc*      stamp-last-update
> autoconf/     emacs/     libc/           pack-gdb*      stamp-prev-update
> automake/     erc/       libtool/        pack-gnats*    strace/
> binutils/     gcc/       m4/             pack-grep*     svn/
> bk/          gdb/        m68k/           pack-libtool*  tar/
> branch/      git/        oprofile/       pack-m4*       test/
> config/      gnash/      pack-autoconf*  pack-strace*   texinfo/
> coreutils/    gnats/     pack-automake*  pack-w3*       texmacs/
> cvsroot/      gnulib/    pack-binutils*  pbbuttons/     wotan/

Hi Andreas,

Thanks for the report.
Here's a patch:

2006-08-17  Jim Meyering  <address@hidden>

        ls -CF would misalign columns in some cases.
        * src/ls.c (get_type_indicator): New function.  extracted from...
        (print_type_indicator): ...here.  Use it.
        (length_of_file_name_and_frills): Use it here, too, rather than
        assuming stat.st_mode is valid.
        Reported by Andreas Schwab, here:
        <http://article.gmane.org/gmane.comp.gnu.core-utils.bugs/7774>
        FIXME: add a test for this: FYI, I did ls -CF /proc and visually
        inspected the result.

Index: src/ls.c
===================================================================
RCS file: /fetish/cu/src/ls.c,v
retrieving revision 1.435
diff -u -p -r1.435 ls.c
--- src/ls.c    9 Aug 2006 09:45:17 -0000       1.435
+++ src/ls.c    17 Aug 2006 15:32:19 -0000
@@ -3797,8 +3797,10 @@ print_file_name_and_frills (const struct
     print_type_indicator (f->stat_ok, f->stat.st_mode, f->filetype);
 }
 
-static void
-print_type_indicator (bool stat_ok, mode_t mode, enum filetype type)
+/* Given these arguments describing a file, return the single-byte
+   type indicator, or 0.  */
+static char
+get_type_indicator (bool stat_ok, mode_t mode, enum filetype type)
 {
   char c;
 
@@ -3826,7 +3828,13 @@ print_type_indicator (bool stat_ok, mode
       else
        c = 0;
     }
+  return c;
+}
 
+static void
+print_type_indicator (bool stat_ok, mode_t mode, enum filetype type)
+{
+  char c = get_type_indicator (stat_ok, mode, type);
   if (c)
     DIRED_PUTCHAR (c);
 }
@@ -3950,16 +3958,8 @@ length_of_file_name_and_frills (const st
 
   if (indicator_style != none)
     {
-      mode_t mode = f->stat.st_mode;
-
-      len += (S_ISREG (mode)
-             ? (indicator_style == classify && (mode & S_IXUGO))
-             : (S_ISDIR (mode)
-                || (indicator_style != slash
-                    && (S_ISLNK (mode)
-                        || S_ISFIFO (mode)
-                        || S_ISSOCK (mode)
-                        || S_ISDOOR (mode)))));
+      char c = get_type_indicator (f->stat_ok, f->stat.st_mode, f->filetype);
+      len += (c != 0);
     }
 
   return len;