Re: [PATCH v4 3/4] merge: remove global variable head[]
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 23, 2011, 18:46 UTC
- Message-ID
- <7vty98q82k.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1313765407-29925-3-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 24 quoted lines
> @@ -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.