On 2/13/24 4:43 PM, Junio C Hamano wrote:
"Jeff Hostetler via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:From: Jeff Hostetler <jeffhostetler@xxxxxxxxxx> 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@xxxxxxxxxx> --- 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); + + 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; +}void adjust_dirname_case(struct index_state *istate, char *name)Missing inter-function blank line, before the new function. I wonder if we can avoid such repetition---the body of index_dir_exists() is 100% shared with this new function. Isn't it extremely unusual to receive "struct strbuf *" and call strbuf_init() on it? It means that the caller is expected to have a strbuf and pass a pointer to it, but also it is expected to leave the strbuf uninitialized. I'd understand if it calls strbuf_reset(), but it may not even be necessary, if we make it responsibility of the caller to pass a valid strbuf to be appended into. int index_dir_find(struct index_state *istate, const char *name, int namelen, struct strbuf *canonical_path) { struct dir_entry *dir; lazy_init_name_hash(istate); expand_to_path(istate, name, namelen, 0); dir = find_dir_entry(istate, name, namelen); if (canonical_path && dir && dir->nr) { // strbuf_reset(canonical_path); ??? strbuf_add(canonical_path, dir->name, dir->namelen); } return dir && dir->nr; } Then we can do #define index_dir_exists(i, n, l) index_dir_find((i), (n), (l), NULL) in the header for existing callers.
I'm always a little hesitant to change the signature of an existing function and chasing all of the callers in the middle of another task. It can sometimes be distracting to reviewers. I like your macro approach here. I'll do that in the next version. Thanks Jeff
