Re: [PATCH v2 2/2] merge: remember conflict labels

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Note that merge_switch_to_result()
> we assign "result->priv" to "opt->priv" and later clear "opt->priv" in
> order to get a pointer to the private struct as result->priv is void*.

I missed this part.

>  		trace2_region_leave("merge", "write_auto_merge", opt->repo);
> +
> +		trace2_region_enter("merge", "write_merge_labels", opt->repo);
> +		opt->priv = result->priv;
> +		write_merge_labels(opt->repo, opt->priv->labels[0], opt->priv->labels[1],
> +				   opt->priv->labels[2]);
> +		opt->priv = NULL;
> +		trace2_region_leave("merge", "write_merge_labels", opt->repo);

Would it be better to do it this way instead?

	struct merge_options_internal *priv = result->priv;
	write_merge_labels(opt->repo,
			   priv->labels[0], priv->labels[1], priv->labels[2]);

Also, with the way merge labels are prepared and passed around, I
wonder if we should just tighten its function signature and take

	write_merge_labels(struct repository *repo, const char *labels[3])

so that this calling site becomes[*]

	struct merge_options_internal *priv = result->priv;
	write_merge_labels(opt->repo, priv->labels);



[Footnote]

 * Here, I deviate from the usual naming convention to call an array
   of things in singular (so the second label would become
   label[2]), because from the point of view of the API consumer,
   "labels" as a unit is what they pass around, and call it in
   plural.