[apparmor] [patch] C tools: rename __unused macro
Seth Arnold
seth.arnold at canonical.com
Tue Sep 23 19:35:01 UTC 2014
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 <steve at nxnw.org>.
Acked-by: Seth Arnold <seth.arnold at canonical.com>
for the first one, whatever you sed the name to :)
Thanks
>
> Thanks.
> --
> Steve Beattie
> <sbeattie at ubuntu.com>
> 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 <steve at nxnw.org>
> ---
> 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
> AppArmor at lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20140923/91860c47/attachment-0001.pgp>
More information about the AppArmor
mailing list