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

From: Alexander Potashev <[email-masked]>
Date: Fri, 2 Jan 2009 01:17:20 +0300

On 05:00 Thu 01 Jan     , Junio C Hamano wrote:
> Alexander Potashev <[email-masked]> 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