Re: [RFC PATCH] vsnprintf: Remove use of %n and convert existing uses

[Joe Perches <joe@perches.com>]

On Thu, 2013-09-12 at 01:19 +0100, Al Viro wrote:
> On Wed, Sep 11, 2013 at 05:04:17PM -0700, Joe Perches wrote:
> > On Thu, 2013-09-12 at 08:40 +0900, Tetsuo Handa wrote:
> > > Joe Perches wrote:
> > > > -	seq_printf(m, "%s%d%n", con->name, con->index, &len);
> > > > +	len = seq_printf(m, "%s%d", con->name, con->index);
> > > 
> > > Isn't len always 0 or -1 ?
> > 
> > Right.  Well you're no fun...
> > 
> > These uses would seem broken anyway because the
> > seq_printf isn't itself tested for correctness.
> > 
> > Hmm.
> > 
> > Also, there's a large amount of code that appears
> > to do calculations with pos or len like:
> > 
> > 	pos += seq_printf(handle, fmt. ...)
> 
> ... and most of that code proceeds to ignore pos completely.
> Note that ->show() is *NOT* supposed to return the number of
> characters it has/would like to have produced.  Just return
> 0 and be done with that; overflows are dealt with just fine.
> The large amount, BTW, is below 100 lines, AFAICS, in rather
> few files.

Unfortunately, when you count the uses of
	return seq_printf(...)
it's rather higher and all the callers need
to be chased down too.

$ grep -rP --include=*.[ch] "^[ \t]*(\S[ \t\S]*=|return[\s\(]*)\s*\bseq_[v]?printf\b" * | wc -l
320
$ grep -rPl --include=*.[ch] "^[ \t]*(\S[ \t\S]*=|return[\s\(]*)\s*\bseq_[v]?printf\b" *|wc -l
81

> Just bury the cargo-culting crap.  All those += seq_printf() should
> be simply calling it.

Most likely.

> The *only* reason to look at the return
> value is "if we'd already overflown the buffer, I'd rather skipped
> the costly generation of the rest of the record".  In that case
> seq_printf() returning -1 means "skip it, nothing else will fit and
> caller will be repeating with bigger buffer anyway".

Perhaps changing the seq_vprintf return from 0 to len
and testing for -1 would work.

Still would need to change a few lines in netfilter
and probably a few other places.