On Wed, Sep 28, 2011 at 03:37:32PM -0700, Junio C Hamano wrote: > Pang Yan Han <pangyanhan@xxxxxxxxx> writes: > > > +/* For invalid refs */ > > +static struct command **invalid_delete; > > +static size_t invalid_delete_nr; > > +static size_t invalid_delete_alloc; > > Do you have to have these separately only to keep track of the corner case > errors? I would have expected that it would be more natural to mark them > by adding a single bitfield to "struct command". Yes they are purely for keeping track of deleting non-existent refs. Ok I will add the bitfield to struct command. > > > @@ -447,6 +467,8 @@ static const char *update(struct command *cmd) > > if (!parse_object(old_sha1)) { > > rp_warning("Allowing deletion of corrupt ref."); > > old_sha1 = NULL; > > + if (!ref_exists((char *) name)) > > + invalid_delete_append(cmd); > > This is not an "invalid" delete but deleting a non-existing ref. Perhaps > you would want to move the warning and optionally reword it as well, e.g. > > if (!parse_object(old_sha1)) { > old_sha1 = NULL; > if (ref_exists(name)) { > rp_warning("Allowing deletion of corrupt ref."); > } else { > rp_warning("Deleting a ref that does not exist."); > cmd->did_not_exist = 1; > } > ... Sure thing. I am unable to reply this until much later, but are the tests in this patch ok? It's the first time I'm writing test cases for git. Thanks for the feedback! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html