Re: [PATCH] vfprintf: validate nargs and positional offsets

[Roland McGrath <roland at hack dot frob dot com>]

> I have FSF assignment via Google. (Sent from @outflux since that's how I'm
> subscribed here, but CL shows @chromium.org as part of my Google work.)

Posting here is not limited to subscribers.  
Send from whichever address you want to receive replies on.

> 	[BZ 13656]

The format is "[BZ #nnn]".

> 	* stdio-common/bug13656.c: New file.

I'm not sure we have any precedent for naming files for bugzilla numbers.
It might be a reasonable thing.  But the numbered ones we have are just
increasing numbers (so bug25 is next).  Personally I have something at
least vaguely descriptive in the name (e.g. bug-vfprintf-fortify-nargs),
though I'll bow to consensus on that.

> +   02111-1307 USA.  */

Blank line here.

> +  if (sprintf (output, fmt, 1, 2, 3, "test") > 0 &&
> +      strcmp (output, expected) != 0)

Operator goes on the second line when a clause is line-wrapped like this.

> +  /* Check behavior of 32bit positional overflow.  */

Say "32-bit".

> +/* Positional arguments are constructed via read_int(), so nargs

I personally hate the convention of appending () to a function name when
referring to it.  Comments are written in English, with normal punctuation
standards.  But people often do it, so that's not a blocker, just a pet
peeve.

> +# define EXPECTED_SIGNAL 11

Use SIGSEGV, not the integer literal.

> +    /* Check for potential integer overflow. */

Third time's the charm.

I'm being extremely pedantic just because you are a new contributor and I
want to teach all the conventions for future reference.  We are often
looser about some of this stuff, especially in test cases.


Thanks,
Roland