From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 25 Aug 2010 21:54:04 GMT
Subject: Re: [PATCH 2/2] builtin/fetch.c: ignore merge config when not fetching from branch's remote
Message-ID: <7vtymipddv.fsf@alter.siamese.dyndns.org>
In-Reply-To: <O7UxM6KEqdDAhjJAF7ODSlo_kZavb8gBCJ6laH3QPOlG9a1q29koMQOkS7wDMj0BpyrLYfAcEh4@cipher.nrlssc.navy.mil>

Brandon Casey <casey@nrlssc.navy.mil> writes:

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index ea14d5d..be6c27a 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -146,7 +146,8 @@ static struct ref *get_ref_map(struct transport *transport,
>  		struct remote *remote = transport->remote;
>  		struct branch *branch = branch_get(NULL);
>  		int has_merge = branch_has_merge_config(branch);
> -		if (remote && (remote->fetch_refspec_nr || has_merge)) {
> +		if (remote && (remote->fetch_refspec_nr || (has_merge &&
> +				!strcmp(branch->remote_name, remote->name)))) {

Couldn't branch->remote_name or remote->name be NULL here?

I think remote->name would be the same as the URL given on the command
line (i.e. no risk of being NULL), and has_merge would be false even when
branch.<name>.merge is specified if branch.<name>.remote is missing
(i.e. no risk of running this strcmp() to begin with), but the latter
safety guarantee is a bit too subtle for my taste to go without a in-code
comment.

Thanks.