On Tue, Sep 23, 2014 at 12:35:01PM -0700, Seth Arnold wrote: > On Mon, Sep 22, 2014 at 06:55:26PM -0700, Steve Beattie wrote: > > On Thu, Sep 11, 2014 at 07:49:36PM +0200, Christian Boltz wrote: > > > Am Donnerstag, 11. September 2014 schrieb Steve Beattie: > > > > Bug: https://bugzilla.novell.com/show_bug.cgi?id=895495 > > > > > > > > We define the __unused macro as a shortcut for __attribute__((unused)) > > > > to quiet compiler warnings for functions where an argument is unused, > > > > for whatever reason. However, on 64 bit architectures, older glibc's > > > > bits/stat.h header defines an array variable with the name __unused > > > > that collides with our macro and causes the parser to fail to build, > > > > because the resulting macro expansion generates invalid C code. > > > > > > > > This patch renames the macro to __aa_unused, > > > > > > >From https://bugzilla.novell.com/show_bug.cgi?id=895495#c4 > > > > > > it's invalid to use the implementation namespace (two leading > > > underscores). > > > > > > That's exactly what we have - with or without your patch. The little > > > difference is that your patch adds an additional "aa" to make it less > > > likely to clash with existing names. > > > > > > Wouldn't it be a better idea to use something that does _not_ start with > > > two underscores? > > > > Yes and no. Yes because it could potentially clash with some > > compiler/system symbol that begins with __aa_, which, while unlikely is > > possible. No, because the use of underscores gives a hint to a code > > reader that it has a special meaning, and isn't just a weird complex > > type or variable. > > > > Anyway, attached is an updated version of the patch that converts it to > > aa_unused, as well as making the use of it consistent by having it be > > the last part of the variable declaration; i.e. "type var aa_unused", > > rather than "aa_unused type var" or "type aa_unused var". > > I preferred the __aa_unused to aa_unused; if we're going to follow the > letter of the spec and ditch the leading underscores I think I'd rather > follow the lead of other projects and go capitalized, e.g. AA_UNUSED, but > that's pretty deep in pink vs chartruse bikeshedding land, so meh.
Right, a common convention is to capitalize macros, to make them distinct from regular symbols. > > A second patch is attached that applies on top of that and adds a > > second macro function called aa_unused_arg(), that wraps the variable > > declaration name. so the above would be 'type aa_unused_arg(var)'. > > I don't have a strong feeling either way about the second patch. > > I don't like this one. Sorry. :) Not at all. Honestly, my preferences are: 1) __unused — simple, clear, straightforward annotation. Alas, conflicts with a symbol in older glibcs and with convention of __ prefixed symbols being system synbols. I wish gcc just went ahead and defined this. 2) __aa_unused — nearly as simple as __unused; the underscores indicate specialness, the __aa_ prefix makes it less likely to conflict with any actual system symbol name, despite intruding on the system symbol convention. NACK'ed by Christian, so would need at least one more ACK to be acceptable. (_aa_unused has the same problem, standards-wise,) 3) aa_unused — easy to confuse with the rest of an argument declaration. 4) aa_unused_arg() — mildly confusing in argument lists, but makes it more explicit that it's not to be confused with the type or name declaration. 5) AA_UNUSED — so shouty, hurts my ears and eyes. But follows the all-caps convention for macros, so is explicit about what it is. Sooo, a great big shrug from me. But I'm getting tired of jenkins nagging me that the parser fails to build on Ubuntu 12.04. -- Steve Beattie <[email protected]> http://NxNW.org/~steve/
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
