Merge branch 'jk/a-thread-only-dies-once'
authorJunio C Hamano <gitster@pobox.com>
Fri, 19 Apr 2013 20:45:04 +0000 (13:45 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 19 Apr 2013 20:45:05 +0000 (13:45 -0700)
A regression fix for the logic to detect die() handler triggering
itself recursively.

* jk/a-thread-only-dies-once:
run-command: use thread-aware die_is_recursing routine
usage: allow pluggable die-recursion checks

1  2 
git-compat-util.h
run-command.c
usage.c
diff --combined git-compat-util.h
index cde442fb5f47cd13757e09ae435963bc061db083,6aee9df86893459999314554fd246c2f8f847128..e955bb5e8b3101cc8c753cf541beabf5cd037b39
@@@ -75,7 -75,7 +75,7 @@@
  # endif
  #elif !defined(__APPLE__) && !defined(__FreeBSD__) && !defined(__USLC__) && \
        !defined(_M_UNIX) && !defined(__sgi) && !defined(__DragonFly__) && \
 -      !defined(__TANDEM)
 +      !defined(__TANDEM) && !defined(__QNX__)
  #define _XOPEN_SOURCE 600 /* glibc2 and AIX 5.3L need 500, OpenBSD needs 600 for S_ISLNK() */
  #define _XOPEN_SOURCE_EXTENDED 1 /* AIX 5.3L needs this */
  #endif
@@@ -86,9 -86,6 +86,9 @@@
  #define _SGI_SOURCE 1
  
  #ifdef WIN32 /* Both MinGW and MSVC */
 +# if defined (_MSC_VER)
 +#  define _WIN32_WINNT 0x0502
 +# endif
  #define WIN32_LEAN_AND_MEAN  /* stops windows.h including winsock.h */
  #include <winsock2.h>
  #include <windows.h>
  #include <stdlib.h>
  #include <stdarg.h>
  #include <string.h>
 -#ifdef __TANDEM /* or HAVE_STRINGS_H or !NO_STRINGS_H? */
 +#ifdef HAVE_STRINGS_H
  #include <strings.h> /* for strcasecmp() */
  #endif
  #include <errno.h>
  #include <limits.h>
 +#ifdef NEEDS_SYS_PARAM_H
  #include <sys/param.h>
 +#endif
  #include <sys/types.h>
  #include <dirent.h>
  #include <sys/time.h>
  #include <time.h>
  #include <signal.h>
 +#ifndef USE_WILDMATCH
  #include <fnmatch.h>
 +#endif
  #include <assert.h>
  #include <regex.h>
  #include <utime.h>
  typedef long intptr_t;
  typedef unsigned long uintptr_t;
  #endif
 +int get_st_mode_bits(const char *path, int *mode);
  #if defined(__CYGWIN__)
  #undef _XOPEN_SOURCE
  #include <grp.h>
@@@ -297,17 -289,6 +297,17 @@@ extern char *gitbasename(char *)
  
  #include "compat/bswap.h"
  
 +#ifdef USE_WILDMATCH
 +#include "wildmatch.h"
 +#define FNM_PATHNAME WM_PATHNAME
 +#define FNM_CASEFOLD WM_CASEFOLD
 +#define FNM_NOMATCH  WM_NOMATCH
 +static inline int fnmatch(const char *pattern, const char *string, int flags)
 +{
 +      return wildmatch(pattern, string, flags, NULL);
 +}
 +#endif
 +
  /* General helper functions */
  extern void vreportf(const char *prefix, const char *err, va_list params);
  extern void vwritef(int fd, const char *prefix, const char *err, va_list params);
@@@ -318,19 -299,9 +318,20 @@@ extern NORETURN void die_errno(const ch
  extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
  extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));
  
 +/*
 + * Let callers be aware of the constant return value; this can help
 + * gcc with -Wuninitialized analysis. We restrict this trick to gcc, though,
 + * because some compilers may not support variadic macros. Since we're only
 + * trying to help gcc, anyway, it's OK; other compilers will fall back to
 + * using the function as usual.
 + */
 +#if defined(__GNUC__) && ! defined(__clang__)
 +#define error(...) (error(__VA_ARGS__), -1)
 +#endif
 +
  extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params));
  extern void set_error_routine(void (*routine)(const char *err, va_list params));
+ extern void set_die_is_recursing_routine(int (*routine)(void));
  
  extern int prefixcmp(const char *str, const char *prefix);
  extern int suffixcmp(const char *str, const char *suffix);
@@@ -436,6 -407,11 +437,6 @@@ extern uintmax_t gitstrtoumax(const cha
  extern intmax_t gitstrtoimax(const char *, char **, int);
  #endif
  
 -#ifdef NO_STRTOK_R
 -#define strtok_r gitstrtok_r
 -extern char *gitstrtok_r(char *s, const char *delim, char **save_ptr);
 -#endif
 -
  #ifdef NO_HSTRERROR
  #define hstrerror githstrerror
  extern const char *githstrerror(int herror);
@@@ -447,10 -423,6 +448,10 @@@ void *gitmemmem(const void *haystack, s
                  const void *needle, size_t needlelen);
  #endif
  
 +#ifdef NO_GETPAGESIZE
 +#define getpagesize() sysconf(_SC_PAGESIZE)
 +#endif
 +
  #ifdef FREAD_READS_DIRECTORIES
  #ifdef fopen
  #undef fopen
@@@ -551,19 -523,13 +552,19 @@@ extern const char tolower_trans_tbl[256
  #undef isupper
  #undef tolower
  #undef toupper
 -extern unsigned char sane_ctype[256];
 +#undef iscntrl
 +#undef ispunct
 +#undef isxdigit
 +
 +extern const unsigned char sane_ctype[256];
  #define GIT_SPACE 0x01
  #define GIT_DIGIT 0x02
  #define GIT_ALPHA 0x04
  #define GIT_GLOB_SPECIAL 0x08
  #define GIT_REGEX_SPECIAL 0x10
  #define GIT_PATHSPEC_MAGIC 0x20
 +#define GIT_CNTRL 0x40
 +#define GIT_PUNCT 0x80
  #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
  #define isascii(x) (((x) & ~0x7f) == 0)
  #define isspace(x) sane_istest(x,GIT_SPACE)
  #define isupper(x) sane_iscase(x, 0)
  #define is_glob_special(x) sane_istest(x,GIT_GLOB_SPECIAL)
  #define is_regex_special(x) sane_istest(x,GIT_GLOB_SPECIAL | GIT_REGEX_SPECIAL)
 +#define iscntrl(x) (sane_istest(x,GIT_CNTRL))
 +#define ispunct(x) sane_istest(x, GIT_PUNCT | GIT_REGEX_SPECIAL | \
 +              GIT_GLOB_SPECIAL | GIT_PATHSPEC_MAGIC)
 +#define isxdigit(x) (hexval_table[x] != -1)
  #define tolower(x) sane_case((unsigned char)(x), 0x20)
  #define toupper(x) sane_case((unsigned char)(x), 0)
  #define is_pathspec_magic(x) sane_istest(x,GIT_PATHSPEC_MAGIC)
diff --combined run-command.c
index 765c2ce0567ad0d75270283397d63eac3674c01a,87ddce02efc5920428450fa1addbd3568eec91dc..1b32a12a29b64fcc7d8b2401b00fb8a70ff81bad
@@@ -273,8 -273,7 +273,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;
                }
                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,6 -583,7 +588,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)
  {
@@@ -614,6 -610,14 +615,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)
@@@ -740,15 -746,6 +751,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 usage.c
index 40b3de51c7dfa3fdaaeb44e1c64a2208560414c5,9d2961ec200fa235bf9a761a2ea0a948b3578334..ed146453cabeb9e82bef0c5610cda47d1b2a0b1c
+++ b/usage.c
@@@ -6,8 -6,6 +6,6 @@@
  #include "git-compat-util.h"
  #include "cache.h"
  
- static int dying;
  void vreportf(const char *prefix, const char *err, va_list params)
  {
        char msg[4096];
@@@ -49,12 -47,19 +47,19 @@@ static void warn_builtin(const char *wa
        vreportf("warning: ", warn, params);
  }
  
+ static int die_is_recursing_builtin(void)
+ {
+       static int dying;
+       return dying++;
+ }
  /* If we are in a dlopen()ed .so write to a global variable would segfault
   * (ugh), so keep things static. */
  static NORETURN_PTR void (*usage_routine)(const char *err, va_list params) = usage_builtin;
  static NORETURN_PTR void (*die_routine)(const char *err, va_list params) = die_builtin;
  static void (*error_routine)(const char *err, va_list params) = error_builtin;
  static void (*warn_routine)(const char *err, va_list params) = warn_builtin;
+ static int (*die_is_recursing)(void) = die_is_recursing_builtin;
  
  void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params))
  {
@@@ -66,6 -71,11 +71,11 @@@ void set_error_routine(void (*routine)(
        error_routine = routine;
  }
  
+ void set_die_is_recursing_routine(int (*routine)(void))
+ {
+       die_is_recursing = routine;
+ }
  void NORETURN usagef(const char *err, ...)
  {
        va_list params;
@@@ -84,11 -94,10 +94,10 @@@ void NORETURN die(const char *err, ...
  {
        va_list params;
  
-       if (dying) {
+       if (die_is_recursing()) {
                fputs("fatal: recursion detected in die handler\n", stderr);
                exit(128);
        }
-       dying = 1;
  
        va_start(params, err);
        die_routine(err, params);
@@@ -102,12 -111,11 +111,11 @@@ void NORETURN die_errno(const char *fmt
        char str_error[256], *err;
        int i, j;
  
-       if (dying) {
+       if (die_is_recursing()) {
                fputs("fatal: recursion detected in die_errno handler\n",
                        stderr);
                exit(128);
        }
-       dying = 1;
  
        err = strerror(errno);
        for (i = j = 0; err[i] && j < sizeof(str_error) - 1; ) {
        va_end(params);
  }
  
 +#undef error
  int error(const char *err, ...)
  {
        va_list params;