Re: [PATCH v4 3/4] merge: remove global variable head[]
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:52
Nguyễn Thái Ngọc Duy [off-list ref] writes:
quoted hunk
@@ -1012,9 +1014,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix) { unsigned char result_tree[20]; unsigned char stash[20]; + unsigned char head[20]; + struct commit *head_commit = NULL; struct strbuf buf = STRBUF_INIT; const char *head_arg; - int flag, head_invalid = 0, i; + int flag, i; int best_cnt = -1, merge_was_ok = 0, automerge_was_ok = 0; struct commit_list *common = NULL; const char *best_strategy = NULL, *wt_strategy = NULL;@@ -1030,8 +1034,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix) branch = resolve_ref("HEAD", head, 0, &flag); if (branch && !prefixcmp(branch, "refs/heads/")) branch += 11; - if (is_null_sha1(head)) - head_invalid = 1; + if (!is_null_sha1(head)) { + head_commit = lookup_commit(head); + if (!head_commit) + die(_("could not parse HEAD")); + }
Is this is_null_sha1() valid without first clearing head[]?
Also, would it be too much trouble and code churn to employ the same
strategy as my rewrite of your [1/4] and pass only head_commit around in
the call chain?
Because this is the way to set up head_commit in the first place, this
particular resolve_ref() -> is_null_sha1() chain is unavoidable and needs
to be written carefully, but after the
!head_commit === is_null_sha1(head) === is_initial_commit
invariant is established, I suspect that it would reduce the chance of
similar mistakes in later parts of the code if it can check and use only
one argument.