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". 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. Both are Signed-off-by: Steve Beattie <[email protected]>. 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;
signature.asc
Description: Digital signature
-- AppArmor mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
