Hi Crystal,
Crystal Kolipe wrote on Tue, Oct 03, 2023 at 06:47:42PM -0300:
> Two display format bugs seems to have crept in to ls due to the
> addition of scaled human readable values and large minor numbers.
I think you are right that the current situation isn't good.
Thank you for bringing this up.
In general, ls(1) strives to dynamically determine the required
column width. It already does that for the file size column.
Given that device numbers use the same column, i think the solution
that is cleanest, most robust, most aesthetically pleasing, least
wasteful with respect to space, and easiest to maintain is simply
doing the same for device numbers, see the patch below.
Two remarks:
* The reason for keeping the local variable "bcfile" is that
(0, 0) represents a valid device (console). There is no
good reason why it should be in the DISPLAY struct, though.
* The line "d.s_major = d.s_minor = 3;" is likely redundant because
with bcfile == 0, nothing is supposed to use these fields.
While local variables should only be initialized when needed, i
think defensive programming recommends keeping data that is used
by (even internal) APIs initialized to reasonable values rather
than having random memory content lying around. That reduces
the risk of future code changes in *other* modules accidentally
accessing random data, if developers get confused regarding
which invariants the internal API guarantees or requires.
OK?
Ingo
$ cd /dev
$ ls -l MAKEDEV *vid*
-r-xr-xr-x 1 root wheel 12254 Oct 3 07:07 MAKEDEV
lrwxr-xr-x 1 root wheel 6 Aug 29 2016 video -> video0
crw------- 1 root wheel 44, 0 Oct 3 13:34 video0
crw------- 1 root wheel 44, 1 Oct 3 13:34 video1
$ ls -lh MAKEDEV *vid*
-r-xr-xr-x 1 root wheel 12.0K Oct 3 07:07 MAKEDEV
lrwxr-xr-x 1 root wheel 6B Aug 29 2016 video -> video0
crw------- 1 root wheel 44, 0 Oct 3 13:34 video0
crw------- 1 root wheel 44, 1 Oct 3 13:34 video1
$ ls -l null
crw-rw-rw- 1 root wheel 2, 2 Oct 5 00:32 null
$ ls -l cua00
crw-rw---- 1 root dialer 8, 128 Oct 3 13:34 cua00
Index: ls.c
===================================================================
RCS file: /cvs/src/bin/ls/ls.c,v
retrieving revision 1.54
diff -u -p -r1.54 ls.c
--- ls.c 7 Oct 2020 21:03:09 -0000 1.54
+++ ls.c 5 Oct 2023 14:47:37 -0000
@@ -436,6 +436,7 @@ display(FTSENT *p, FTSENT *list)
unsigned long long btotal;
blkcnt_t maxblock;
ino_t maxinode;
+ unsigned int maxmajor, maxminor;
int bcfile, flen, glen, ulen, maxflags, maxgroup, maxuser, maxlen;
int entries, needstats;
int width;
@@ -449,6 +450,7 @@ display(FTSENT *p, FTSENT *list)
btotal = maxblock = maxinode = maxlen = maxnlink = 0;
bcfile = 0;
maxuser = maxgroup = maxflags = 0;
+ maxmajor = maxminor = 0;
maxsize = 0;
for (cur = list, entries = 0; cur != NULL; cur = cur->fts_link) {
if (cur->fts_info == FTS_ERR || cur->fts_info == FTS_NS) {
@@ -523,9 +525,13 @@ display(FTSENT *p, FTSENT *list)
(void)strlcpy(np->group, group, glen + 1);
if (S_ISCHR(sp->st_mode) ||
- S_ISBLK(sp->st_mode))
+ S_ISBLK(sp->st_mode)) {
bcfile = 1;
-
+ if (maxmajor < major(sp->st_rdev))
+ maxmajor = major(sp->st_rdev);
+ if (maxminor < minor(sp->st_rdev))
+ maxminor = minor(sp->st_rdev);
+ }
if (f_flags) {
np->flags = &np->data[ulen + 1 + glen +
1];
(void)strlcpy(np->flags, flags, flen +
1);
@@ -551,7 +557,6 @@ display(FTSENT *p, FTSENT *list)
d.entries = entries;
d.maxlen = maxlen;
if (needstats) {
- d.bcfile = bcfile;
d.btotal = btotal;
(void)snprintf(buf, sizeof(buf), "%llu",
(unsigned long long)maxblock);
@@ -570,6 +575,17 @@ display(FTSENT *p, FTSENT *list)
d.s_size = strlen(buf);
} else
d.s_size = FMT_SCALED_STRSIZE-2; /* no - or '\0' */
+ d.s_major = d.s_minor = 3;
+ if (bcfile) {
+ (void)snprintf(buf, sizeof(buf), "%u", maxmajor);
+ d.s_major = strlen(buf);
+ (void)snprintf(buf, sizeof(buf), "%u", maxminor);
+ d.s_minor = strlen(buf);
+ if (d.s_size <= d.s_major + 2 + d.s_minor)
+ d.s_size = d.s_major + 2 + d.s_minor;
+ else
+ d.s_major = d.s_size - 2 - d.s_minor;
+ }
d.s_user = maxuser;
}
Index: ls.h
===================================================================
RCS file: /cvs/src/bin/ls/ls.h,v
retrieving revision 1.9
diff -u -p -r1.9 ls.h
--- ls.h 30 May 2013 16:34:32 -0000 1.9
+++ ls.h 5 Oct 2023 14:47:37 -0000
@@ -55,7 +55,6 @@ extern int f_typedir; /* add type chara
typedef struct {
FTSENT *list;
unsigned long long btotal;
- int bcfile;
int entries;
int maxlen;
int s_block;
@@ -64,6 +63,8 @@ typedef struct {
int s_inode;
int s_nlink;
int s_size;
+ int s_major;
+ int s_minor;
int s_user;
} DISPLAY;
Index: print.c
===================================================================
RCS file: /cvs/src/bin/ls/print.c,v
retrieving revision 1.39
diff -u -p -r1.39 print.c
--- print.c 7 Oct 2020 21:03:09 -0000 1.39
+++ print.c 5 Oct 2023 14:47:37 -0000
@@ -110,12 +110,9 @@ printlong(DISPLAY *dp)
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 ",
- 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);
+ (void)printf("%*u, %*u ",
+ dp->s_major, major(sp->st_rdev),
+ dp->s_minor, minor(sp->st_rdev));
else
printsize(dp->s_size, sp->st_size);
if (f_accesstime)
