Re: [PATCH 2/2] git: continue alias lookup on EACCES errors

From: Frans Klaver <fransklaver@gmail.com>
Date: Mar 29, 2012, 11:31 UTC
Message-ID: <CAH6sp9OcWUks_n1bD2n1KbePHeUX+FSY0+wLFu+zPik1Pwj3Aw@mail.gmail.com>
In-Reply-To: <20120328215704.GB10795@sigill.intra.peff.net>

On Wed, Mar 28, 2012 at 11:57 PM, Jeff King <peff@peff.net> wrote:

Show 33 quoted lines

> +static int exists_in_PATH(const char *file)
> +{
> +       const char *p = getenv("PATH");
> +       struct strbuf buf = STRBUF_INIT;
> +
> +       if (!p || !*p)
> +               return 0;
> +
> +       while (1) {
> +               const char *end = strchrnul(p, ':');
> +
> +               strbuf_reset(&buf);
> +
> +               /* POSIX specifies an empty entry as the current directory. */
> +               if (end != p) {
> +                       strbuf_add(&buf, p, end - p);
> +                       strbuf_addch(&buf, '/');
> +               }
> +               strbuf_addstr(&buf, file);
> +
> +               if (!access(buf.buf, F_OK)) {
> +                       strbuf_release(&buf);
> +                       return 1;
> +               }
> +
> +               if (!*end)
> +                       break;
> +               p = end + 1;
> +       }
> +
> +       strbuf_release(&buf);
> +       return 0;
> +}

I expect that if more post-mortem checking is done, this function is going to need a sibling that provides you with the first found entry in PATH, so you can do more checks on it.

Show 5 quoted lines

> +
> +int sane_execvp(const char *file, char * const argv[])
> +{
> +       if (!execvp(file, argv))
> +               return 0;

> +       if (errno == EACCES && !strchr(file, '/'))
> +               errno = exists_in_PATH(file) ? EACCES : ENOENT;
> +       return -1;
> +}

One of the things I ran into while working on [1] is that quite some errors that are produced can also be caused by the interpreter. This does cover most of the itch I had earlier. I will still want to have the interpreter check [2] in though; errno can for example also be set to ENOENT if the interpreter or a required library isn't available. In that case you wouldn't want to continue to the aliases, right?