From: Junio C Hamano Date: Mon, 19 Apr 2010 20:39:32 GMT Subject: Re: [PATCH v2 2/2] receive-pack: detect aliased updates which can occur with symrefs Message-ID: <7veiibqirf.fsf@alter.siamese.dyndns.org> In-Reply-To: <1271695179-32131-1-git-send-email-jaysoffian@gmail.com> Jay Soffian writes: > @@ -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? > @@ -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.