Re: [PATCH v5.1 3.5/4] revision: make helper for pathspec to bloom key

> 
> This diff is still bigger than I was hoping, so I'm sending a couple
> of patches that simplify this code movement. Feel free to ignore
> them as being too nit-picky.
> 
> --- >8 ---
> 
> From 69fa36dc615e140ae842b536f7da792beaebb272 Mon Sep 17 00:00:00 2001
> From: Derrick Stolee <stolee@gmail.com>
> Date: Thu, 10 Jul 2025 08:06:29 -0400
> Subject: [PATCH v5.1 3.5/4] revision: make helper for pathspec to bloom key
> 
> When preparing to use bloom filters in a revision walk, Git populates a
> boom_keyvec with an array of bloom keys for the components of a path.
> Before we create the ability to map multiple pathspecs to multiple
> bloom_keyvecs, extract the conversion from a pathspec to a bloom_keyvec
> into its own helper method. This simplifies the state that persists in
> prepare_to_use_bloom_filter() as well as makes the next change much
> simpler.
> 
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
> revision.c | 50 +++++++++++++++++++++++++++++++-------------------
> 1 file changed, 31 insertions(+), 19 deletions(-)
> 
> diff --git a/revision.c b/revision.c
> index 22bcfab7f93..4c09b594c55 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -687,14 +687,37 @@ static int forbid_bloom_filters(struct pathspec *spec)
> 
> static void release_revisions_bloom_keyvecs(struct rev_info *revs);
> 
> -static void prepare_to_use_bloom_filter(struct rev_info *revs)
> +static int convert_pathspec_to_filter(const struct pathspec_item *pi,
> +      struct bloom_keyvec **bloom_keyvec,
> +      const struct bloom_filter_settings *settings)
> {
> - struct pathspec_item *pi;
> - struct bloom_keyvec *bloom_keyvec;
> - char *path_alloc = NULL;
> - const char *path, *p;
> size_t len;
> + const char *path;
> + char *path_alloc = NULL;
> + int res = 0;
> +
> + /* remove single trailing slash from path, if needed */
> + if (pi->len > 0 && pi->match[pi->len - 1] == '/') {
> + path_alloc = xmemdupz(pi->match, pi->len - 1);
> + path = path_alloc;
> + } else
> + path = pi->match;
> +
> + len = strlen(path);
> + if (!len) {
> + res = -1;
> + goto cleanup;
> + }
> +
> + *bloom_keyvec = bloom_keyvec_new(path, len, settings);
> 
> +cleanup:
> + FREE_AND_NULL(path_alloc);

> + return res;
> +}
> +
> +static void prepare_to_use_bloom_filter(struct rev_info *revs)
> +{
> if (!revs->commits)
> return;
> 
> @@ -712,22 +735,12 @@ static void prepare_to_use_bloom_filter(struct rev_info *revs)
> 
> revs->bloom_keyvecs_nr = 1;
> CALLOC_ARRAY(revs->bloom_keyvecs, 1);
> - pi = &revs->pruning.pathspec.items[0];
> 
> - /* remove single trailing slash from path, if needed */
> - if (pi->len > 0 && pi->match[pi->len - 1] == '/') {
> - path_alloc = xmemdupz(pi->match, pi->len - 1);
> - path = path_alloc;
> - } else
> - path = pi->match;
> -
> - len = strlen(path);
> - if (!len)
> + if (convert_pathspec_to_filter(&revs->pruning.pathspec.items[0],
> +       &revs->bloom_keyvecs[0],
> +       revs->bloom_filter_settings))
> goto fail;
> 
> - revs->bloom_keyvecs[0] =
> - bloom_keyvec_new(path, len, revs->bloom_filter_settings);
> -
> if (trace2_is_enabled() && !bloom_filter_atexit_registered) {
> atexit(trace2_bloom_filter_statistics_atexit);
> bloom_filter_atexit_registered = 1;
> @@ -737,7 +750,6 @@ static void prepare_to_use_bloom_filter(struct rev_info *revs)
> 
> fail:
> revs->bloom_filter_settings = NULL;
> - free(path_alloc);
> release_revisions_bloom_keyvecs(revs);
> }
> 
> -- 
> 2.47.2.vfs.0.2