From: Jay Soffian Date: Mon, 19 Apr 2010 20:57:06 GMT Subject: Re: [PATCH v2 2/2] receive-pack: detect aliased updates which can occur with symrefs Message-ID: In-Reply-To: <7veiibqirf.fsf@alter.siamese.dyndns.org> On Mon, Apr 19, 2010 at 4:39 PM, Junio C Hamano wrote: >> -     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? Oh wow, the perils of coding before coffee. I wish I could call it a typo, it's a thinko which is much worse. >> @@ -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. Okay. > 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). I didn't realize you'd queued it before I sent out v2. But I'm not sure what you're asking for beyond what I already said in the email: - Reformatted commit message; minor rewording. - Detect situation where there is an inconsistent aliased update and give a better diagnostic than "failed to lock" - Add additional test case for inconsistent update situation j.