Re: [PATCH] Limit filename for format-patch

[Robin Rosenberg <robin.rosenberg@dewire.com>]

fredag 23 februari 2007 01:49 skrev Johannes Schindelin:
> 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.

> >  		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