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

[Scott Parish <sRp@srparish.net>]

Actually, i didn't test it right, execve() were using the files in
my cwd. In addition to you patch, you'd need to use execvp() instead
of execve().

sRp

On Sat, Oct 20, 2007 at 03:12:17AM -0500, Scott R Parish wrote:

> 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" <Johannes.Schindelin@gmx.de>
> 
> > 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 majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> >
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Scott Parish
http://srparish.net/