On Tue, Sep 25, 2018 at 06:48:08PM -0700, Michael Forney wrote: > > Rather than keeping track of the full prefix for `mkent` and `output`, > what do you think about just tracking the directory fd, and using the > *at family of functions to work relative to that directory? I attached > a patch to show what I'm thinking. It looks like this might even > simplify some of the code. >
I just read your patch in full and it looks good to me. It seems to describe and exhibit the desired behaviour. One minor thing I would propose adding is changing the stat error message to display the full path rather than just the name of the file/directory that cannot be stat-ed. This clarifies the source of errors in the case of `ls -R`- style commands. Regarding the stat failure, please find a patch attached based on your patch which allows stat failure. Thanks, David
>From 856b408a098415d98e6c6711113c7d1df7a9dde0 Mon Sep 17 00:00:00 2001 From: David Phillips <[email protected]> Date: Tue, 2 Oct 2018 15:11:36 +1300 Subject: [PATCH] ls: allow fstatat failure, don't output filenames on fstatat failure This patch allows fstatat failure without stopping the ls operation entirely. Instead, a validity flag is added to `struct entry` so that output can list unknown file information as such. A return code is added to mkent so that callers may avoid outputting a filename if fstatat on that file fails. This avoids erroneously outputting a filename when we don't know if it actually exists. --- ls.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/ls.c b/ls.c index 28d797a..a629503 100644 --- a/ls.c +++ b/ls.c @@ -33,6 +33,7 @@ struct entry { dev_t dev; dev_t rdev; ino_t ino, tino; + int valid; }; static struct { @@ -65,16 +66,22 @@ static int showdirs; static void ls(int, const char *, const struct entry *, int); -static void +static int mkent(int dirfd, struct entry *ent, char *path, int dostat, int follow) { struct stat st; ent->name = path; if (!dostat) - return; - if (fstatat(dirfd, path, &st, follow ? 0 : AT_SYMLINK_NOFOLLOW) < 0) - eprintf("stat %s:", path); + return 0; + if (fstatat(dirfd, path, &st, follow ? 0 : AT_SYMLINK_NOFOLLOW) < 0) { + ent->valid = 0; + ret = 1; + weprintf("stat %s:", path); + return 1; + } else { + ent->valid = 1; + } ent->mode = st.st_mode; ent->nlink = st.st_nlink; ent->uid = st.st_uid; @@ -98,6 +105,7 @@ mkent(int dirfd, struct entry *ent, char *path, int dostat, int follow) ent->tmode = ent->tino = 0; } } + return 0; } static char * @@ -171,6 +179,13 @@ output(int dirfd, const struct entry *ent) else mode[0] = '?'; + if (!ent->valid) { + printf("%c????????? ? ? ? ? ? ", mode[0]); + printname(ent->name); + puts(indicator(ent->mode)); + return; + } + if (ent->mode & S_IRUSR) mode[1] = 'r'; if (ent->mode & S_IWUSR) mode[2] = 'w'; if (ent->mode & S_IXUSR) mode[3] = 'x'; @@ -271,8 +286,12 @@ lsdir(int dirfd, const char *path, const struct entry *dir) continue; ents = ereallocarray(ents, ++n, sizeof(*ents)); + ent = &ents[n - 1]; mkent(dirfd, &ents[n - 1], estrdup(d->d_name), Fflag || iflag || lflag || pflag || Rflag || sort, Lflag); + if (!ent->valid) { + ent->mode = DTTOIF(d->d_type); + } } if (!Uflag) @@ -445,15 +464,18 @@ main(int argc, char *argv[]) case 0: /* fallthrough */ *--argv = ".", ++argc; case 1: - mkent(AT_FDCWD, &ent, argv[0], 1, Hflag || Lflag); - ls(AT_FDCWD, "", &ent, (!dflag && S_ISDIR(ent.mode)) || - (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) && - !(dflag || Fflag || lflag))); + if (!mkent(AT_FDCWD, &ent, argv[0], 1, Hflag || Lflag)) { + ls(AT_FDCWD, "", &ent, (!dflag && S_ISDIR(ent.mode)) || + (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) && + !(dflag || Fflag || lflag))); + } break; default: for (i = ds = fs = 0, fents = dents = NULL; i < argc; ++i) { - mkent(AT_FDCWD, &ent, argv[i], 1, Hflag || Lflag); + if (mkent(AT_FDCWD, &ent, argv[i], 1, Hflag || Lflag)) { + continue; + } if ((!dflag && S_ISDIR(ent.mode)) || (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) && -- 2.18.0
