Re: [PATCH v3 02/14] remote.c: refactor setup of branch->merge list

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

Jeff King <peff@peff.net> writes:

> When we call branch_get() to lookup or create a "struct
> branch", we make sure the "merge" field is filled in so that
> callers can access it. But the conditions under which we do
> so are a little confusing, and can lead to two funny
> situations:
> ...
> In addition to those two fixes, this patch pushes the "do we
> need to setup merge?" logic down into set_merge, where it is
> a bit easier to follow.

Nicely done.  Thanks.

>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  remote.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/remote.c b/remote.c
> index bec8b31..ac17e66 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1636,6 +1636,19 @@ static void set_merge(struct branch *ret)
>  	unsigned char sha1[20];
>  	int i;
>  
> +	if (!ret)
> +		return; /* no branch */
> +	if (ret->merge)
> +		return; /* already run */
> +	if (!ret->remote_name || !ret->merge_nr) {
> +		/*
> +		 * no merge config; let's make sure we don't confuse callers
> +		 * with a non-zero merge_nr but a NULL merge
> +		 */
> +		ret->merge_nr = 0;
> +		return;
> +	}
> +
>  	ret->merge = xcalloc(ret->merge_nr, sizeof(*ret->merge));
>  	for (i = 0; i < ret->merge_nr; i++) {
>  		ret->merge[i] = xcalloc(1, sizeof(**ret->merge));
> @@ -1660,11 +1673,9 @@ struct branch *branch_get(const char *name)
>  		ret = current_branch;
>  	else
>  		ret = make_branch(name, 0);
> -	if (ret && ret->remote_name) {
> +	if (ret && ret->remote_name)
>  		ret->remote = remote_get(ret->remote_name);
> -		if (ret->merge_nr)
> -			set_merge(ret);
> -	}
> +	set_merge(ret);
>  	return ret;
>  }