Re: [PATCH 1/2] remote.c: don't BUG() on 0-length branch names

[Jeff King <peff@peff.net>]

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