Re: [RFC PATCH] builtin-apply: prevent non-explicit permission changes

From: Alexander Potashev <aspotashev@gmail.com>
Date: Jan 1, 2009, 22:17 UTC
Message-ID: <20090101221720.GA5603@myhost>
In-Reply-To: <7vfxk3npuc.fsf@gitster.siamese.dyndns.org>

On 05:00 Thu 01 Jan     , Junio C Hamano wrote:

Show 26 quoted lines

> Alexander Potashev <aspotashev@gmail.com> writes:
> 
> >  builtin-apply.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/builtin-apply.c b/builtin-apply.c
> > index 07244b0..071f6d8 100644
> > --- a/builtin-apply.c
> > +++ b/builtin-apply.c
> > @@ -630,7 +630,7 @@ static int gitdiff_index(const char *line, struct patch *patch)
> >  	memcpy(patch->new_sha1_prefix, line, len);
> >  	patch->new_sha1_prefix[len] = 0;
> >  	if (*ptr == ' ')
> > -		patch->new_mode = patch->old_mode = strtoul(ptr+1, NULL, 8);
> > +		patch->old_mode = strtoul(ptr+1, NULL, 8);
> >  	return 0;
> >  }
> >  
> > @@ -2447,6 +2447,7 @@ static int check_preimage(struct patch *patch, struct cache_entry **ce, struct s
> >  	if (st_mode != patch->old_mode)
> >  		fprintf(stderr, "warning: %s has type %o, expected %o\n",
> >  			old_name, st_mode, patch->old_mode);
> > +	patch->new_mode = st_mode;
> 
> Can you do this unconditionally, overwriting whatever we read from the
> patch header metainfo lines?

Do you mean overwriting of 'patch->new_mode' right after patch parsing? If so, there would be yet another call to 'stat' to get the permissions of the existing file (that is not very good).

I'm not very familiar with Git sources.

Also, I don't understand what are the permissions in 'index ...' lines for (e.g. "index fc3c3a4..066a4ac 100644"), my patch simply drops them:

> > -		patch->new_mode = patch->old_mode = strtoul(ptr+1, NULL, 8);
> > +		patch->old_mode = strtoul(ptr+1, NULL, 8);

...not completely drops, probably we should cross out this line completely (I don't know whether it breaks something).

					Alexander