Merge branch 'tr/fd-gotcha-fixes'
authorJunio C Hamano <gitster@pobox.com>
Mon, 22 Jul 2013 18:23:12 +0000 (11:23 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 22 Jul 2013 18:23:13 +0000 (11:23 -0700)
Two places we did not check return value (expected to be a file
descriptor) correctly.

* tr/fd-gotcha-fixes:
run-command: dup_devnull(): guard against syscalls failing
git_mkstemps: correctly test return value of open()

1  2 
run-command.c
wrapper.c
diff --combined run-command.c
index aece872e331caa28bf515c98f5cceb27d8414dff,afc573ed41061f7c704209de2d229e7a6a900165..1b7f88eeb1f1971f1568d5991978192c2d00645e
@@@ -72,11 -72,14 +72,14 @@@ static inline void close_pair(int fd[2]
        close(fd[1]);
  }
  
 -#ifndef WIN32
 +#ifndef GIT_WINDOWS_NATIVE
  static inline void dup_devnull(int to)
  {
        int fd = open("/dev/null", O_RDWR);
-       dup2(fd, to);
+       if (fd < 0)
+               die_errno(_("open /dev/null failed"));
+       if (dup2(fd, to) < 0)
+               die_errno(_("dup2(%d,%d) failed"), fd, to);
        close(fd);
  }
  #endif
@@@ -159,7 -162,7 +162,7 @@@ static const char **prepare_shell_cmd(c
                die("BUG: shell command is empty");
  
        if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
 -#ifndef WIN32
 +#ifndef GIT_WINDOWS_NATIVE
                nargv[nargc++] = SHELL_PATH;
  #else
                nargv[nargc++] = "sh";
        return nargv;
  }
  
 -#ifndef WIN32
 +#ifndef GIT_WINDOWS_NATIVE
  static int execv_shell_cmd(const char **argv)
  {
        const char **nargv = prepare_shell_cmd(argv);
  }
  #endif
  
 -#ifndef WIN32
 +#ifndef GIT_WINDOWS_NATIVE
  static int child_err = 2;
  static int child_notifier = -1;
  
@@@ -273,8 -276,7 +276,8 @@@ int start_command(struct child_process 
  {
        int need_in, need_out, need_err;
        int fdin[2], fdout[2], fderr[2];
 -      int failed_errno = failed_errno;
 +      int failed_errno;
 +      char *str;
  
        /*
         * In case of errors we must keep the promise to close FDs
                        failed_errno = errno;
                        if (cmd->out > 0)
                                close(cmd->out);
 +                      str = "standard input";
                        goto fail_pipe;
                }
                cmd->in = fdin[1];
                                close_pair(fdin);
                        else if (cmd->in)
                                close(cmd->in);
 +                      str = "standard output";
                        goto fail_pipe;
                }
                cmd->out = fdout[0];
                                close_pair(fdout);
                        else if (cmd->out)
                                close(cmd->out);
 +                      str = "standard error";
  fail_pipe:
 -                      error("cannot create pipe for %s: %s",
 -                              cmd->argv[0], strerror(failed_errno));
 +                      error("cannot create %s pipe for %s: %s",
 +                              str, cmd->argv[0], strerror(failed_errno));
                        errno = failed_errno;
                        return -1;
                }
        trace_argv_printf(cmd->argv, "trace: run_command:");
        fflush(NULL);
  
 -#ifndef WIN32
 +#ifndef GIT_WINDOWS_NATIVE
  {
        int notify_pipe[2];
        if (pipe(notify_pipe))
                notify_pipe[0] = notify_pipe[1] = -1;
  
        cmd->pid = fork();
 +      failed_errno = errno;
        if (!cmd->pid) {
                /*
                 * Redirect the channel to write syscall error messages to
        }
        if (cmd->pid < 0)
                error("cannot fork() for %s: %s", cmd->argv[0],
 -                      strerror(failed_errno = errno));
 +                      strerror(errno));
        else if (cmd->clean_on_exit)
                mark_child_for_cleanup(cmd->pid);
  
@@@ -588,7 -586,6 +591,7 @@@ int run_command_v_opt_cd_env(const cha
  static pthread_t main_thread;
  static int main_thread_set;
  static pthread_key_t async_key;
 +static pthread_key_t async_die_counter;
  
  static void *run_thread(void *data)
  {
@@@ -615,14 -612,6 +618,14 @@@ static NORETURN void die_async(const ch
  
        exit(128);
  }
 +
 +static int async_die_is_recursing(void)
 +{
 +      void *ret = pthread_getspecific(async_die_counter);
 +      pthread_setspecific(async_die_counter, (void *)1);
 +      return ret != NULL;
 +}
 +
  #endif
  
  int start_async(struct async *async)
                main_thread_set = 1;
                main_thread = pthread_self();
                pthread_key_create(&async_key, NULL);
 +              pthread_key_create(&async_die_counter, NULL);
                set_die_routine(die_async);
 +              set_die_is_recursing_routine(async_die_is_recursing);
        }
  
        if (proc_in >= 0)
@@@ -751,15 -738,6 +754,15 @@@ int finish_async(struct async *async
  #endif
  }
  
 +char *find_hook(const char *name)
 +{
 +      char *path = git_path("hooks/%s", name);
 +      if (access(path, X_OK) < 0)
 +              path = NULL;
 +
 +      return path;
 +}
 +
  int run_hook(const char *index_file, const char *name, ...)
  {
        struct child_process hook;
        va_list args;
        int ret;
  
 -      if (access(git_path("hooks/%s", name), X_OK) < 0)
 +      p = find_hook(name);
 +      if (!p)
                return 0;
  
 +      argv_array_push(&argv, p);
 +
        va_start(args, name);
 -      argv_array_push(&argv, git_path("hooks/%s", name));
        while ((p = va_arg(args, const char *)))
                argv_array_push(&argv, p);
        va_end(args);
diff --combined wrapper.c
index dd7ecbb115edd979f657e2e209126d364e6ccfac,094b2ab9836a4d1d13d432982c6b2582ee3cb13a..6a015de5f0564e1ccc9c8cffca891f4ddf4a3ac3
+++ b/wrapper.c
@@@ -322,7 -322,7 +322,7 @@@ int git_mkstemps_mode(char *pattern, in
                template[5] = letters[v % num_letters]; v /= num_letters;
  
                fd = open(pattern, O_CREAT | O_EXCL | O_RDWR, mode);
-               if (fd > 0)
+               if (fd >= 0)
                        return fd;
                /*
                 * Fatal error (EPERM, ENOSPC etc).
@@@ -408,24 -408,18 +408,24 @@@ void warn_on_inaccessible(const char *p
        warning(_("unable to access '%s': %s"), path, strerror(errno));
  }
  
 -int access_or_warn(const char *path, int mode)
 +static int access_error_is_ok(int err, unsigned flag)
 +{
 +      return err == ENOENT || err == ENOTDIR ||
 +              ((flag & ACCESS_EACCES_OK) && err == EACCES);
 +}
 +
 +int access_or_warn(const char *path, int mode, unsigned flag)
  {
        int ret = access(path, mode);
 -      if (ret && errno != ENOENT && errno != ENOTDIR)
 +      if (ret && !access_error_is_ok(errno, flag))
                warn_on_inaccessible(path);
        return ret;
  }
  
 -int access_or_die(const char *path, int mode)
 +int access_or_die(const char *path, int mode, unsigned flag)
  {
        int ret = access(path, mode);
 -      if (ret && errno != ENOENT && errno != ENOTDIR)
 +      if (ret && !access_error_is_ok(errno, flag))
                die_errno(_("unable to access '%s'"), path);
        return ret;
  }