Re: [hackers] [sbase] [PATCH] which: absolute path not handled

From: Pieter Kockx <pieterkockx_AT_gmail.com>
Date: Wed, 11 Oct 2017 22:21:20 +0000

_AT_Evan
I agree, 'which' does nothing the shell can't do better.
The utility is not standardized and implementations vary accordingly:
https://man.openbsd.org/which.1
https://linux.die.net/man/1/which
https://git.busybox.net/busybox/tree/debianutils/which.c

_AT_Michael
If removing the utility is not an option this is the way to go.

I used fstatat(AT_FDCWD, ...) instead of stat to deal with the case of
relative paths.
I generalized the error message to "external command":
"external" to differentiate from builtin shell commands
"command" since which essentially tries to mimic a typical shell

Also included is a patch to correct some whitespacing in the same file
(spaces where tabs should have been used).

-- Pieter


On Wed, 11 Oct 2017 at 05:06 Michael Forney <mforney_AT_mforney.org> wrote:

> On 10/10/17, Pieter Kockx <pieterkockx_AT_gmail.com> wrote:
> > Hello hackers
> >
> > $ which /usr/bin/env
> > /usr/bin//usr/bin/env
> >
> > which should probably be idempotent when applied to its own output as
> > in which $(which $(which env)).
> >
> > The underlying reason is that fstatat ignores dirfd if the the given
> > filename is an absolute path.
> > The following fix is possible:
> >
> > if ((dirfd = open(p, O_RDONLY, 0)) >= 0) {
> > if (!fstatat(dirfd, name, &st, 0) &&
> > S_ISREG(st.st_mode) &&
> > !faccessat(dirfd, name, X_OK, 0)) {
> > found = 1;
> > + if (name[0] == '/') {
> > + puts(name);
> > + close(dirfd);
> > + break;
> > + }
> > fputs(p, stdout);
> > if (i && ptr[i - 1] != '/')
> > fputc('/', stdout);
> > puts(name);
> > if (!aflag) {
> > close(dirfd);
> > break;
> > }
> > }
> > }
> >
> > This fix is not perfect though, since which will (still) return
> > a different result if PATH contains no valid directories:
>
> I think the right thing to do here is to try to stat the file if it
> contains a slash, otherwise search through PATH.
>
> > $ PATH=/foo which /usr/bin/env
> > Error
> > $ PATH=/ which /usr/bin/env
> > /usr/bin/env
> >
> > Patch incoming.
> >
> > Something else, utilities like which are often used used to check for the
> > existence of a file, so printing an error message seems undesirable.
> >
> > Thoughts?
> >
> > -- Pieter
> >
>
>

Received on Thu Oct 12 2017 - 00:21:20 CEST

This archive was generated by hypermail 2.3.0 : Thu Oct 12 2017 - 00:24:20 CEST