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. > 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. :) > Both are Signed-off-by: Steve Beattie <[email protected]>. Acked-by: Seth Arnold <[email protected]> for the first one, whatever you sed the name to :) Thanks > > Thanks. > -- > Steve Beattie > <[email protected]> > http://NxNW.org/~steve/ > Subject: C tools: rename __unused macro > > 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, as well as getting rid of > it from the mod_apparmor codebase, where it is unneeded. > > (For comparison, swig and bison also use custom macro names in > their generated code to avoid name collisions, SWIGUNUSED (and > SWIGUNUSEDPARM), and YY_ATTRIBUTE_UNUSED respectively.) > > Confirmed to build on Ubuntu 12.04, 14.04, and 14.10. > > Signed-off-by: Steve Beattie <[email protected]> > --- > changehat/mod_apparmor/mod_apparmor.c | 3 --- > parser/af_rule.h | 2 +- > parser/af_unix.h | 2 +- > parser/dbus.h | 2 +- > parser/mount.cc | 2 +- > parser/mount.h | 4 ++-- > parser/parser.h | 4 ++-- > parser/parser_alias.c | 4 ++-- > parser/parser_lex.l | 2 +- > parser/parser_main.c | 6 +++--- > parser/parser_misc.c | 6 +++--- > parser/parser_symtab.c | 6 +++--- > parser/ptrace.h | 2 +- > parser/signal.h | 2 +- > 14 files changed, 22 insertions(+), 25 deletions(-) > > Index: b/changehat/mod_apparmor/mod_apparmor.c > =================================================================== > --- a/changehat/mod_apparmor/mod_apparmor.c > +++ b/changehat/mod_apparmor/mod_apparmor.c > @@ -29,9 +29,6 @@ > #include <unistd.h> > > /* #define DEBUG */ > -#ifndef __unused > -#define __unused __attribute__((unused)) > -#endif > > /* should the following be configurable? */ > #define DEFAULT_HAT "HANDLING_UNTRUSTED_INPUT" > Index: b/parser/mount.h > =================================================================== > --- a/parser/mount.h > +++ b/parser/mount.h > @@ -127,7 +127,7 @@ public: > int deny; > > mnt_rule(struct cond_entry *src_conds, char *device_p, > - struct cond_entry *dst_conds __unused, char *mnt_point_p, > + struct cond_entry *dst_conds aa_unused, char *mnt_point_p, > int allow_p); > virtual ~mnt_rule() > { > @@ -141,7 +141,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof __unused); > + virtual void post_process(Profile &prof aa_unused); > }; > > int is_valid_mnt_cond(const char *name, int src); > Index: b/parser/parser.h > =================================================================== > --- a/parser/parser.h > +++ b/parser/parser.h > @@ -183,8 +183,8 @@ extern int preprocess_only; > #define MIN_PORT 0 > #define MAX_PORT 65535 > > -#ifndef __unused > -#define __unused __attribute__ ((unused)) > +#ifndef aa_unused > +#define aa_unused __attribute__ ((unused)) > #endif > > > Index: b/parser/af_rule.h > =================================================================== > --- a/parser/af_rule.h > +++ b/parser/af_rule.h > @@ -72,7 +72,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof) = 0; > - virtual void post_process(Profile &prof __unused) { }; > + virtual void post_process(Profile &prof aa_unused) { }; > }; > > #endif /* __AA_AF_RULE_H */ > Index: b/parser/af_unix.h > =================================================================== > --- a/parser/af_unix.h > +++ b/parser/af_unix.h > @@ -57,7 +57,7 @@ public: > virtual ostream &dump_peer(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof __unused) { }; > + virtual void post_process(Profile &prof aa_unused) { }; > }; > > #endif /* __AA_AF_UNIX_H */ > Index: b/parser/dbus.h > =================================================================== > --- a/parser/dbus.h > +++ b/parser/dbus.h > @@ -57,7 +57,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof __unused) { }; > + virtual void post_process(Profile &prof aa_unused) { }; > > > }; > Index: b/parser/signal.h > =================================================================== > --- a/parser/signal.h > +++ b/parser/signal.h > @@ -52,7 +52,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof __unused) { }; > + virtual void post_process(Profile &prof aa_unused) { }; > }; > > #endif /* __AA_SIGNAL_H */ > Index: b/parser/ptrace.h > =================================================================== > --- a/parser/ptrace.h > +++ b/parser/ptrace.h > @@ -46,7 +46,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof __unused) { }; > + virtual void post_process(Profile &prof aa_unused) { }; > }; > > #endif /* __AA_PTRACE_H */ > Index: b/parser/parser_lex.l > =================================================================== > --- a/parser/parser_lex.l > +++ b/parser/parser_lex.l > @@ -119,7 +119,7 @@ struct cb_struct { > const char *filename; > }; > > -static int include_dir_cb(__unused DIR *dir, const char *name, struct stat > *st, > +static int include_dir_cb(DIR *dir aa_unused, const char *name, struct stat > *st, > void *data) > { > struct cb_struct *d = (struct cb_struct *) data; > Index: b/parser/parser_main.c > =================================================================== > --- a/parser/parser_main.c > +++ b/parser/parser_main.c > @@ -1026,7 +1026,7 @@ out: > } > > /* data - name of parent dir */ > -static int profile_dir_cb(__unused DIR *dir, const char *name, struct stat > *st, > +static int profile_dir_cb(DIR *dir aa_unused, const char *name, struct stat > *st, > void *data) > { > int rc = 0; > @@ -1043,7 +1043,7 @@ static int profile_dir_cb(__unused DIR * > } > > /* data - name of parent dir */ > -static int binary_dir_cb(__unused DIR *dir, const char *name, struct stat > *st, > +static int binary_dir_cb(DIR *dir aa_unused, const char *name, struct stat > *st, > void *data) > { > int rc = 0; > @@ -1060,7 +1060,7 @@ static int binary_dir_cb(__unused DIR *d > } > > static int clear_cache_cb(DIR *dir, const char *path, struct stat *st, > - __unused void *data) > + void *data aa_unused) > { > /* remove regular files */ > if (S_ISREG(st->st_mode)) > Index: b/parser/mount.cc > =================================================================== > --- a/parser/mount.cc > +++ b/parser/mount.cc > @@ -386,7 +386,7 @@ static struct value_list *extract_option > } > > mnt_rule::mnt_rule(struct cond_entry *src_conds, char *device_p, > - struct cond_entry *dst_conds __unused, char *mnt_point_p, > + struct cond_entry *dst_conds aa_unused, char *mnt_point_p, > int allow_p): > mnt_point(mnt_point_p), device(device_p), trans(NULL), opts(NULL), > flags(0), inv_flags(0), audit(0), deny(0) > Index: b/parser/parser_alias.c > =================================================================== > --- a/parser/parser_alias.c > +++ b/parser/parser_alias.c > @@ -106,7 +106,7 @@ static char *do_alias(struct alias_rule > > static Profile *target_prof; > static struct cod_entry *target_list; > -static void process_entries(const void *nodep, VISIT value, int __unused > level) > +static void process_entries(const void *nodep, VISIT value, int level > aa_unused) > { > struct alias_rule **t = (struct alias_rule **) nodep; > struct cod_entry *entry, *dup = NULL; > @@ -150,7 +150,7 @@ static void process_entries(const void * > } > } > > -static void process_name(const void *nodep, VISIT value, int __unused level) > +static void process_name(const void *nodep, VISIT value, int level aa_unused) > { > struct alias_rule **t = (struct alias_rule **) nodep; > Profile *prof = target_prof; > Index: b/parser/parser_misc.c > =================================================================== > --- a/parser/parser_misc.c > +++ b/parser/parser_misc.c > @@ -183,7 +183,7 @@ static struct keyword_table rlimit_table > }; > > /* for alpha matches, check for keywords */ > -static int get_table_token(const char *name __unused, struct keyword_table > *table, > +static int get_table_token(const char *name aa_unused, struct keyword_table > *table, > const char *keyword) > { > int i; > @@ -342,7 +342,7 @@ void warn_uppercase(void) > } > } > > -static int parse_sub_mode(const char *str_mode, const char *mode_desc > __unused) > +static int parse_sub_mode(const char *str_mode, const char *mode_desc > aa_unused) > { > > #define IS_DIFF_QUAL(mode, q) (((mode) & AA_MAY_EXEC) && (((mode) & > AA_EXEC_TYPE) != ((q) & AA_EXEC_TYPE))) > @@ -529,7 +529,7 @@ int parse_mode(const char *str_mode) > return mode; > } > > -static int parse_X_sub_mode(const char *X, const char *str_mode, int > *result, int fail, const char *mode_desc __unused) > +static int parse_X_sub_mode(const char *X, const char *str_mode, int > *result, int fail, const char *mode_desc aa_unused) > { > int mode = 0; > const char *p; > Index: b/parser/parser_symtab.c > =================================================================== > --- a/parser/parser_symtab.c > +++ b/parser/parser_symtab.c > @@ -495,7 +495,7 @@ out: > return retval; > } > > -static void expand_variable(const void *nodep, VISIT value, int __unused > level) > +static void expand_variable(const void *nodep, VISIT value, int level > aa_unused) > { > struct symtab **t = (struct symtab **) nodep; > > @@ -547,7 +547,7 @@ static void __dump_symtab_entry(struct s > } > } > > -static void dump_symtab_entry(const void *nodep, VISIT value, int __unused > level) > +static void dump_symtab_entry(const void *nodep, VISIT value, int level > aa_unused) > { > struct symtab **t = (struct symtab **) nodep; > > @@ -557,7 +557,7 @@ static void dump_symtab_entry(const void > __dump_symtab_entry(*t, 0); > } > > -static void dump_expanded_symtab_entry(const void *nodep, VISIT value, int > __unused level) > +static void dump_expanded_symtab_entry(const void *nodep, VISIT value, int > level aa_unused) > { > struct symtab **t = (struct symtab **) nodep; > > --- > parser/af_rule.h | 2 +- > parser/af_unix.h | 2 +- > parser/dbus.h | 2 +- > parser/mount.cc | 2 +- > parser/mount.h | 4 ++-- > parser/parser.h | 3 +++ > parser/parser_alias.c | 4 ++-- > parser/parser_lex.l | 2 +- > parser/parser_main.c | 6 +++--- > parser/parser_misc.c | 6 +++--- > parser/parser_symtab.c | 6 +++--- > parser/ptrace.h | 2 +- > parser/signal.h | 2 +- > 13 files changed, 23 insertions(+), 20 deletions(-) > > Index: b/parser/parser.h > =================================================================== > --- a/parser/parser.h > +++ b/parser/parser.h > @@ -186,6 +186,9 @@ extern int preprocess_only; > #ifndef aa_unused > #define aa_unused __attribute__ ((unused)) > #endif > +#ifndef aa_unused_arg > +#define aa_unused_arg(x) (x) aa_unused > +#endif > > > #define list_for_each(LIST, ENTRY) \ > Index: b/parser/parser_alias.c > =================================================================== > --- a/parser/parser_alias.c > +++ b/parser/parser_alias.c > @@ -106,7 +106,7 @@ static char *do_alias(struct alias_rule > > static Profile *target_prof; > static struct cod_entry *target_list; > -static void process_entries(const void *nodep, VISIT value, int level > aa_unused) > +static void process_entries(const void *nodep, VISIT value, int > aa_unused_arg(level)) > { > struct alias_rule **t = (struct alias_rule **) nodep; > struct cod_entry *entry, *dup = NULL; > @@ -150,7 +150,7 @@ static void process_entries(const void * > } > } > > -static void process_name(const void *nodep, VISIT value, int level aa_unused) > +static void process_name(const void *nodep, VISIT value, int > aa_unused_arg(level)) > { > struct alias_rule **t = (struct alias_rule **) nodep; > Profile *prof = target_prof; > Index: b/parser/mount.cc > =================================================================== > --- a/parser/mount.cc > +++ b/parser/mount.cc > @@ -386,7 +386,7 @@ static struct value_list *extract_option > } > > mnt_rule::mnt_rule(struct cond_entry *src_conds, char *device_p, > - struct cond_entry *dst_conds aa_unused, char *mnt_point_p, > + struct cond_entry aa_unused_arg(*dst_conds), char > *mnt_point_p, > int allow_p): > mnt_point(mnt_point_p), device(device_p), trans(NULL), opts(NULL), > flags(0), inv_flags(0), audit(0), deny(0) > Index: b/parser/parser_symtab.c > =================================================================== > --- a/parser/parser_symtab.c > +++ b/parser/parser_symtab.c > @@ -495,7 +495,7 @@ out: > return retval; > } > > -static void expand_variable(const void *nodep, VISIT value, int level > aa_unused) > +static void expand_variable(const void *nodep, VISIT value, int > aa_unused_arg(level)) > { > struct symtab **t = (struct symtab **) nodep; > > @@ -547,7 +547,7 @@ static void __dump_symtab_entry(struct s > } > } > > -static void dump_symtab_entry(const void *nodep, VISIT value, int level > aa_unused) > +static void dump_symtab_entry(const void *nodep, VISIT value, int > aa_unused_arg(level)) > { > struct symtab **t = (struct symtab **) nodep; > > @@ -557,7 +557,7 @@ static void dump_symtab_entry(const void > __dump_symtab_entry(*t, 0); > } > > -static void dump_expanded_symtab_entry(const void *nodep, VISIT value, int > level aa_unused) > +static void dump_expanded_symtab_entry(const void *nodep, VISIT value, int > aa_unused_arg(level)) > { > struct symtab **t = (struct symtab **) nodep; > > Index: b/parser/mount.h > =================================================================== > --- a/parser/mount.h > +++ b/parser/mount.h > @@ -127,7 +127,7 @@ public: > int deny; > > mnt_rule(struct cond_entry *src_conds, char *device_p, > - struct cond_entry *dst_conds aa_unused, char *mnt_point_p, > + struct cond_entry aa_unused_arg(*dst_conds), char > *mnt_point_p, > int allow_p); > virtual ~mnt_rule() > { > @@ -141,7 +141,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof aa_unused); > + virtual void post_process(Profile aa_unused_arg(&prof)); > }; > > int is_valid_mnt_cond(const char *name, int src); > Index: b/parser/af_rule.h > =================================================================== > --- a/parser/af_rule.h > +++ b/parser/af_rule.h > @@ -72,7 +72,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof) = 0; > - virtual void post_process(Profile &prof aa_unused) { }; > + virtual void post_process(Profile aa_unused_arg(&prof)) { }; > }; > > #endif /* __AA_AF_RULE_H */ > Index: b/parser/af_unix.h > =================================================================== > --- a/parser/af_unix.h > +++ b/parser/af_unix.h > @@ -57,7 +57,7 @@ public: > virtual ostream &dump_peer(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof aa_unused) { }; > + virtual void post_process(Profile aa_unused_arg(&prof)) { }; > }; > > #endif /* __AA_AF_UNIX_H */ > Index: b/parser/parser_lex.l > =================================================================== > --- a/parser/parser_lex.l > +++ b/parser/parser_lex.l > @@ -119,7 +119,7 @@ struct cb_struct { > const char *filename; > }; > > -static int include_dir_cb(DIR *dir aa_unused, const char *name, struct stat > *st, > +static int include_dir_cb(DIR aa_unused_arg(*dir), const char *name, struct > stat *st, > void *data) > { > struct cb_struct *d = (struct cb_struct *) data; > Index: b/parser/ptrace.h > =================================================================== > --- a/parser/ptrace.h > +++ b/parser/ptrace.h > @@ -46,7 +46,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof aa_unused) { }; > + virtual void post_process(Profile aa_unused_arg(&prof)) { }; > }; > > #endif /* __AA_PTRACE_H */ > Index: b/parser/dbus.h > =================================================================== > --- a/parser/dbus.h > +++ b/parser/dbus.h > @@ -57,7 +57,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof aa_unused) { }; > + virtual void post_process(Profile aa_unused_arg(&prof)) { }; > > > }; > Index: b/parser/signal.h > =================================================================== > --- a/parser/signal.h > +++ b/parser/signal.h > @@ -52,7 +52,7 @@ public: > virtual ostream &dump(ostream &os); > virtual int expand_variables(void); > virtual int gen_policy_re(Profile &prof); > - virtual void post_process(Profile &prof aa_unused) { }; > + virtual void post_process(Profile aa_unused_arg(&prof)) { }; > }; > > #endif /* __AA_SIGNAL_H */ > Index: b/parser/parser_main.c > =================================================================== > --- a/parser/parser_main.c > +++ b/parser/parser_main.c > @@ -1026,7 +1026,7 @@ out: > } > > /* data - name of parent dir */ > -static int profile_dir_cb(DIR *dir aa_unused, const char *name, struct stat > *st, > +static int profile_dir_cb(DIR aa_unused_arg(*dir), const char *name, struct > stat *st, > void *data) > { > int rc = 0; > @@ -1043,7 +1043,7 @@ static int profile_dir_cb(DIR *dir aa_un > } > > /* data - name of parent dir */ > -static int binary_dir_cb(DIR *dir aa_unused, const char *name, struct stat > *st, > +static int binary_dir_cb(DIR aa_unused_arg(*dir), const char *name, struct > stat *st, > void *data) > { > int rc = 0; > @@ -1060,7 +1060,7 @@ static int binary_dir_cb(DIR *dir aa_unu > } > > static int clear_cache_cb(DIR *dir, const char *path, struct stat *st, > - void *data aa_unused) > + void aa_unused_arg(*data)) > { > /* remove regular files */ > if (S_ISREG(st->st_mode)) > Index: b/parser/parser_misc.c > =================================================================== > --- a/parser/parser_misc.c > +++ b/parser/parser_misc.c > @@ -183,7 +183,7 @@ static struct keyword_table rlimit_table > }; > > /* for alpha matches, check for keywords */ > -static int get_table_token(const char *name aa_unused, struct keyword_table > *table, > +static int get_table_token(const char aa_unused_arg(*name), struct > keyword_table *table, > const char *keyword) > { > int i; > @@ -342,7 +342,7 @@ void warn_uppercase(void) > } > } > > -static int parse_sub_mode(const char *str_mode, const char *mode_desc > aa_unused) > +static int parse_sub_mode(const char *str_mode, const char > aa_unused_arg(*mode_desc)) > { > > #define IS_DIFF_QUAL(mode, q) (((mode) & AA_MAY_EXEC) && (((mode) & > AA_EXEC_TYPE) != ((q) & AA_EXEC_TYPE))) > @@ -529,7 +529,7 @@ int parse_mode(const char *str_mode) > return mode; > } > > -static int parse_X_sub_mode(const char *X, const char *str_mode, int > *result, int fail, const char *mode_desc aa_unused) > +static int parse_X_sub_mode(const char *X, const char *str_mode, int > *result, int fail, const char aa_unused_arg(*mode_desc)) > { > int mode = 0; > const char *p; > -- > AppArmor mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/apparmor
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
