Merge branch 'rs/checkout-am-fix-unborn'
authorJunio C Hamano <gitster@pobox.com>
Tue, 23 May 2017 04:46:05 +0000 (13:46 +0900)
committerJunio C Hamano <gitster@pobox.com>
Tue, 23 May 2017 04:46:05 +0000 (13:46 +0900)
A few codepaths in "checkout" and "am" working on an unborn branch
tried to access an uninitialized piece of memory.

* rs/checkout-am-fix-unborn:
am: check return value of resolve_refdup before using hash
checkout: check return value of resolve_refdup before using hash

1  2 
builtin/am.c
builtin/checkout.c
diff --combined builtin/am.c
index a63935cc83409ef99551a69da79d52ac297fe83a,695968622a88771db584cb9dd394de48f361bbd3..8e9ac1144d205252551e4b22605d8d8da55297b3
@@@ -134,15 -134,17 +134,15 @@@ struct am_state 
  };
  
  /**
 - * Initializes am_state with the default values. The state directory is set to
 - * dir.
 + * Initializes am_state with the default values.
   */
 -static void am_state_init(struct am_state *state, const char *dir)
 +static void am_state_init(struct am_state *state)
  {
        int gpgsign;
  
        memset(state, 0, sizeof(*state));
  
 -      assert(dir);
 -      state->dir = xstrdup(dir);
 +      state->dir = git_pathdup("rebase-apply");
  
        state->prec = 4;
  
@@@ -760,18 -762,14 +760,18 @@@ static int split_mail_conv(mail_conv_f
                mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1);
  
                out = fopen(mail, "w");
 -              if (!out)
 +              if (!out) {
 +                      if (in != stdin)
 +                              fclose(in);
                        return error_errno(_("could not open '%s' for writing"),
                                           mail);
 +              }
  
                ret = fn(out, in, keep_cr);
  
                fclose(out);
 -              fclose(in);
 +              if (in != stdin)
 +                      fclose(in);
  
                if (ret)
                        return error(_("could not parse patch '%s'"), *paths);
@@@ -879,12 -877,12 +879,12 @@@ static int hg_patch_to_mail(FILE *out, 
                if (skip_prefix(sb.buf, "# User ", &str))
                        fprintf(out, "From: %s\n", str);
                else if (skip_prefix(sb.buf, "# Date ", &str)) {
 -                      unsigned long timestamp;
 +                      timestamp_t timestamp;
                        long tz, tz2;
                        char *end;
  
                        errno = 0;
 -                      timestamp = strtoul(str, &end, 10);
 +                      timestamp = parse_timestamp(str, &end, 10);
                        if (errno)
                                return error(_("invalid timestamp"));
  
@@@ -1051,7 -1049,7 +1051,7 @@@ static void am_setup(struct am_state *s
        } else {
                write_state_text(state, "abort-safety", "");
                if (!state->rebasing)
 -                      delete_ref("ORIG_HEAD", NULL, 0);
 +                      delete_ref(NULL, "ORIG_HEAD", NULL, 0);
        }
  
        /*
@@@ -1183,39 -1181,42 +1183,39 @@@ static void NORETURN die_user_resolve(c
        exit(128);
  }
  
 -static void am_signoff(struct strbuf *sb)
 +/**
 + * Appends signoff to the "msg" field of the am_state.
 + */
 +static void am_append_signoff(struct am_state *state)
  {
        char *cp;
        struct strbuf mine = STRBUF_INIT;
 +      struct strbuf sb = STRBUF_INIT;
 +
 +      strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);
  
 -      /* Does it end with our own sign-off? */
 +      /* our sign-off */
        strbuf_addf(&mine, "\n%s%s\n",
                    sign_off_header,
                    fmt_name(getenv("GIT_COMMITTER_NAME"),
                             getenv("GIT_COMMITTER_EMAIL")));
 -      if (mine.len < sb->len &&
 -          !strcmp(mine.buf, sb->buf + sb->len - mine.len))
 +
 +      /* Does sb end with it already? */
 +      if (mine.len < sb.len &&
 +          !strcmp(mine.buf, sb.buf + sb.len - mine.len))
                goto exit; /* no need to duplicate */
  
        /* Does it have any Signed-off-by: in the text */
 -      for (cp = sb->buf;
 +      for (cp = sb.buf;
             cp && *cp && (cp = strstr(cp, sign_off_header)) != NULL;
             cp = strchr(cp, '\n')) {
 -              if (sb->buf == cp || cp[-1] == '\n')
 +              if (sb.buf == cp || cp[-1] == '\n')
                        break;
        }
  
 -      strbuf_addstr(sb, mine.buf + !!cp);
 +      strbuf_addstr(&sb, mine.buf + !!cp);
  exit:
        strbuf_release(&mine);
 -}
 -
 -/**
 - * Appends signoff to the "msg" field of the am_state.
 - */
 -static void am_append_signoff(struct am_state *state)
 -{
 -      struct strbuf sb = STRBUF_INIT;
 -
 -      strbuf_attach(&sb, state->msg, state->msg_len, state->msg_len);
 -      am_signoff(&sb);
        state->msg = strbuf_detach(&sb, &state->msg_len);
  }
  
@@@ -1320,6 -1321,9 +1320,6 @@@ static int parse_mail(struct am_state *
        strbuf_addbuf(&msg, &mi.log_message);
        strbuf_stripspace(&msg, 0);
  
 -      if (state->signoff)
 -              am_signoff(&msg);
 -
        assert(!state->author_name);
        state->author_name = strbuf_detach(&author_name, NULL);
  
@@@ -1372,33 -1376,40 +1372,33 @@@ static int get_mail_commit_oid(struct o
   */
  static void get_commit_info(struct am_state *state, struct commit *commit)
  {
 -      const char *buffer, *ident_line, *author_date, *msg;
 +      const char *buffer, *ident_line, *msg;
        size_t ident_len;
 -      struct ident_split ident_split;
 -      struct strbuf sb = STRBUF_INIT;
 +      struct ident_split id;
  
        buffer = logmsg_reencode(commit, NULL, get_commit_output_encoding());
  
        ident_line = find_commit_header(buffer, "author", &ident_len);
  
 -      if (split_ident_line(&ident_split, ident_line, ident_len) < 0) {
 -              strbuf_add(&sb, ident_line, ident_len);
 -              die(_("invalid ident line: %s"), sb.buf);
 -      }
 +      if (split_ident_line(&id, ident_line, ident_len) < 0)
 +              die(_("invalid ident line: %.*s"), (int)ident_len, ident_line);
  
        assert(!state->author_name);
 -      if (ident_split.name_begin) {
 -              strbuf_add(&sb, ident_split.name_begin,
 -                      ident_split.name_end - ident_split.name_begin);
 -              state->author_name = strbuf_detach(&sb, NULL);
 -      } else
 +      if (id.name_begin)
 +              state->author_name =
 +                      xmemdupz(id.name_begin, id.name_end - id.name_begin);
 +      else
                state->author_name = xstrdup("");
  
        assert(!state->author_email);
 -      if (ident_split.mail_begin) {
 -              strbuf_add(&sb, ident_split.mail_begin,
 -                      ident_split.mail_end - ident_split.mail_begin);
 -              state->author_email = strbuf_detach(&sb, NULL);
 -      } else
 +      if (id.mail_begin)
 +              state->author_email =
 +                      xmemdupz(id.mail_begin, id.mail_end - id.mail_begin);
 +      else
                state->author_email = xstrdup("");
  
 -      author_date = show_ident_date(&ident_split, DATE_MODE(NORMAL));
 -      strbuf_addstr(&sb, author_date);
        assert(!state->author_date);
 -      state->author_date = strbuf_detach(&sb, NULL);
 +      state->author_date = xstrdup(show_ident_date(&id, DATE_MODE(NORMAL)));
  
        assert(!state->msg);
        msg = strstr(buffer, "\n\n");
                die(_("unable to parse commit %s"), oid_to_hex(&commit->object.oid));
        state->msg = xstrdup(msg + 2);
        state->msg_len = strlen(state->msg);
 +      unuse_commit_buffer(commit, buffer);
  }
  
  /**
@@@ -1838,9 -1848,6 +1838,9 @@@ static void am_run(struct am_state *sta
                        if (skip)
                                goto next; /* mail should be skipped */
  
 +                      if (state->signoff)
 +                              am_append_signoff(state);
 +
                        write_author_script(state);
                        write_commit_msg(state);
                }
@@@ -2150,7 -2157,7 +2150,7 @@@ static void am_abort(struct am_state *s
        am_rerere_clear();
  
        curr_branch = resolve_refdup("HEAD", 0, curr_head.hash, NULL);
-       has_curr_head = !is_null_oid(&curr_head);
+       has_curr_head = curr_branch && !is_null_oid(&curr_head);
        if (!has_curr_head)
                hashcpy(curr_head.hash, EMPTY_TREE_SHA1_BIN);
  
                                has_curr_head ? &curr_head : NULL, 0,
                                UPDATE_REFS_DIE_ON_ERR);
        else if (curr_branch)
 -              delete_ref(curr_branch, NULL, REF_NODEREF);
 +              delete_ref(NULL, curr_branch, NULL, REF_NODEREF);
  
        free(curr_branch);
        am_destroy(state);
@@@ -2315,7 -2322,7 +2315,7 @@@ int cmd_am(int argc, const char **argv
  
        git_config(git_am_config, NULL);
  
 -      am_state_init(&state, git_path("rebase-apply"));
 +      am_state_init(&state);
  
        in_progress = am_in_progress(&state);
        if (in_progress)
diff --combined builtin/checkout.c
index bfa5419f335dc1db98059869de24f152cffb9aa6,5744499098e6c2a4c92879af9118ff3a2c6ba0ae..6c3d2e4f4cca135fa0d2562f2bcaa0af8a6aa340
  #include "submodule-config.h"
  #include "submodule.h"
  
 +static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 +
  static const char * const checkout_usage[] = {
        N_("git checkout [<options>] <branch>"),
        N_("git checkout [<options>] [<branch>] -- <file>..."),
        NULL,
  };
  
 +static int option_parse_recurse_submodules(const struct option *opt,
 +                                         const char *arg, int unset)
 +{
 +      if (unset) {
 +              recurse_submodules = RECURSE_SUBMODULES_OFF;
 +              return 0;
 +      }
 +      if (arg)
 +              recurse_submodules =
 +                      parse_update_recurse_submodules_arg(opt->long_name,
 +                                                          arg);
 +      else
 +              recurse_submodules = RECURSE_SUBMODULES_ON;
 +
 +      return 0;
 +}
 +
  struct checkout_opts {
        int patch_mode;
        int quiet;
@@@ -833,7 -814,8 +833,8 @@@ static int switch_branches(const struc
        int flag, writeout_error = 0;
        memset(&old, 0, sizeof(old));
        old.path = path_to_free = resolve_refdup("HEAD", 0, rev.hash, &flag);
-       old.commit = lookup_commit_reference_gently(rev.hash, 1);
+       if (old.path)
+               old.commit = lookup_commit_reference_gently(rev.hash, 1);
        if (!(flag & REF_ISSYMREF))
                old.path = NULL;
  
@@@ -908,10 -890,11 +909,10 @@@ static int check_tracking_name(struct r
  static const char *unique_tracking_name(const char *name, struct object_id *oid)
  {
        struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
 -      char src_ref[PATH_MAX];
 -      snprintf(src_ref, PATH_MAX, "refs/heads/%s", name);
 -      cb_data.src_ref = src_ref;
 +      cb_data.src_ref = xstrfmt("refs/heads/%s", name);
        cb_data.dst_oid = oid;
        for_each_remote(check_tracking_name, &cb_data);
 +      free(cb_data.src_ref);
        if (cb_data.unique)
                return cb_data.dst_ref;
        free(cb_data.dst_ref);
@@@ -1181,9 -1164,6 +1182,9 @@@ int cmd_checkout(int argc, const char *
                                N_("second guess 'git checkout <no-such-branch>'")),
                OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees,
                         N_("do not check if another worktree is holding the given ref")),
 +              { OPTION_CALLBACK, 0, "recurse-submodules", &recurse_submodules,
 +                          "checkout", "control recursive updating of submodules",
 +                          PARSE_OPT_OPTARG, option_parse_recurse_submodules },
                OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")),
                OPT_END(),
        };
                git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
        }
  
 +      if (recurse_submodules != RECURSE_SUBMODULES_OFF) {
 +              git_config(submodule_config, NULL);
 +              if (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)
 +                      set_config_update_recurse_submodules(recurse_submodules);
 +      }
 +
        if ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)
                die(_("-b, -B and --orphan are mutually exclusive"));