Re: [PATCH] Limit filename for format-patch

From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Feb 23, 2007, 21:39 UTC
Message-ID: <200702232239.19504.robin.rosenberg@dewire.com>
In-Reply-To: <Pine.LNX.4.63.0702230143370.22628@wbgn013.biozentrum.uni-wuerzburg.de>

fredag 23 februari 2007 01:49 skrev Johannes Schindelin:

Show 12 quoted lines

> Hi,
> 
> I agree that your patch fixes a long-time issue.
> 
> On Fri, 23 Feb 2007, Robin Rosenberg wrote:
> 
> > +		if (strlen(output_directory) > sizeof filename - suffix_len)
> 
> I know that "sizeof filename" works, but in git.git, `git grep sizeof' 
> returns > 700 instances, only 23 of which do not use the 
> "sizeof(filename)" form. It's just a style issue, but I prefer the latter 
> nevertheless...

Actually there are (only) about 500 of them that are not the sizeof(type) kind.

I prefer not to use extra parentheses except for grouping complex expressions for readability.

Show 16 quoted lines

> >  		for (j = 0;
> 
> Does this:
> 
> > +		     j< 64-suffix_len-5 && 
> >  		     len < sizeof(filename) - suffix_len &&
> >  			     sol[j] && sol[j] != '\n';
> >  		     j++) {
> 
> not make this:
> 
> > +	if (len + strlen(fmt_patch_suffix) >= sizeof filename)
> > +		return error("Patch pathname too long");
> 
> unnecessary for the case that there _was_ an output directory specified? 
> I'd make that an "else if"... But I might be missing something.

The last statement errs out if the total length is too long which is a different issue than truncating the patch name. There is no reason to two paths when one does perfectly well. But I could replace strlen there.

-- robin