Re: [PATCH 02/12] name-hash: add index_dir_exists2()

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 15, 2024, 09:31 UTC
Message-ID: <Zc3aCWwJzawKYE7p@tanuki>
In-Reply-To: <3464545fe3feceb08408618c77a70cc95745bfe9.1707857541.git.gitgitgadget@gmail.com>

On Tue, Feb 13, 2024 at 08:52:11PM +0000, Jeff Hostetler via GitGitGadget wrote:

Show 34 quoted lines

> From: Jeff Hostetler <jeffhostetler@github.com>
> 
> Create a new version of index_dir_exists() to return the canonical
> spelling of the matched directory prefix.
> 
> The existing index_dir_exists() returns a boolean to indicate if
> there is a case-insensitive match in the directory name-hash, but
> it doesn't tell the caller the exact spelling of that match.
> 
> The new version also copies the matched spelling to a provided strbuf.
> This lets the caller, for example, then call index_name_pos() with the
> correct case to search the cache-entry array for the real insertion
> position.
> 
> Signed-off-by: Jeff Hostetler <jeffhostetler@github.com>
> ---
>  name-hash.c | 16 ++++++++++++++++
>  name-hash.h |  2 ++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/name-hash.c b/name-hash.c
> index 251f036eef6..d735c81acb3 100644
> --- a/name-hash.c
> +++ b/name-hash.c
> @@ -694,6 +694,22 @@ int index_dir_exists(struct index_state *istate, const char *name, int namelen)
>  	dir = find_dir_entry(istate, name, namelen);
>  	return dir && dir->nr;
>  }
> +int index_dir_exists2(struct index_state *istate, const char *name, int namelen,
> +		      struct strbuf *canonical_path)
> +{
> +	struct dir_entry *dir;
> +
> +	strbuf_init(canonical_path, namelen+1);

Missing spaces: `namelen + 1`.

Show 9 quoted lines

> +	lazy_init_name_hash(istate);
> +	expand_to_path(istate, name, namelen, 0);
> +	dir = find_dir_entry(istate, name, namelen);
> +
> +	if (dir && dir->nr)
> +		strbuf_add(canonical_path, dir->name, dir->namelen);
> +
> +	return dir && dir->nr;
> +}

Can we maybe give this function a more descriptive name? `index_dir_exists2()` doesn't give the reader any indicator what is different about it compared to `index_dir_exists()`. How about `index_dir_exists_with_canonical()`?

Show 12 quoted lines

>  void adjust_dirname_case(struct index_state *istate, char *name)
>  {
> diff --git a/name-hash.h b/name-hash.h
> index b1b4b0fb337..2fcac5c4870 100644
> --- a/name-hash.h
> +++ b/name-hash.h
> @@ -5,6 +5,8 @@ struct cache_entry;
>  struct index_state;
>  
>  int index_dir_exists(struct index_state *istate, const char *name, int namelen);
> +int index_dir_exists2(struct index_state *istate, const char *name, int namelen,
> +		      struct strbuf *canonical_path);

It would also be great to add comments here that explain what those functions do and what the difference between them is.

Patrick

Show 7 quoted lines

>  void adjust_dirname_case(struct index_state *istate, char *name);
>  struct cache_entry *index_file_exists(struct index_state *istate, const char *name, int namelen, int igncase);
>  
> -- 
> gitgitgadget
> 
>