Re: [PATCH/RFCv3 2/2] receive-pack: don't pass non-existent refs to post-{receive,update} hooks in push deletions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]