Re: [PATCH] unpack-tree.c: remove dead code

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 12, 2014, 18:13 UTC
Message-ID: <xmqqha1h60fy.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1407786268-15249-1-git-send-email-stefanbeller@gmail.com>

Stefan Beller <stefanbeller@gmail.com> writes:

Show 12 quoted lines

> In line 1763 of unpack-tree.c we have a condition on the current tree
> 	if (current) {
> 		...
> Within this block of code we can assume current to be non NULL, hence
> the code after the statement in line 1796:
> 	if (current)
> 		return ...
>
> cannot be reached.
>
> The proposed patch here changes the order of the current tree and the
> newtree part. I'm not sure if that's the right way to handle it.

If the existing code decides to reject the merge and falls into that code path, src[0] aka current is not NULL at that point as you noticed, and we would call reject_merge(current, o); we would keep doing so after this "remove dead code" patch is applied.

If you remove the dead code, which are the inner check for current, reject_merge() call with newtree and the final fallback of returning -1, then it would be a faithful "remove dead code without changing anything else" update.

Having said that, I think current/newtree/oldtree are used in the call to reject_merge() *only* for their path aka ce->name, and they all point at the same name (there is no rename funkies here); hence "all other failures" code path should just rely on current always being present and become something like this instead:

		/* 20 or 21 */
                ...
	} else if (o->gently) {
        	return -1;
	} else {
        	return reject_merge(current, o);
	}

Thanks.