On Tue, May 31, 2022 at 11:12:33PM +0000, Glen Choo via GitGitGadget wrote: > Fix the bug by removing the convenience strlen functionality, so that > 0 means that the string is 0-length. We still insert a bogus branch name > into the hash map, but this will be fixed in a later commit. I think this is a good change, regardless of whether we take the second commit or not. These kind of "automagically run strlen() sometimes" interfaces are subtle, and I think have bitten us before. > diff --git a/remote.c b/remote.c > index 42a4e7106e1..cf7015ae8ab 100644 > --- a/remote.c > +++ b/remote.c > @@ -195,9 +195,6 @@ static struct branch *find_branch(struct remote_state *remote_state, > struct branches_hash_key lookup; > struct hashmap_entry lookup_entry, *e; > > - if (!len) > - len = strlen(name); > - > lookup.str = name; > lookup.len = len; > hashmap_entry_init(&lookup_entry, memhash(name, len)); This changes the behavior of find_branch() without changing its signature. So any topics in flight that use it might be subtly broken. I think that's probably OK in this instance, since it's a file-local static, and was added relatively recently (i.e., the blast radius is pretty small and unlikely). -Peff
