Merge branch 'ma/ts-cleanups' into next
authorJunio C Hamano <gitster@pobox.com>
Fri, 25 Aug 2017 21:41:11 +0000 (14:41 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 25 Aug 2017 21:41:11 +0000 (14:41 -0700)
Assorted bugfixes and clean-ups.

* ma/ts-cleanups:
ThreadSanitizer: add suppressions
strbuf_setlen: don't write to strbuf_slopbuf
pack-objects: take lock before accessing `remaining`
convert: always initialize attr_action in convert_attrs

1  2 
builtin/pack-objects.c
color.c
convert.c
strbuf.h
transport-helper.c
diff --combined builtin/pack-objects.c
index 82ad6e0c813375b480129c22035afc4d6674992a,ffb13f7800feac6622afb2dc345f3e0af302d145..a57b4f058dff3ceec80355560bf6a856f130b8c0
@@@ -25,7 -25,6 +25,7 @@@
  #include "sha1-array.h"
  #include "argv-array.h"
  #include "mru.h"
 +#include "packfile.h"
  
  static const char *pack_usage[] = {
        N_("git pack-objects --stdout [<options>...] [< <ref-list> | < <object-list>]"),
@@@ -1290,7 -1289,7 +1290,7 @@@ static int done_pbase_path_pos(unsigne
  
  static int check_pbase_path(unsigned hash)
  {
 -      int pos = (!done_pbase_paths) ? -1 : done_pbase_path_pos(hash);
 +      int pos = done_pbase_path_pos(hash);
        if (0 <= pos)
                return 1;
        pos = -pos - 1;
                   done_pbase_paths_alloc);
        done_pbase_paths_num++;
        if (pos < done_pbase_paths_num)
 -              memmove(done_pbase_paths + pos + 1,
 -                      done_pbase_paths + pos,
 -                      (done_pbase_paths_num - pos - 1) * sizeof(unsigned));
 +              MOVE_ARRAY(done_pbase_paths + pos + 1, done_pbase_paths + pos,
 +                         done_pbase_paths_num - pos - 1);
        done_pbase_paths[pos] = hash;
        return 0;
  }
@@@ -2171,7 -2171,10 +2171,10 @@@ static void *threaded_find_deltas(void 
  {
        struct thread_params *me = arg;
  
+       progress_lock();
        while (me->remaining) {
+               progress_unlock();
                find_deltas(me->list, &me->remaining,
                            me->window, me->depth, me->processed);
  
                        pthread_cond_wait(&me->cond, &me->mutex);
                me->data_ready = 0;
                pthread_mutex_unlock(&me->mutex);
+               progress_lock();
        }
+       progress_unlock();
        /* leave ->working 1 so that this doesn't get more work assigned */
        return NULL;
  }
diff --combined color.c
index 7aa8b076f045e5c0b74826153030e2923c0fc56e,9a9261ac164f503e2a9240806f4f44bedf9b8192..9ccd954d6ba5cea1dd275cac23bb19966b9c6386
+++ b/color.c
@@@ -338,6 -338,13 +338,13 @@@ static int check_auto_color(void
  
  int want_color(int var)
  {
+       /*
+        * NEEDSWORK: This function is sometimes used from multiple threads, and
+        * we end up using want_auto racily. That "should not matter" since
+        * we always write the same value, but it's still wrong. This function
+        * is listed in .tsan-suppressions for the time being.
+        */
        static int want_auto = -1;
  
        if (var < 0)
@@@ -361,6 -368,14 +368,6 @@@ int git_color_config(const char *var, c
        return 0;
  }
  
 -int git_color_default_config(const char *var, const char *value, void *cb)
 -{
 -      if (git_color_config(var, value, cb) < 0)
 -              return -1;
 -
 -      return git_default_config(var, value, cb);
 -}
 -
  void color_print_strbuf(FILE *fp, const char *color, const struct strbuf *sb)
  {
        if (*color)
diff --combined convert.c
index c5f0b210370c0ee83ef59298fa2260d627332399,77b7615a5481c1ff55ffb0cdfdd2e9b7a782db1d..02962261c58f71485a27f9799a5db29ec77e0bc7
+++ b/convert.c
@@@ -501,7 -501,6 +501,7 @@@ static int apply_single_file_filter(con
  
  #define CAP_CLEAN    (1u<<0)
  #define CAP_SMUDGE   (1u<<1)
 +#define CAP_DELAY    (1u<<2)
  
  struct cmd2process {
        struct subprocess_entry subprocess; /* must be the first member! */
@@@ -513,49 -512,69 +513,49 @@@ static struct hashmap subprocess_map
  
  static int start_multi_file_filter_fn(struct subprocess_entry *subprocess)
  {
 -      int err;
 +      static int versions[] = {2, 0};
 +      static struct subprocess_capability capabilities[] = {
 +              { "clean",  CAP_CLEAN  },
 +              { "smudge", CAP_SMUDGE },
 +              { "delay",  CAP_DELAY  },
 +              { NULL, 0 }
 +      };
        struct cmd2process *entry = (struct cmd2process *)subprocess;
 -      struct string_list cap_list = STRING_LIST_INIT_NODUP;
 -      char *cap_buf;
 -      const char *cap_name;
 -      struct child_process *process = &subprocess->process;
 -      const char *cmd = subprocess->cmd;
 -
 -      sigchain_push(SIGPIPE, SIG_IGN);
 -
 -      err = packet_writel(process->in, "git-filter-client", "version=2", NULL);
 -      if (err)
 -              goto done;
 -
 -      err = strcmp(packet_read_line(process->out, NULL), "git-filter-server");
 -      if (err) {
 -              error("external filter '%s' does not support filter protocol version 2", cmd);
 -              goto done;
 -      }
 -      err = strcmp(packet_read_line(process->out, NULL), "version=2");
 -      if (err)
 -              goto done;
 -      err = packet_read_line(process->out, NULL) != NULL;
 -      if (err)
 -              goto done;
 -
 -      err = packet_writel(process->in, "capability=clean", "capability=smudge", NULL);
 -
 -      for (;;) {
 -              cap_buf = packet_read_line(process->out, NULL);
 -              if (!cap_buf)
 -                      break;
 -              string_list_split_in_place(&cap_list, cap_buf, '=', 1);
 -
 -              if (cap_list.nr != 2 || strcmp(cap_list.items[0].string, "capability"))
 -                      continue;
 -
 -              cap_name = cap_list.items[1].string;
 -              if (!strcmp(cap_name, "clean")) {
 -                      entry->supported_capabilities |= CAP_CLEAN;
 -              } else if (!strcmp(cap_name, "smudge")) {
 -                      entry->supported_capabilities |= CAP_SMUDGE;
 -              } else {
 -                      warning(
 -                              "external filter '%s' requested unsupported filter capability '%s'",
 -                              cmd, cap_name
 -                      );
 -              }
 +      return subprocess_handshake(subprocess, "git-filter", versions, NULL,
 +                                  capabilities,
 +                                  &entry->supported_capabilities);
 +}
  
 -              string_list_clear(&cap_list, 0);
 +static void handle_filter_error(const struct strbuf *filter_status,
 +                              struct cmd2process *entry,
 +                              const unsigned int wanted_capability) {
 +      if (!strcmp(filter_status->buf, "error"))
 +              ; /* The filter signaled a problem with the file. */
 +      else if (!strcmp(filter_status->buf, "abort") && wanted_capability) {
 +              /*
 +               * The filter signaled a permanent problem. Don't try to filter
 +               * files with the same command for the lifetime of the current
 +               * Git process.
 +               */
 +               entry->supported_capabilities &= ~wanted_capability;
 +      } else {
 +              /*
 +               * Something went wrong with the protocol filter.
 +               * Force shutdown and restart if another blob requires filtering.
 +               */
 +              error("external filter '%s' failed", entry->subprocess.cmd);
 +              subprocess_stop(&subprocess_map, &entry->subprocess);
 +              free(entry);
        }
 -
 -done:
 -      sigchain_pop(SIGPIPE);
 -
 -      return err;
  }
  
  static int apply_multi_file_filter(const char *path, const char *src, size_t len,
                                   int fd, struct strbuf *dst, const char *cmd,
 -                                 const unsigned int wanted_capability)
 +                                 const unsigned int wanted_capability,
 +                                 struct delayed_checkout *dco)
  {
        int err;
 +      int can_delay = 0;
        struct cmd2process *entry;
        struct child_process *process;
        struct strbuf nbuf = STRBUF_INIT;
  
        if (!subprocess_map_initialized) {
                subprocess_map_initialized = 1;
 -              hashmap_init(&subprocess_map, (hashmap_cmp_fn) cmd2process_cmp,
 -                           NULL, 0);
 +              hashmap_init(&subprocess_map, cmd2process_cmp, NULL, 0);
                entry = NULL;
        } else {
                entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd);
        }
        process = &entry->subprocess.process;
  
 -      if (!(wanted_capability & entry->supported_capabilities))
 +      if (!(entry->supported_capabilities & wanted_capability))
                return 0;
  
 -      if (CAP_CLEAN & wanted_capability)
 +      if (wanted_capability & CAP_CLEAN)
                filter_type = "clean";
 -      else if (CAP_SMUDGE & wanted_capability)
 +      else if (wanted_capability & CAP_SMUDGE)
                filter_type = "smudge";
        else
                die("unexpected filter type");
        if (err)
                goto done;
  
 +      if ((entry->supported_capabilities & CAP_DELAY) &&
 +          dco && dco->state == CE_CAN_DELAY) {
 +              can_delay = 1;
 +              err = packet_write_fmt_gently(process->in, "can-delay=1\n");
 +              if (err)
 +                      goto done;
 +      }
 +
        err = packet_flush_gently(process->in);
        if (err)
                goto done;
        if (err)
                goto done;
  
 -      err = strcmp(filter_status.buf, "success");
 +      if (can_delay && !strcmp(filter_status.buf, "delayed")) {
 +              string_list_insert(&dco->filters, cmd);
 +              string_list_insert(&dco->paths, path);
 +      } else {
 +              /* The filter got the blob and wants to send us a response. */
 +              err = strcmp(filter_status.buf, "success");
 +              if (err)
 +                      goto done;
 +
 +              err = read_packetized_to_strbuf(process->out, &nbuf) < 0;
 +              if (err)
 +                      goto done;
 +
 +              err = subprocess_read_status(process->out, &filter_status);
 +              if (err)
 +                      goto done;
 +
 +              err = strcmp(filter_status.buf, "success");
 +      }
 +
 +done:
 +      sigchain_pop(SIGPIPE);
 +
 +      if (err)
 +              handle_filter_error(&filter_status, entry, wanted_capability);
 +      else
 +              strbuf_swap(dst, &nbuf);
 +      strbuf_release(&nbuf);
 +      return !err;
 +}
 +
 +
 +int async_query_available_blobs(const char *cmd, struct string_list *available_paths)
 +{
 +      int err;
 +      char *line;
 +      struct cmd2process *entry;
 +      struct child_process *process;
 +      struct strbuf filter_status = STRBUF_INIT;
 +
 +      assert(subprocess_map_initialized);
 +      entry = (struct cmd2process *)subprocess_find_entry(&subprocess_map, cmd);
 +      if (!entry) {
 +              error("external filter '%s' is not available anymore although "
 +                    "not all paths have been filtered", cmd);
 +              return 0;
 +      }
 +      process = &entry->subprocess.process;
 +      sigchain_push(SIGPIPE, SIG_IGN);
 +
 +      err = packet_write_fmt_gently(
 +              process->in, "command=list_available_blobs\n");
        if (err)
                goto done;
  
 -      err = read_packetized_to_strbuf(process->out, &nbuf) < 0;
 +      err = packet_flush_gently(process->in);
        if (err)
                goto done;
  
 +      while ((line = packet_read_line(process->out, NULL))) {
 +              const char *path;
 +              if (skip_prefix(line, "pathname=", &path))
 +                      string_list_insert(available_paths, xstrdup(path));
 +              else
 +                      ; /* ignore unknown keys */
 +      }
 +
        err = subprocess_read_status(process->out, &filter_status);
        if (err)
                goto done;
  done:
        sigchain_pop(SIGPIPE);
  
 -      if (err) {
 -              if (!strcmp(filter_status.buf, "error")) {
 -                      /* The filter signaled a problem with the file. */
 -              } else if (!strcmp(filter_status.buf, "abort")) {
 -                      /*
 -                       * The filter signaled a permanent problem. Don't try to filter
 -                       * files with the same command for the lifetime of the current
 -                       * Git process.
 -                       */
 -                       entry->supported_capabilities &= ~wanted_capability;
 -              } else {
 -                      /*
 -                       * Something went wrong with the protocol filter.
 -                       * Force shutdown and restart if another blob requires filtering.
 -                       */
 -                      error("external filter '%s' failed", cmd);
 -                      subprocess_stop(&subprocess_map, &entry->subprocess);
 -                      free(entry);
 -              }
 -      } else {
 -              strbuf_swap(dst, &nbuf);
 -      }
 -      strbuf_release(&nbuf);
 +      if (err)
 +              handle_filter_error(&filter_status, entry, 0);
        return !err;
  }
  
@@@ -725,8 -699,7 +725,8 @@@ static struct convert_driver 
  
  static int apply_filter(const char *path, const char *src, size_t len,
                        int fd, struct strbuf *dst, struct convert_driver *drv,
 -                      const unsigned int wanted_capability)
 +                      const unsigned int wanted_capability,
 +                      struct delayed_checkout *dco)
  {
        const char *cmd = NULL;
  
        if (!dst)
                return 1;
  
 -      if ((CAP_CLEAN & wanted_capability) && !drv->process && drv->clean)
 +      if ((wanted_capability & CAP_CLEAN) && !drv->process && drv->clean)
                cmd = drv->clean;
 -      else if ((CAP_SMUDGE & wanted_capability) && !drv->process && drv->smudge)
 +      else if ((wanted_capability & CAP_SMUDGE) && !drv->process && drv->smudge)
                cmd = drv->smudge;
  
        if (cmd && *cmd)
                return apply_single_file_filter(path, src, len, fd, dst, cmd);
        else if (drv->process && *drv->process)
 -              return apply_multi_file_filter(path, src, len, fd, dst, drv->process, wanted_capability);
 +              return apply_multi_file_filter(path, src, len, fd, dst,
 +                      drv->process, wanted_capability, dco);
  
        return 0;
  }
@@@ -1040,7 -1012,6 +1040,6 @@@ static void convert_attrs(struct conv_a
                ca->crlf_action = git_path_check_crlf(ccheck + 4);
                if (ca->crlf_action == CRLF_UNDEFINED)
                        ca->crlf_action = git_path_check_crlf(ccheck + 0);
-               ca->attr_action = ca->crlf_action;
                ca->ident = git_path_check_ident(ccheck + 1);
                ca->drv = git_path_check_convert(ccheck + 2);
                if (ca->crlf_action != CRLF_BINARY) {
                        else if (eol_attr == EOL_CRLF)
                                ca->crlf_action = CRLF_TEXT_CRLF;
                }
-               ca->attr_action = ca->crlf_action;
        } else {
                ca->drv = NULL;
                ca->crlf_action = CRLF_UNDEFINED;
                ca->ident = 0;
        }
+       /* Save attr and make a decision for action */
+       ca->attr_action = ca->crlf_action;
        if (ca->crlf_action == CRLF_TEXT)
                ca->crlf_action = text_eol_is_crlf() ? CRLF_TEXT_CRLF : CRLF_TEXT_INPUT;
        if (ca->crlf_action == CRLF_UNDEFINED && auto_crlf == AUTO_CRLF_FALSE)
@@@ -1086,7 -1059,7 +1087,7 @@@ int would_convert_to_git_filter_fd(cons
        if (!ca.drv->required)
                return 0;
  
 -      return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN);
 +      return apply_filter(path, NULL, 0, -1, NULL, ca.drv, CAP_CLEAN, NULL);
  }
  
  const char *get_convert_attr_ascii(const char *path)
@@@ -1124,7 -1097,7 +1125,7 @@@ int convert_to_git(const struct index_s
  
        convert_attrs(&ca, path);
  
 -      ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN);
 +      ret |= apply_filter(path, src, len, -1, dst, ca.drv, CAP_CLEAN, NULL);
        if (!ret && ca.drv && ca.drv->required)
                die("%s: clean filter '%s' failed", path, ca.drv->name);
  
                src = dst->buf;
                len = dst->len;
        }
 -      ret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);
 -      if (ret && dst) {
 -              src = dst->buf;
 -              len = dst->len;
 +      if (checksafe != SAFE_CRLF_KEEP_CRLF) {
 +              ret |= crlf_to_git(istate, path, src, len, dst, ca.crlf_action, checksafe);
 +              if (ret && dst) {
 +                      src = dst->buf;
 +                      len = dst->len;
 +              }
        }
        return ret | ident_to_git(path, src, len, dst, ca.ident);
  }
@@@ -1152,7 -1123,7 +1153,7 @@@ void convert_to_git_filter_fd(const str
        assert(ca.drv);
        assert(ca.drv->clean || ca.drv->process);
  
 -      if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN))
 +      if (!apply_filter(path, NULL, 0, fd, dst, ca.drv, CAP_CLEAN, NULL))
                die("%s: clean filter '%s' failed", path, ca.drv->name);
  
        crlf_to_git(istate, path, dst->buf, dst->len, dst, ca.crlf_action, checksafe);
  
  static int convert_to_working_tree_internal(const char *path, const char *src,
                                            size_t len, struct strbuf *dst,
 -                                          int normalizing)
 +                                          int normalizing, struct delayed_checkout *dco)
  {
        int ret = 0, ret_filter = 0;
        struct conv_attrs ca;
                }
        }
  
 -      ret_filter = apply_filter(path, src, len, -1, dst, ca.drv, CAP_SMUDGE);
 +      ret_filter = apply_filter(
 +              path, src, len, -1, dst, ca.drv, CAP_SMUDGE, dco);
        if (!ret_filter && ca.drv && ca.drv->required)
                die("%s: smudge filter %s failed", path, ca.drv->name);
  
        return ret | ret_filter;
  }
  
 +int async_convert_to_working_tree(const char *path, const char *src,
 +                                size_t len, struct strbuf *dst,
 +                                void *dco)
 +{
 +      return convert_to_working_tree_internal(path, src, len, dst, 0, dco);
 +}
 +
  int convert_to_working_tree(const char *path, const char *src, size_t len, struct strbuf *dst)
  {
 -      return convert_to_working_tree_internal(path, src, len, dst, 0);
 +      return convert_to_working_tree_internal(path, src, len, dst, 0, NULL);
  }
  
  int renormalize_buffer(const struct index_state *istate, const char *path,
                       const char *src, size_t len, struct strbuf *dst)
  {
 -      int ret = convert_to_working_tree_internal(path, src, len, dst, 1);
 +      int ret = convert_to_working_tree_internal(path, src, len, dst, 1, NULL);
        if (ret) {
                src = dst->buf;
                len = dst->len;
diff --combined strbuf.h
index e705b94db55578aabb2063f7364220978cea5a40,1a77fe146a9c0a83bda4461b084148a5a89b63e2..7496cb8ec5a1f8baeda90f8653b2fcbbe2527390
+++ b/strbuf.h
@@@ -68,7 -68,7 +68,7 @@@ struct strbuf 
  };
  
  extern char strbuf_slopbuf[];
 -#define STRBUF_INIT  { 0, 0, strbuf_slopbuf }
 +#define STRBUF_INIT  { .alloc = 0, .len = 0, .buf = strbuf_slopbuf }
  
  /**
   * Life Cycle Functions
@@@ -147,7 -147,10 +147,10 @@@ static inline void strbuf_setlen(struc
        if (len > (sb->alloc ? sb->alloc - 1 : 0))
                die("BUG: strbuf_setlen() beyond buffer");
        sb->len = len;
-       sb->buf[len] = '\0';
+       if (sb->buf != strbuf_slopbuf)
+               sb->buf[len] = '\0';
+       else
+               assert(!strbuf_slopbuf[0]);
  }
  
  /**
diff --combined transport-helper.c
index 8f68d69a86bd919162b62514af0b1d1d55e204a1,2b830b22900545209fd4e697d88c6faf3d1fd431..f50b34df2dba4827db6a96cf97d1c4ac7ca811a0
@@@ -927,7 -927,7 +927,7 @@@ static int push_refs_with_export(struc
                struct object_id oid;
  
                private = apply_refspecs(data->refspecs, data->refspec_nr, ref->name);
 -              if (private && !get_sha1(private, oid.hash)) {
 +              if (private && !get_oid(private, &oid)) {
                        strbuf_addf(&buf, "^%s", private);
                        string_list_append(&revlist_args, strbuf_detach(&buf, NULL));
                        oidcpy(&ref->old_oid, &oid);
@@@ -1117,6 -1117,13 +1117,13 @@@ int transport_helper_init(struct transp
  __attribute__((format (printf, 1, 2)))
  static void transfer_debug(const char *fmt, ...)
  {
+       /*
+        * NEEDSWORK: This function is sometimes used from multiple threads, and
+        * we end up using debug_enabled racily. That "should not matter" since
+        * we always write the same value, but it's still wrong. This function
+        * is listed in .tsan-suppressions for the time being.
+        */
        va_list args;
        char msgbuf[PBUFFERSIZE];
        static int debug_enabled = -1;