On 4/20/2023 3:14 AM, Elijah Newren via GitGitGadget wrote: > From: Elijah Newren <newren@xxxxxxxxx> > While at it, ensure the FREE_AND_NULL() in the function does something > useful with the nulling aspect, namely sets result->priv to NULL rather > than a mere temporary. Good call. It also makes the code look better. > void merge_finalize(struct merge_options *opt, > struct merge_result *result) > { > - struct merge_options_internal *opti = result->priv; > - > if (opt->renormalize) > git_attr_set_direction(GIT_ATTR_CHECKIN); > assert(opt->priv == NULL); > > - clear_or_reinit_internal_opts(opti, 0); > - FREE_AND_NULL(opti); > + if (!result->priv) > + return; > + clear_or_reinit_internal_opts(result->priv, 0); > + FREE_AND_NULL(result->priv); Perhaps this would be better as if (result->priv) { clear_or_reinit_internal_opts(result->priv, 0); FREE_AND_NULL(result->priv); } to avoid an accidental addition of code to the end of this method that doesn't depend on result->priv? Thanks, -Stolee