Re: [PATCH] Fix segfault in merge-recursive

From: Junio C Hamano <gitster@pobox.com>
Date: May 8, 2009, 23:36 UTC
Message-ID: <7vocu3p3pr.fsf@alter.siamese.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.0905082229520.4601@intel-tinevez-2-302>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When there is no "common" tree (for whatever reason), we must not
> throw a segmentation fault.

You described why the old code was wrong (i.e. "init_tree_desc_from_tree is called with common == NULL"), but there is no mention why the new code is correct. For the purpose of satisfying the above statement, you could have just exit(0) as well ;-)

Show 7 quoted lines

> +	else {
> +		opts.merge = 0;
> +		opts.head_idx = 1;
> +		init_tree_desc_from_tree(t+0, head);
> +		init_tree_desc_from_tree(t+1, merge);
> +		rc = unpack_trees(2, t, &opts);
> +	}

This looks more like a half of branch-switch from HEAD to MERGE, not a merge between HEAD and MERGE as two equal histories. Shouldn't it be doing a three-way tree merge using an empty tree object as the common ancestor instead, just like merge_recursive.c::merge_recursive() itself does?