From: Junio C Hamano Date: Mon, 05 Oct 2026 16:31:22 GMT Subject: Re: [PATCH v2 2/2] merge: remember conflict labels Message-ID: In-Reply-To: <18bdf7df49dde2c8e7f73f3b46c656abb6b26293.1791206658.git.phillip.wood@dunelm.org.uk> Phillip Wood 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.