OSDN Git Service

Merge branch 'rs/strbuf-leakfix'
authorJunio C Hamano <gitster@pobox.com>
Tue, 19 Sep 2017 01:47:57 +0000 (10:47 +0900)
committerJunio C Hamano <gitster@pobox.com>
Tue, 19 Sep 2017 01:47:57 +0000 (10:47 +0900)
Many leaks of strbuf have been fixed.

* rs/strbuf-leakfix: (34 commits)
  wt-status: release strbuf after use in wt_longstatus_print_tracking()
  wt-status: release strbuf after use in read_rebase_todolist()
  vcs-svn: release strbuf after use in end_revision()
  utf8: release strbuf on error return in strbuf_utf8_replace()
  userdiff: release strbuf after use in userdiff_get_textconv()
  transport-helper: release strbuf after use in process_connect_service()
  sequencer: release strbuf after use in save_head()
  shortlog: release strbuf after use in insert_one_record()
  sha1_file: release strbuf on error return in index_path()
  send-pack: release strbuf on error return in send_pack()
  remote: release strbuf after use in set_url()
  remote: release strbuf after use in migrate_file()
  remote: release strbuf after use in read_remote_branches()
  refs: release strbuf on error return in write_pseudoref()
  notes: release strbuf after use in notes_copy_from_stdin()
  merge: release strbuf after use in write_merge_heads()
  merge: release strbuf after use in save_state()
  mailinfo: release strbuf on error return in handle_boundary()
  mailinfo: release strbuf after use in handle_from()
  help: release strbuf on error return in exec_woman_emacs()
  ...

1  2 
builtin/merge.c
commit.c
convert.c
diff.c
refs.c
sha1_file.c
transport-helper.c

diff --combined builtin/merge.c
@@@ -73,7 -73,6 +73,7 @@@ static int show_progress = -1
  static int default_to_upstream = 1;
  static int signoff;
  static const char *sign_commit;
 +static int verify_msg = 1;
  
  static struct strategy all_strategy[] = {
        { "recursive",  DEFAULT_TWOHEAD | NO_TRIVIAL },
@@@ -237,7 -236,6 +237,7 @@@ static struct option builtin_merge_opti
          N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
        OPT_BOOL(0, "overwrite-ignore", &overwrite_ignore, N_("update ignored files (default)")),
        OPT_BOOL(0, "signoff", &signoff, N_("add Signed-off-by:")),
 +      OPT_BOOL(0, "verify", &verify_msg, N_("verify commit-msg hook")),
        OPT_END()
  };
  
@@@ -255,6 -253,7 +255,7 @@@ static int save_state(struct object_id 
        struct child_process cp = CHILD_PROCESS_INIT;
        struct strbuf buffer = STRBUF_INIT;
        const char *argv[] = {"stash", "create", NULL};
+       int rc = -1;
  
        cp.argv = argv;
        cp.out = -1;
        if (finish_command(&cp) || len < 0)
                die(_("stash failed"));
        else if (!len)          /* no changes */
-               return -1;
+               goto out;
        strbuf_setlen(&buffer, buffer.len-1);
        if (get_oid(buffer.buf, stash))
                die(_("not a valid object: %s"), buffer.buf);
-       return 0;
+       rc = 0;
+ out:
+       strbuf_release(&buffer);
+       return rc;
  }
  
  static void read_empty(unsigned const char *sha1, int verbose)
@@@ -782,12 -784,6 +786,12 @@@ static void prepare_to_commit(struct co
                if (launch_editor(git_path_merge_msg(), NULL, NULL))
                        abort_commit(remoteheads, NULL);
        }
 +
 +      if (verify_msg && run_commit_hook(0 < option_edit, get_index_file(),
 +                                        "commit-msg",
 +                                        git_path_merge_msg(), NULL))
 +              abort_commit(remoteheads, NULL);
 +
        read_merge_msg(&msg);
        strbuf_stripspace(&msg, 0 < option_edit);
        if (!msg.len)
@@@ -942,6 -938,7 +946,7 @@@ static void write_merge_heads(struct co
        if (fast_forward == FF_NO)
                strbuf_addstr(&buf, "no-ff");
        write_file_buf(git_path_merge_mode(), buf.buf, buf.len);
+       strbuf_release(&buf);
  }
  
  static void write_merge_state(struct commit_list *remoteheads)
@@@ -1367,7 -1364,7 +1372,7 @@@ int cmd_merge(int argc, const char **ar
                 * If head can reach all the merge then we are up to date.
                 * but first the most common case of merging one remote.
                 */
 -              finish_up_to_date(_("Already up-to-date."));
 +              finish_up_to_date(_("Already up to date."));
                goto done;
        } else if (fast_forward != FF_NO && !remoteheads->next &&
                        !common->next &&
                        }
                }
                if (up_to_date) {
 -                      finish_up_to_date(_("Already up-to-date. Yeeah!"));
 +                      finish_up_to_date(_("Already up to date. Yeeah!"));
                        goto done;
                }
        }
diff --combined commit.c
+++ b/commit.c
@@@ -134,41 -134,35 +134,41 @@@ int register_commit_graft(struct commit
        return 0;
  }
  
 -struct commit_graft *read_graft_line(char *buf, int len)
 +struct commit_graft *read_graft_line(struct strbuf *line)
  {
        /* The format is just "Commit Parent1 Parent2 ...\n" */
 -      int i;
 +      int i, phase;
 +      const char *tail = NULL;
        struct commit_graft *graft = NULL;
 -      const int entry_size = GIT_SHA1_HEXSZ + 1;
 +      struct object_id dummy_oid, *oid;
  
 -      while (len && isspace(buf[len-1]))
 -              buf[--len] = '\0';
 -      if (buf[0] == '#' || buf[0] == '\0')
 +      strbuf_rtrim(line);
 +      if (!line->len || line->buf[0] == '#')
                return NULL;
 -      if ((len + 1) % entry_size)
 -              goto bad_graft_data;
 -      i = (len + 1) / entry_size - 1;
 -      graft = xmalloc(st_add(sizeof(*graft), st_mult(GIT_SHA1_RAWSZ, i)));
 -      graft->nr_parent = i;
 -      if (get_oid_hex(buf, &graft->oid))
 -              goto bad_graft_data;
 -      for (i = GIT_SHA1_HEXSZ; i < len; i += entry_size) {
 -              if (buf[i] != ' ')
 -                      goto bad_graft_data;
 -              if (get_sha1_hex(buf + i + 1, graft->parent[i/entry_size].hash))
 +      /*
 +       * phase 0 verifies line, counts hashes in line and allocates graft
 +       * phase 1 fills graft
 +       */
 +      for (phase = 0; phase < 2; phase++) {
 +              oid = graft ? &graft->oid : &dummy_oid;
 +              if (parse_oid_hex(line->buf, oid, &tail))
                        goto bad_graft_data;
 +              for (i = 0; *tail != '\0'; i++) {
 +                      oid = graft ? &graft->parent[i] : &dummy_oid;
 +                      if (!isspace(*tail++) || parse_oid_hex(tail, oid, &tail))
 +                              goto bad_graft_data;
 +              }
 +              if (!graft) {
 +                      graft = xmalloc(st_add(sizeof(*graft),
 +                                             st_mult(sizeof(struct object_id), i)));
 +                      graft->nr_parent = i;
 +              }
        }
        return graft;
  
  bad_graft_data:
 -      error("bad graft data: %s", buf);
 -      free(graft);
 +      error("bad graft data: %s", line->buf);
 +      assert(!graft);
        return NULL;
  }
  
@@@ -180,7 -174,7 +180,7 @@@ static int read_graft_file(const char *
                return -1;
        while (!strbuf_getwholeline(&buf, fp, '\n')) {
                /* The format is just "Commit Parent1 Parent2 ...\n" */
 -              struct commit_graft *graft = read_graft_line(buf.buf, buf.len);
 +              struct commit_graft *graft = read_graft_line(&buf);
                if (!graft)
                        continue;
                if (register_commit_graft(graft, 1))
@@@ -1570,10 -1564,13 +1570,13 @@@ int commit_tree_extended(const char *ms
        if (encoding_is_utf8 && !verify_utf8(&buffer))
                fprintf(stderr, _(commit_utf8_warn));
  
-       if (sign_commit && do_sign_commit(&buffer, sign_commit))
-               return -1;
+       if (sign_commit && do_sign_commit(&buffer, sign_commit)) {
+               result = -1;
+               goto out;
+       }
  
        result = write_sha1_file(buffer.buf, buffer.len, commit_type, ret);
+ out:
        strbuf_release(&buffer);
        return result;
  }
diff --combined convert.c
+++ b/convert.c
@@@ -423,8 -423,10 +423,10 @@@ static int filter_buffer_or_fd(int in, 
        child_process.in = -1;
        child_process.out = out;
  
-       if (start_command(&child_process))
+       if (start_command(&child_process)) {
+               strbuf_release(&cmd);
                return error("cannot fork to run external filter '%s'", params->cmd);
+       }
  
        sigchain_push(SIGPIPE, SIG_IGN);
  
@@@ -1040,6 -1042,7 +1042,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)
diff --combined diff.c
--- 1/diff.c
--- 2/diff.c
+++ b/diff.c
@@@ -459,7 -459,7 +459,7 @@@ static struct diff_tempfile 
         * If this diff_tempfile instance refers to a temporary file,
         * this tempfile object is used to manage its lifetime.
         */
 -      struct tempfile tempfile;
 +      struct tempfile *tempfile;
  } diff_temp[2];
  
  struct emit_callback {
@@@ -1414,7 -1414,7 +1414,7 @@@ static void remove_tempfile(void
  {
        int i;
        for (i = 0; i < ARRAY_SIZE(diff_temp); i++) {
 -              if (is_tempfile_active(&diff_temp[i].tempfile))
 +              if (is_tempfile_active(diff_temp[i].tempfile))
                        delete_tempfile(&diff_temp[i].tempfile);
                diff_temp[i].name = NULL;
        }
@@@ -2583,6 -2583,7 +2583,7 @@@ static void show_stats(struct diffstat_
        }
  
        print_stat_summary_inserts_deletes(options, total_files, adds, dels);
+       strbuf_release(&out);
  }
  
  static void show_shortstats(struct diffstat_t *data, struct diff_options *options)
@@@ -3720,6 -3721,7 +3721,6 @@@ static void prep_temp_blob(const char *
                           const struct object_id *oid,
                           int mode)
  {
 -      int fd;
        struct strbuf buf = STRBUF_INIT;
        struct strbuf template = STRBUF_INIT;
        char *path_dup = xstrdup(path);
        strbuf_addstr(&template, "XXXXXX_");
        strbuf_addstr(&template, base);
  
 -      fd = mks_tempfile_ts(&temp->tempfile, template.buf, strlen(base) + 1);
 -      if (fd < 0)
 +      temp->tempfile = mks_tempfile_ts(template.buf, strlen(base) + 1);
 +      if (!temp->tempfile)
                die_errno("unable to create temp-file");
        if (convert_to_working_tree(path,
                        (const char *)blob, (size_t)size, &buf)) {
                blob = buf.buf;
                size = buf.len;
        }
 -      if (write_in_full(fd, blob, size) != size)
 +      if (write_in_full(temp->tempfile->fd, blob, size) != size ||
 +          close_tempfile_gently(temp->tempfile))
                die_errno("unable to write temp-file");
 -      close_tempfile(&temp->tempfile);
 -      temp->name = get_tempfile_path(&temp->tempfile);
 +      temp->name = get_tempfile_path(temp->tempfile);
        oid_to_hex_r(temp->hex, oid);
        xsnprintf(temp->mode, sizeof(temp->mode), "%06o", mode);
        strbuf_release(&buf);
@@@ -5288,6 -5290,7 +5289,7 @@@ static void show_rename_copy(struct dif
        emit_diff_symbol(opt, DIFF_SYMBOL_SUMMARY,
                                 sb.buf, sb.len, 0);
        show_mode_change(opt, p, 0);
+       strbuf_release(&sb);
  }
  
  static void diff_summary(struct diff_options *opt, struct diff_filepair *p)
                        strbuf_addf(&sb, " (%d%%)\n", similarity_index(p));
                        emit_diff_symbol(opt, DIFF_SYMBOL_SUMMARY,
                                         sb.buf, sb.len, 0);
+                       strbuf_release(&sb);
                }
                show_mode_change(opt, p, !p->score);
                break;
diff --combined refs.c
--- 1/refs.c
--- 2/refs.c
+++ b/refs.c
@@@ -336,6 -336,12 +336,6 @@@ int for_each_tag_ref(each_ref_fn fn, vo
        return refs_for_each_tag_ref(get_main_ref_store(), fn, cb_data);
  }
  
 -int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 -{
 -      return refs_for_each_tag_ref(get_submodule_ref_store(submodule),
 -                                   fn, cb_data);
 -}
 -
  int refs_for_each_branch_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
  {
        return refs_for_each_ref_in(refs, "refs/heads/", fn, cb_data);
@@@ -346,6 -352,12 +346,6 @@@ int for_each_branch_ref(each_ref_fn fn
        return refs_for_each_branch_ref(get_main_ref_store(), fn, cb_data);
  }
  
 -int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 -{
 -      return refs_for_each_branch_ref(get_submodule_ref_store(submodule),
 -                                      fn, cb_data);
 -}
 -
  int refs_for_each_remote_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
  {
        return refs_for_each_ref_in(refs, "refs/remotes/", fn, cb_data);
@@@ -356,6 -368,12 +356,6 @@@ int for_each_remote_ref(each_ref_fn fn
        return refs_for_each_remote_ref(get_main_ref_store(), fn, cb_data);
  }
  
 -int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 -{
 -      return refs_for_each_remote_ref(get_submodule_ref_store(submodule),
 -                                      fn, cb_data);
 -}
 -
  int head_ref_namespaced(each_ref_fn fn, void *cb_data)
  {
        struct strbuf buf = STRBUF_INIT;
@@@ -594,7 -612,7 +594,7 @@@ static int write_pseudoref(const char *
        if (fd < 0) {
                strbuf_addf(err, "could not open '%s' for writing: %s",
                            filename, strerror(errno));
-               return -1;
+               goto done;
        }
  
        if (old_sha1) {
@@@ -1248,13 -1266,19 +1248,13 @@@ int refs_rename_ref_available(struct re
        return ok;
  }
  
 -int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 +int refs_head_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
  {
        struct object_id oid;
        int flag;
  
 -      if (submodule) {
 -              if (resolve_gitlink_ref(submodule, "HEAD", oid.hash) == 0)
 -                      return fn("HEAD", &oid, 0, cb_data);
 -
 -              return 0;
 -      }
 -
 -      if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag))
 +      if (!refs_read_ref_full(refs, "HEAD", RESOLVE_REF_READING,
 +                              oid.hash, &flag))
                return fn("HEAD", &oid, flag, cb_data);
  
        return 0;
  
  int head_ref(each_ref_fn fn, void *cb_data)
  {
 -      return head_ref_submodule(NULL, fn, cb_data);
 +      return refs_head_ref(get_main_ref_store(), fn, cb_data);
  }
  
  struct ref_iterator *refs_ref_iterator_begin(
@@@ -1320,6 -1344,11 +1320,6 @@@ int for_each_ref(each_ref_fn fn, void *
        return refs_for_each_ref(get_main_ref_store(), fn, cb_data);
  }
  
 -int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 -{
 -      return refs_for_each_ref(get_submodule_ref_store(submodule), fn, cb_data);
 -}
 -
  int refs_for_each_ref_in(struct ref_store *refs, const char *prefix,
                         each_ref_fn fn, void *cb_data)
  {
@@@ -1341,15 -1370,23 +1341,15 @@@ int for_each_fullref_in(const char *pre
                               prefix, fn, 0, flag, cb_data);
  }
  
 -int for_each_ref_in_submodule(const char *submodule, const char *prefix,
 -                            each_ref_fn fn, void *cb_data)
 -{
 -      return refs_for_each_ref_in(get_submodule_ref_store(submodule),
 -                                  prefix, fn, cb_data);
 -}
 -
 -int for_each_fullref_in_submodule(const char *submodule, const char *prefix,
 -                                each_ref_fn fn, void *cb_data,
 -                                unsigned int broken)
 +int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 +                           each_ref_fn fn, void *cb_data,
 +                           unsigned int broken)
  {
        unsigned int flag = 0;
  
        if (broken)
                flag = DO_FOR_EACH_INCLUDE_BROKEN;
 -      return do_for_each_ref(get_submodule_ref_store(submodule),
 -                             prefix, fn, 0, flag, cb_data);
 +      return do_for_each_ref(refs, prefix, fn, 0, flag, cb_data);
  }
  
  int for_each_replace_ref(each_ref_fn fn, void *cb_data)
@@@ -1484,10 -1521,25 +1484,10 @@@ const char *resolve_ref_unsafe(const ch
  int resolve_gitlink_ref(const char *submodule, const char *refname,
                        unsigned char *sha1)
  {
 -      size_t len = strlen(submodule);
        struct ref_store *refs;
        int flags;
  
 -      while (len && submodule[len - 1] == '/')
 -              len--;
 -
 -      if (!len)
 -              return -1;
 -
 -      if (submodule[len]) {
 -              /* We need to strip off one or more trailing slashes */
 -              char *stripped = xmemdupz(submodule, len);
 -
 -              refs = get_submodule_ref_store(stripped);
 -              free(stripped);
 -      } else {
 -              refs = get_submodule_ref_store(submodule);
 -      }
 +      refs = get_submodule_ref_store(submodule);
  
        if (!refs)
                return -1;
@@@ -1602,32 -1654,31 +1602,32 @@@ struct ref_store *get_submodule_ref_sto
  {
        struct strbuf submodule_sb = STRBUF_INIT;
        struct ref_store *refs;
 -      int ret;
 +      char *to_free = NULL;
 +      size_t len;
  
 -      if (!submodule || !*submodule) {
 -              /*
 -               * FIXME: This case is ideally not allowed. But that
 -               * can't happen until we clean up all the callers.
 -               */
 -              return get_main_ref_store();
 -      }
 +      if (!submodule)
 +              return NULL;
 +
 +      len = strlen(submodule);
 +      while (len && is_dir_sep(submodule[len - 1]))
 +              len--;
 +      if (!len)
 +              return NULL;
 +
 +      if (submodule[len])
 +              /* We need to strip off one or more trailing slashes */
 +              submodule = to_free = xmemdupz(submodule, len);
  
        refs = lookup_ref_store_map(&submodule_ref_stores, submodule);
        if (refs)
 -              return refs;
 +              goto done;
  
        strbuf_addstr(&submodule_sb, submodule);
 -      ret = is_nonbare_repository_dir(&submodule_sb);
 -      strbuf_release(&submodule_sb);
 -      if (!ret)
 -              return NULL;
 +      if (!is_nonbare_repository_dir(&submodule_sb))
 +              goto done;
  
 -      ret = submodule_to_gitdir(&submodule_sb, submodule);
 -      if (ret) {
 -              strbuf_release(&submodule_sb);
 -              return NULL;
 -      }
 +      if (submodule_to_gitdir(&submodule_sb, submodule))
 +              goto done;
  
        /* assume that add_submodule_odb() has been called */
        refs = ref_store_init(submodule_sb.buf,
        register_ref_store_map(&submodule_ref_stores, "submodule",
                               refs, submodule);
  
 +done:
        strbuf_release(&submodule_sb);
 +      free(to_free);
 +
        return refs;
  }
  
diff --combined sha1_file.c
@@@ -30,7 -30,7 +30,7 @@@
  #include "quote.h"
  #include "packfile.h"
  
 -const unsigned char null_sha1[20];
 +const unsigned char null_sha1[GIT_MAX_RAWSZ];
  const struct object_id null_oid;
  const struct object_id empty_tree_oid = {
        EMPTY_TREE_SHA1_BIN_LITERAL
@@@ -1820,6 -1820,7 +1820,7 @@@ int index_path(struct object_id *oid, c
  {
        int fd;
        struct strbuf sb = STRBUF_INIT;
+       int rc = 0;
  
        switch (st->st_mode & S_IFMT) {
        case S_IFREG:
                if (!(flags & HASH_WRITE_OBJECT))
                        hash_sha1_file(sb.buf, sb.len, blob_type, oid->hash);
                else if (write_sha1_file(sb.buf, sb.len, blob_type, oid->hash))
-                       return error("%s: failed to insert into database",
-                                    path);
+                       rc = error("%s: failed to insert into database", path);
                strbuf_release(&sb);
                break;
        case S_IFDIR:
        default:
                return error("%s: unsupported file type", path);
        }
-       return 0;
+       return rc;
  }
  
  int read_pack_header(int fd, struct pack_header *header)
diff --combined transport-helper.c
@@@ -604,6 -604,7 +604,7 @@@ static int process_connect_service(stru
                        cmdbuf.buf);
  
  exit:
+       strbuf_release(&cmdbuf);
        fclose(input);
        return ret;
  }
@@@ -1117,13 -1118,6 +1118,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;