This is the mail archive of the
mailing list for the glibc project.
Re: [PATCH] vfprintf: validate nargs and positional offsets
- From: Roland McGrath <roland at hack dot frob dot com>
- To: Kees Cook <kees at outflux dot net>
- Cc: libc-alpha at sourceware dot org
- Date: Thu, 2 Feb 2012 14:15:12 -0800 (PST)
- Subject: Re: [PATCH] vfprintf: validate nargs and positional offsets
- References: <20120202215702.GC4979@outflux.net>
> 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. */
> +/* 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
> +# 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.