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

From: Junio C Hamano <[email-masked]>
Date: Wed, 21 Sep 2022 15:24:58 -0700

"Johannes Schindelin via GitGitGadget" <[email-masked]>
writes:

> +	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-<.