Re: [PATCH v2] merge-tree: fix segmentation fault in read-only repositories

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 21, 2022, 22:24 UTC
Message-ID: <xmqqfsgk1i2t.fsf@gitster.g>
In-Reply-To: <pull.1362.v2.git.1663798083240.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 11 quoted lines

> +	const struct object_id *tree_oid;
>  
>  	parent1 = get_merge_parent(branch1);
>  	if (!parent1)
> @@ -446,7 +447,8 @@ static int real_merge(struct merge_tree_options *o,
>  	if (o->show_messages == -1)
>  		o->show_messages = !result.clean;
>  
> -	printf("%s%c", oid_to_hex(&result.tree->object.oid), line_termination);
> +	tree_oid = result.tree ? &result.tree->object.oid : null_oid();
> +	printf("%s%c", oid_to_hex(tree_oid), line_termination);

As we are not using tree_oid anywhere else, it might be cleaner to switch on result.tree more explicitly, especially given that the return statement also uses result.tree to decide what exit status to use, e.g.

	if (result.tree)
		fputs(oid_to_hex(&result.tree->object.oid), stdout);
	else
		fputs(oid_to_hex(null_oid()), stdout);
	fputc(line_termination, stdout);
	
but that is merely "might be cleaner".

> -	return !result.clean; /* result.clean < 0 handled above */
> +	return !result.tree || !result.clean; /* result.clean < 0 handled above */

The original returned 1 when we failed to cleanly merge and 0 when we cleanly merged. Now, if we failed to come up with a merged tree, result.tree would be NULL and we return 1 from the function, because we failed to cleanly merge.

OK.  These negations make my head spin X-<.