Re: [PATCH v5 3/4] branch: add --delete-integrated to delete rebase and squash merged branches

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 30, 2026, 19:14 UTC
Message-ID: <xmqq33tnvw58.fsf@gitster.g>
In-Reply-To: <bf30f57ceb77c5052a72aa7dc01e3bb163993a63.1793357582.git.gitgitgadget@gmail.com>

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 6 quoted lines

> +static void find_landed_commits(struct landing_search *searches,
> +				size_t searches_nr,
> +				struct strmap *landed_commits)
> +{
> +	struct changed_paths cache;
> +	struct diff_options opt;

A tiny nit, but because so many things are called x_option in our codebase, I am used to seeing the variable also carry "x" to match, or the use of mere "opt" down below would make us go back to see what kind of option it refers to while reading the code. I'd liked to see this called "diffopt" instead.

Show 6 quoted lines

> +	repo_diff_setup(the_repository, &opt);
> +	opt.flags.recursive = 1;
> +	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
> +	opt.no_free = 1;
> +	diff_setup_done(&opt);
> +	init_changed_paths(&cache);

The .no_free member is set to avoid the diff_free() call made in changed_path_of(), which in turn is called by find_landed_commit(). Everytime I see manipulation of .no_free member, it makes me look at the code twice but this seems to be a good use of the feature. And ...

Show 14 quoted lines

> +	for (size_t i = 0; i < searches_nr; i++) {
> +		struct commit *landed;
> +
> +		if (!searches[i].changes_nr)
> +			continue;
> +		landed = find_landed_commit(&searches[i], &cache, &opt);
> +		if (landed)
> +			strmap_put(landed_commits, searches[i].branch_name,
> +				   landed);
> +	}
> +
> +	deep_clear_changed_paths(&cache, free_changed_paths);
> +	opt.no_free = 0;
> +	diff_free(&opt);

... clearing the member before calling diff_free() to release the resources is also in line with the established practice.

I didn't have time to look at other parts of this patch (or other patches in the series), but the use of .no_free in this function looked OK.

Thanks.