Re: [PATCH v3 4/8] promisor-remote: add 'local_name' to 'struct promisor_info'

[Junio C Hamano <gitster@pobox.com>]

Christian Couder <christian.couder@gmail.com> writes:

>  struct promisor_info {
> -	const char *name;
> +	const char *name;	/* name the server advertised */
> +	const char *local_name;	/* name used locally (may be auto-generated) */

OK.

> @@ -449,6 +450,7 @@ struct promisor_info {
>  static void promisor_info_free(struct promisor_info *p)
>  {
>  	free((char *)p->name);
> +	free((char *)p->local_name);
>  	free((char *)p->url);
>  	free((char *)p->filter);
>  	free((char *)p->token);

Having to cast away constness is irritating, but to the users of the
struct it may be safer to mark the members const so that they do not
touch them, perhaps.  It is not a new problem with this patch but is
inherited from the existing code, so let's not worry too much about
it.

> +static const char *promisor_info_internal_name(struct promisor_info *p)
> +{
> +	return p->local_name ? p->local_name : p->name;
> +}

Hmph.

> @@ -829,7 +836,7 @@ static bool promisor_store_advertised_fields(struct promisor_info *advertised,
>  {
>  	struct promisor_info *p;
>  	struct string_list_item *item;
> -	const char *remote_name = advertised->name;
> +	const char *remote_name = promisor_info_internal_name(advertised);

Is this really a "remote_name", though?  As ...

> @@ -937,7 +944,8 @@ static void filter_promisor_remote(struct repository *repo,
>  	/* Apply accepted remotes to the stable repo state */
>  	for_each_string_list_item(item, accepted_remotes) {
>  		struct promisor_info *info = item->util;
> -		struct promisor_remote *r = repo_promisor_remote_find(repo, info->name);
> +		const char *local = promisor_info_internal_name(info);

... this name "local" is "the name the thing is locally known to
us", promisor_info_local_name() might be a better name?  I dunno.
I jsut found it odd that the return value of the same function is
stored in variables named "remote" and "local" at the same time ;-)