Hi David,

Thanks for the report and patch.

On 2017-10-17, David Phillips <[email protected]> wrote:
> chdir()ing into a directory with +r-x fails, so we should manually use the
> directory name as a prefix rather than chdir()ing into it.

I wonder if we could make use of directory file descriptors and the
*at functions instead of building up a prefix.

> Also adds new parameters to mkent for the prefix, and for dictating whether
> or not a permission denied error when stat()ing shall be fatal or not. This
> allows errors stat()ing children of `foo` to be reported and non-fatal,
> while
> a permission denied error on `foo` itself would be fatal.

Can you explain the reasoning behind this? Is this behavior
standardized, or consistent with other ls(1) implementations?

> ---
>  ls.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/ls.c b/ls.c
> index b716aba..c01a277 100644
> --- a/ls.c
> +++ b/ls.c
> @@ -10,6 +10,7 @@
>  #include <string.h>
>  #include <time.h>
>  #include <unistd.h>
> +#include <errno.h>
>
>  #include "utf.h"
>  #include "util.h"
> @@ -58,15 +59,29 @@ static int showdirs;
>  static void ls(const char *, const struct entry *, int);
>
>  static void
> -mkent(struct entry *ent, char *path, int dostat, int follow)
> +mkent(struct entry *ent, char *prefix, char *name, int dostat, int follow,
> +      int fatal)
>  {
>       struct stat st;
> +     char path[PATH_MAX];
>
> -     ent->name = path;
> +     ent->name = name;
>       if (!dostat)
>               return;
> -     if ((follow ? stat : lstat)(path, &st) < 0)
> -             eprintf("%s %s:", follow ? "stat" : "lstat", path);
> +
> +     if (prefix != NULL)
> +             snprintf(path, PATH_MAX, "%s/%s", prefix, name);

You need to check whether this returned an error, or the result was truncated.

> +     else
> +             strncpy(path, name, PATH_MAX);

strncpy is almost never what you want. It does not null-terminate the
string if it doesn't fit in the destination, and if it is shorter than
PATH_MAX, the remaining buffer will be filled with zeros.

> +
> +     if ((follow ? stat : lstat)(path, &st) < 0) {
> +             if (errno == EACCES && !fatal) {
> +                     weprintf("%s %s:", follow ? "stat" : "lstat", path);
> +                     return;
> +             } else {
> +                     eprintf("%s %s:", follow ? "stat" : "lstat", path);
> +             }
> +     }
>       ent->mode  = st.st_mode;
>       ent->nlink = st.st_nlink;
>       ent->uid   = st.st_uid;
> @@ -252,8 +267,6 @@ lsdir(const char *path, const struct entry *dir)
>               weprintf("opendir %s%s:", path, dir->name);
>               return;
>       }
> -     if (chdir(dir->name) < 0)
> -             eprintf("chdir %s:", dir->name);
>
>       while ((d = readdir(dp))) {
>               if (d->d_name[0] == '.' && !aflag && !Aflag)
> @@ -264,8 +277,8 @@ lsdir(const char *path, const struct entry *dir)
>                               continue;
>
>               ents = ereallocarray(ents, ++n, sizeof(*ents));
> -             mkent(&ents[n - 1], estrdup(d->d_name), Fflag || iflag ||
> -                 lflag || pflag || Rflag || sort, Lflag);
> +             mkent(&ents[n - 1], dir->name, estrdup(d->d_name), Fflag || 
> iflag ||
> +                 lflag || pflag || Rflag || sort, Lflag, 0);
>       }
>
>       closedir(dp);
> @@ -446,7 +459,7 @@ main(int argc, char *argv[])
>       case 0: /* fallthrough */
>               *--argv = ".", ++argc;
>       case 1:
> -             mkent(&ent, argv[0], 1, Hflag || Lflag);
> +             mkent(&ent, NULL, argv[0], 1, Hflag || Lflag, 1);
>               ls("", &ent, (!dflag && S_ISDIR(ent.mode)) ||
>                   (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
>                    !(dflag || Fflag || lflag)));
> @@ -454,7 +467,7 @@ main(int argc, char *argv[])
>               break;
>       default:
>               for (i = ds = fs = 0, fents = dents = NULL; i < argc; ++i) {
> -                     mkent(&ent, argv[i], 1, Hflag || Lflag);
> +                     mkent(&ent, NULL, argv[i], 1, Hflag || Lflag, 1);
>
>                       if ((!dflag && S_ISDIR(ent.mode)) ||
>                           (S_ISLNK(ent.mode) && S_ISDIR(ent.tmode) &&
> --
> 2.14.2
>
>

Reply via email to