Subject: Re: [PATCH] When exec'ing sub-commands, fall back on execvp (thePATH)

From: "Scott R Parish" <[email-masked]>
Date: Sat, 20 Oct 2007 03:12:17 -0500 (CDT)

Yeah, that seems to work fine. The theoretical drawback to this approach
is that it could possibly effect the order in which the paths are tried.
For instance, if a user did "export GIT_EXEC_PATH=", then the
builtin_exec_path wouldn't be tried before the PATH. (i doubt that it
would be a problem, but thought i should note it)

sRp

----- Original Message -----
Subject: Re: [PATCH] When exec'ing sub-commands, fall back on execvp
(thePATH)
Date: Sat, October 20, 2007 0:30
From: "Johannes Schindelin" <[email-masked]>

> Hi,
>
> On Fri, 19 Oct 2007, Scott Parish wrote:
>
> > diff --git a/exec_cmd.c b/exec_cmd.c
> > index 9b74ed2..674c9f3 100644
> > --- a/exec_cmd.c
> > +++ b/exec_cmd.c
> > @@ -34,15 +34,15 @@ int execv_git_cmd(const char **argv)
> >  {
> >          char git_command[PATH_MAX + 1];
> >          int i;
> > +        int rc;
> >          const char *paths[] = { current_exec_path,
> >                                  getenv(EXEC_PATH_ENVIRONMENT),
> >                                  builtin_exec_path };
> > +        const char *tmp;
> > +        size_t len;
> >
> >          for (i = 0; i < ARRAY_SIZE(paths); ++i) {
> > -                size_t len;
> > -                int rc;
> >                  const char *exec_dir = paths[i];
> > -                const char *tmp;
> >
> >                  if (!exec_dir || !*exec_dir) continue;
> >
> > @@ -106,8 +106,26 @@ int execv_git_cmd(const char **argv)
> >
> >                  argv[0] = tmp;
> >          }
> > -        return -1;
> >
> > +        rc = snprintf(git_command, sizeof(git_command), "git-%s",
argv[0]);
> > +        if (rc < 0 || rc >= sizeof(git_command) - len) {
> > +                fprintf(stderr, "git: command name given is too
long.\n");
> > +                return -1;
> > +        }
> > +
> > +        tmp = argv[0];
> > +        argv[0] = git_command;
> > +
> > +        trace_argv_printf(argv, -1, "trace: exec:");
> > +
> > +        /* execve() can only ever return if it fails */
> > +        execvp(git_command, (char **)argv);
> > +
> > +        trace_printf("trace: exec failed: %s\n", strerror(errno));
> > +
> > +        argv[0] = tmp;
> > +
> > +        return -1;
> >  }
>
> I am not sure that this is elegant enough: Something like this (completely
> untested) might be better:
>
> diff --git a/exec_cmd.c b/exec_cmd.c
> index 9b74ed2..c928f37 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)
>          int i;
>          const char *paths[] = { current_exec_path,
>                                  getenv(EXEC_PATH_ENVIRONMENT),
> -                                builtin_exec_path };
> +                                builtin_exec_path,
> +                                "" };
>
>          for (i = 0; i < ARRAY_SIZE(paths); ++i) {
>                  size_t len;
> @@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)
>                  const char *exec_dir = paths[i];
>                  const char *tmp;
>
> -                if (!exec_dir || !*exec_dir) continue;
> +                if (!exec_dir) continue;
>
> -                if (*exec_dir != '/') {
> +                if (!*exec_dir)
> +                        /* try PATH */
> +                        *git_command = '\0';
> +                else if (*exec_dir != '/') {
>                          if (!getcwd(git_command, sizeof(git_command))) {
>                                  fprintf(stderr, "git: cannot determine "
>                                          "current directory: %s\n",
> @@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)
>
>                  len = strlen(git_command);
>                  rc = snprintf(git_command + len, sizeof(git_command) -
len,
> -                              "/git-%s", argv[0]);
> +                              "%sgit-%s", *exec_dir ? "/" : "", argv[0]);
>                  if (rc < 0 || rc >= sizeof(git_command) - len) {
>                          fprintf(stderr,
>                                  "git: command name given is too long.\n");
>
> Ciao,
> Dscho
>
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to [email-masked]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>