Re: [PATCH] status: store format option as an int

From: Jeff King <peff@peff.net>
Date: Apr 16, 2011, 06:22 UTC
Message-ID: <20110416062233.GA28853@sigill.intra.peff.net>
In-Reply-To: <20110416052704.GA10807@elie>

On Sat, Apr 16, 2011 at 12:27:04AM -0500, Jonathan Nieder wrote:

Show 13 quoted lines

> It is unsafe to pass a pointer to a value of enumerated type to
> OPT_SET_INT (as v1.7.0-rc0~137^2~14, 2009-0905) does, since it might
> have the wrong alignment or width (C99 only says "Each enumerated type
> shall be compatible with char, a signed integer type, or an unsigned
> integer type.  The choice of type is implementation-defined, but shall
> be capable of representing the values of all the members of the
> enumeration.)
> 
> Probably this didn't come up in practice because by default GCC uses
> an 'int' to represent small enums unless passed -fshort-enums (except
> on certain architectures where -fshort-enums is the default).
> 
> Noticed-by: Jeff King <peff@peff.net>

If by "noticed by" you mean "mentioned but was completely unaware of the significance of what he was saying", then yes. :)

Now that you mention it, though, I was reminded that we had run across something similar before. And I think it was this:

  http://article.gmane.org/gmane.comp.version-control.git/144858

Your fix looks sane. I don't think we can do anything more clever on the parse-options side.

-Peff