Re: [PATCH 2/2] remote: add remote_state to struct repository

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 7, 2021, 23:39 UTC
Message-ID: <xmqq4k9so15i.fsf@gitster.g>
In-Reply-To: <5316ac086d89065cd9bda38bd475f6699adc928e.1633633635.git.gitgitgadget@gmail.com>

"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -static void add_pushurl_alias(struct remote *remote, const char *url)
> +static void add_pushurl_alias(struct remote_state *remote_state,
> +			      struct remote *remote, const char *url)
>  {

I am not sure if this is a good interface. It allows a caller to obtain "struct remote *" instance from somewhere, and feed it with an instance of "struct remote_state *" that has nothing to do with the "struct remote *", no?

Show 17 quoted lines

> -static struct remote *make_remote(const char *name, int len)
> +static struct remote *make_remote(struct remote_state *remote_state,
> +				  const char *name, int len)
>  {
>  	struct remote *ret;
>  	struct remotes_hash_key lookup;
> @@ -147,7 +119,7 @@ static struct remote *make_remote(const char *name, int len)
>  	if (!len)
>  		len = strlen(name);
>  
> -	init_remotes_hash();
> +	init_remotes_hash(remote_state);
>  	lookup.str = name;
>  	lookup.len = len;
>  	hashmap_entry_init(&lookup_entry, memhash(name, len));
> @@ -173,6 +145,28 @@ static struct remote *make_remote(const char *name, int len)
>  	return ret;

Instead, shouldn't "struct remote *" _know_ which remote-state it came from?

I didn't look, but I suspect that there may be similar problems with other structures like "branch" in this change.

Thanks.