Two display format bugs seems to have crept in to ls due to the addition of
scaled human readable values and large minor numbers.

A provisional patch is attached, but an aesthetic decision about column padding
needs to be made, (8 vs 13 columns to handle large minor numbers cleanly, see
further below).

Bug 1:
When using ls -lh, (long format, human-readable sizes), if the list of files
includes a mixture of regular files and device nodes then the sizes of the
regular files are displayed in bytes instead of using multipliers.

# ls -lh
total 672
-rw-r--r--  1 root  wheel     1000000 Oct  3 16:41 a
-rw-r--r--  1 root  wheel     10000000 Oct  3 16:41 b
-rw-r--r--  1 root  wheel     100000000 Oct  3 16:41 c
-rw-r--r--  1 root  wheel     1000000000 Oct  3 16:41 d
crw-r--r--  1 root  wheel    0,   0 Oct  3 16:42 dev1
crw-r--r--  1 root  wheel  255, 255 Oct  3 16:42 dev2
crw-r--r--  1 root  wheel  255, 65535 Oct  3 16:42 dev3
crw-r--r--  1 root  wheel  255, 16777214 Oct  3 16:43 dev4
-rw-r--r--  1 root  wheel     10000000000 Oct  3 16:41 e
-rw-r--r--  1 root  wheel     100000000000 Oct  3 16:41 f
-rw-r--r--  1 root  wheel     1000000000000 Oct  3 16:41 g


Bug 2:
When using ls -l, (long format only), columns are padded with too many
additional spaces when values exceed 8 characters, due to a printf format
string argument turning negative.

Compare the following two outputs:

-rw-r--r--  1 root  wheel   1000000 Oct  3 16:41 a
-rw-r--r--  1 root  wheel  10000000 Oct  3 16:41 b
crw-r--r--  1 root  wheel    0,   0 Oct  3 16:42 dev1
crw-r--r--  1 root  wheel  255, 255 Oct  3 16:42 dev2
-rw-r--r--  1 root  wheel         0 Oct  3 17:39 h
-rw-r--r--  1 root  wheel        10 Oct  3 17:39 i
-rw-r--r--  1 root  wheel       100 Oct  3 17:39 j
-rw-r--r--  1 root  wheel      1000 Oct  3 17:39 k
-rw-r--r--  1 root  wheel     10000 Oct  3 17:39 l
-rw-r--r--  1 root  wheel    100000 Oct  3 17:39 m

-rw-r--r--  1 root  wheel         1000000 Oct  3 16:41 a
-rw-r--r--  1 root  wheel        10000000 Oct  3 16:41 b
-rw-r--r--  1 root  wheel       100000000 Oct  3 16:41 c
-rw-r--r--  1 root  wheel      1000000000 Oct  3 16:41 d
crw-r--r--  1 root  wheel    0,   0 Oct  3 16:42 dev1
crw-r--r--  1 root  wheel  255, 255 Oct  3 16:42 dev2
-rw-r--r--  1 root  wheel     10000000000 Oct  3 16:41 e
-rw-r--r--  1 root  wheel               0 Oct  3 17:39 h
-rw-r--r--  1 root  wheel              10 Oct  3 17:39 i
-rw-r--r--  1 root  wheel             100 Oct  3 17:39 j
-rw-r--r--  1 root  wheel            1000 Oct  3 17:39 k
-rw-r--r--  1 root  wheel           10000 Oct  3 17:39 l
-rw-r--r--  1 root  wheel          100000 Oct  3 17:39 m

Observe that in the first output, file b has the largest size and the
leftmost digit aligns with the leftmost digit of 255, 255.

however, in the second output, file e is the largest and has three space
characters too many inserted in front.

This second bug is caused by the following code in printlong() in print.c:

                        (void)printf("%*s%*lld ",
                            8 - dp->s_size, "", dp->s_size,
                            (long long)sp->st_size);

The intention is clearly to align the last digits of the output when a device
file is included in the output, (dp->bcfile tests true).

Device files are printed with "%3u, %3u ", so would traditionally have been
limited to 8 characters plus the trailing space.  Large minor numbers have
obviously changed this assumption and so this field can now expand to be
larger.

However, even without large minors we have a problem because dp->s_size can
be > 8, and in that case we are passing a _negative_ value to printf for the
field width.  Since printf interprets negative values here as their positive
equivalent, we get double spaces once dp->s_size > 8.

This presents an aesthetic decision, because to ensure a large enough column
width for the largest possible minor numbers we need to use:

printf ("%3u, %8u ")

... despite most people not having minor numbers > 255 in /dev, so the display
could be considered a bit ugly.

Possibly the simplest fix for this second issue would be just to expand the
minor number field:

--- print.c.dist        Mon Oct  2 21:03:01 2023
+++ print.c     Tue Oct  3 18:22:54 2023
@@ -110,11 +110,11 @@
                if (f_flags)
                        (void)printf("%-*s ", dp->s_flags, np->flags);
                if (S_ISCHR(sp->st_mode) || S_ISBLK(sp->st_mode))
-                       (void)printf("%3u, %3u ",
+                       (void)printf("%3u, %8u ",
                            major(sp->st_rdev), minor(sp->st_rdev));
                else if (dp->bcfile)
                        (void)printf("%*s%*lld ",
-                           8 - dp->s_size, "", dp->s_size,
+                           13 - dp->s_size, "", dp->s_size,
                            (long long)sp->st_size);
                else
                        printsize(dp->s_size, sp->st_size);

This doesn't fix the issue of double spacing, since 13 - dp->s_size could
still go negative, but it makes it much less likely to happen and is a
trivial change.

However, just calling printsize() with a fixed value of 13 for the field width
in the (dp->bcfile) case seems to produce acceptable output:

# ls -l
-rw-r--r--  1 root  wheel        1000000 Oct  3 16:41 a
-rw-r--r--  1 root  wheel       10000000 Oct  3 16:41 b
-rw-r--r--  1 root  wheel      100000000 Oct  3 16:41 c
-rw-r--r--  1 root  wheel     1000000000 Oct  3 16:41 d
crw-r--r--  1 root  wheel    0,        0 Oct  3 16:42 dev1
crw-r--r--  1 root  wheel  255,      255 Oct  3 16:42 dev2
crw-r--r--  1 root  wheel  255, 16777214 Oct  3 16:43 dev4
-rw-r--r--  1 root  wheel    10000000000 Oct  3 16:41 e
-rw-r--r--  1 root  wheel   100000000000 Oct  3 16:41 f
-rw-r--r--  1 root  wheel  1000000000000 Oct  3 16:41 g
-rw-r--r--  1 root  wheel              0 Oct  3 17:39 h
-rw-r--r--  1 root  wheel             10 Oct  3 17:39 i
-rw-r--r--  1 root  wheel            100 Oct  3 17:39 j
-rw-r--r--  1 root  wheel           1000 Oct  3 17:39 k
-rw-r--r--  1 root  wheel          10000 Oct  3 17:39 l
-rw-r--r--  1 root  wheel         100000 Oct  3 17:39 m

# ls -lh
-rw-r--r--  1 root  wheel           977K Oct  3 16:41 a
-rw-r--r--  1 root  wheel           9.5M Oct  3 16:41 b
-rw-r--r--  1 root  wheel          95.4M Oct  3 16:41 c
-rw-r--r--  1 root  wheel           954M Oct  3 16:41 d
crw-r--r--  1 root  wheel    0,        0 Oct  3 16:42 dev1
crw-r--r--  1 root  wheel  255,      255 Oct  3 16:42 dev2
crw-r--r--  1 root  wheel  255, 16777214 Oct  3 16:43 dev4
-rw-r--r--  1 root  wheel           9.3G Oct  3 16:41 e
-rw-r--r--  1 root  wheel          93.1G Oct  3 16:41 f
-rw-r--r--  1 root  wheel           931G Oct  3 16:41 g
-rw-r--r--  1 root  wheel             0B Oct  3 17:39 h
-rw-r--r--  1 root  wheel            10B Oct  3 17:39 i
-rw-r--r--  1 root  wheel           100B Oct  3 17:39 j
-rw-r--r--  1 root  wheel          1000B Oct  3 17:39 k
-rw-r--r--  1 root  wheel           9.8K Oct  3 17:39 l
-rw-r--r--  1 root  wheel          97.7K Oct  3 17:39 m

This is what the following patch does:

--- print.c.dist        Mon Oct  2 21:03:01 2023
+++ print.c     Tue Oct  3 18:38:46 2023
@@ -110,12 +110,10 @@
                if (f_flags)
                        (void)printf("%-*s ", dp->s_flags, np->flags);
                if (S_ISCHR(sp->st_mode) || S_ISBLK(sp->st_mode))
-                       (void)printf("%3u, %3u ",
+                       (void)printf("%3u, %8u ",
                            major(sp->st_rdev), minor(sp->st_rdev));
                else if (dp->bcfile)
-                       (void)printf("%*s%*lld ",
-                           8 - dp->s_size, "", dp->s_size,
-                           (long long)sp->st_size);
+                       printsize(13, sp->st_size);
                else
                        printsize(dp->s_size, sp->st_size);
                if (f_accesstime)

Reply via email to