Re: [PATCH v2 2/2] receive-pack: detect aliased updates which can occur with symrefs

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 19, 2010, 20:39 UTC
Message-ID: <7veiibqirf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1271695179-32131-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

Show 12 quoted lines

> @@ -503,9 +562,11 @@ static void execute_commands(struct command *commands, const char *unpacker_erro
>  		return;
>  	}
>  
> +	check_aliased_updates(commands);
> +
>  	head_name = resolve_ref("HEAD", sha1, 0, NULL);
>  
> -	for (cmd = commands; cmd; cmd = cmd->next)
> +	for (cmd = commands; cmd && !cmd->skip_update; cmd = cmd->next)
>  		cmd->error_string = update(cmd);
>  }

Do you really mean to have "skip-update" check as the loop termination condition like this (i.e. "upon seeing any skip-update, abandon the rest of the update queue"), or is this a typo of "skip this entry but keep going, processing the rest" that should have been a separate "if" in the body of the loop?

Show 7 quoted lines

> @@ -545,6 +606,7 @@ static struct command *read_head_info(void)
>  		hashcpy(cmd->old_sha1, old_sha1);
>  		hashcpy(cmd->new_sha1, new_sha1);
>  		memcpy(cmd->ref_name, line + 82, len - 81);
> +		cmd->skip_update = 0;
>  		cmd->error_string = NULL;
>  		cmd->next = NULL;

It would make sense to do xcalloc(nmemb, size) of one member of that length to allocate cmd at this point, instead of adding yet another assignment like this.

It also would help me a slight bit if you compared what has been queued with what you sent to catch minor differences between my expectation from this series and what you have (e.g. I'd like to keep this as a fix that is back-mergeable to 'maint' and also I have already done some style fixes to the test).

Thanks.