* [PATCH] audit: Fix compile time warning on kernel/auditsc.c
@ 2009-01-23 19:50 Rakib Mullick
2009-01-25 3:18 ` Valdis.Kletnieks
0 siblings, 1 reply; 5+ messages in thread
From: Rakib Mullick @ 2009-01-23 19:50 UTC (permalink / raw)
To: LKML; +Cc: Ingo Molnar, Andrew Morton
Impact: Fix compile time warning.
The function audit_set_auditable called when CONFIG_AUDIT_TREE is set.
When CONFIG_AUDIT_TREE is not set then it might be unused, which
generates the following warning. Making audit_set_auditable function
inline fixes this problem. If anything else please notice.
CC kernel/auditsc.o
kernel/auditsc.c:745: warning: 'audit_set_auditable' defined but not used
Thanks.
Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
--- linux-2.6-orig/kernel/auditsc.c 2009-01-23 18:28:45.000000000 +0600
+++ linux-2.6/kernel/auditsc.c 2009-01-23 22:31:34.145406088 +0600
@@ -741,7 +741,7 @@ void audit_filter_inodes(struct task_str
rcu_read_unlock();
}
-static void audit_set_auditable(struct audit_context *ctx)
+static inline void audit_set_auditable(struct audit_context *ctx)
{
if (!ctx->prio) {
ctx->prio = 1;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] audit: Fix compile time warning on kernel/auditsc.c
2009-01-23 19:50 [PATCH] audit: Fix compile time warning on kernel/auditsc.c Rakib Mullick
@ 2009-01-25 3:18 ` Valdis.Kletnieks
2009-01-25 6:01 ` Rakib Mullick
0 siblings, 1 reply; 5+ messages in thread
From: Valdis.Kletnieks @ 2009-01-25 3:18 UTC (permalink / raw)
To: Rakib Mullick; +Cc: LKML, Ingo Molnar, Andrew Morton
[-- Attachment #1: Type: text/plain, Size: 1344 bytes --]
On Sat, 24 Jan 2009 01:50:30 +0600, Rakib Mullick said:
> Impact: Fix compile time warning.
>
> The function audit_set_auditable called when CONFIG_AUDIT_TREE is set.
> When CONFIG_AUDIT_TREE is not set then it might be unused, which
> generates the following warning. Making audit_set_auditable function
> inline fixes this problem. If anything else please notice.
>
> CC kernel/auditsc.o
> kernel/auditsc.c:745: warning: 'audit_set_auditable' defined but not used
>
> Thanks.
>
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
>
> --- linux-2.6-orig/kernel/auditsc.c 2009-01-23 18:28:45.000000000 +0600
> +++ linux-2.6/kernel/auditsc.c 2009-01-23 22:31:34.145406088 +0600
> @@ -741,7 +741,7 @@ void audit_filter_inodes(struct task_str
> rcu_read_unlock();
> }
>
> -static void audit_set_auditable(struct audit_context *ctx)
> +static inline void audit_set_auditable(struct audit_context *ctx)
Blech. That's abuse of inline. Can you find some other, more kernel-y
way to address the issue? (Possibly make it an actual inline up in a .h
file, with a #ifdef wrapping around it, or do something matching what's
done at the call site (apparently #ifdef'ing code is accepted in that .c
file, adding another #ifdef around that function to match all the *other*
'#ifdef CONFIG_AUDIT_TREE' would be less ugly than 'inline'.
[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] audit: Fix compile time warning on kernel/auditsc.c
2009-01-25 3:18 ` Valdis.Kletnieks
@ 2009-01-25 6:01 ` Rakib Mullick
2009-01-26 13:38 ` Ingo Molnar
0 siblings, 1 reply; 5+ messages in thread
From: Rakib Mullick @ 2009-01-25 6:01 UTC (permalink / raw)
To: Valdis.Kletnieks; +Cc: LKML, Ingo Molnar, Andrew Morton
On 1/25/09, Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> wrote:
>
> Blech. That's abuse of inline. Can you find some other, more kernel-y
> way to address the issue? (Possibly make it an actual inline up in a .h
> file, with a #ifdef wrapping around it, or do something matching what's
> done at the call site (apparently #ifdef'ing code is accepted in that .c
> file, adding another #ifdef around that function to match all the *other*
> '#ifdef CONFIG_AUDIT_TREE' would be less ugly than 'inline'.
Hi, Valdis. Thanks for your suggestions. Actually without inline-ing
the warning doesn't resolves. So, as you said I moved
audit_set_auditable function into kernel/audit.h, but not #ifdef
wrapping around it rather #ifdef into it. Since wrapping #ifdef
generates same warning few times more. Here is the patch. If I miss
anything, then please notice.
-----
Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
--- linux-2.6-orig/kernel/audit.h 2009-01-23 18:28:45.000000000 +0600
+++ linux-2.6/kernel/audit.h 2009-01-25 11:50:22.520217272 +0600
@@ -121,6 +121,16 @@ extern struct mutex audit_filter_mutex;
extern void audit_free_rule_rcu(struct rcu_head *);
extern struct list_head audit_filter_list[];
+static inline void audit_set_auditable(struct audit_context *ctx)
+{
+#ifdef CONFIG_AUDIT_TREE
+ if (!ctx->prio) {
+ ctx->prio = 1;
+ ctx->current_state = AUDIT_RECORD_CONTEXT;
+ }
+#endif
+}
+
#ifdef CONFIG_AUDIT_TREE
extern struct audit_chunk *audit_tree_lookup(const struct inode *);
extern void audit_put_chunk(struct audit_chunk *);
------
Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
--- linux-2.6-orig/kernel/auditsc.c 2009-01-23 18:28:45.000000000 +0600
+++ linux-2.6/kernel/auditsc.c 2009-01-25 11:50:25.712731936 +0600
@@ -741,14 +741,6 @@ void audit_filter_inodes(struct task_str
rcu_read_unlock();
}
-static void audit_set_auditable(struct audit_context *ctx)
-{
- if (!ctx->prio) {
- ctx->prio = 1;
- ctx->current_state = AUDIT_RECORD_CONTEXT;
- }
-}
-
static inline struct audit_context *audit_get_context(struct task_struct *tsk,
int return_valid,
int return_code)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] audit: Fix compile time warning on kernel/auditsc.c
2009-01-25 6:01 ` Rakib Mullick
@ 2009-01-26 13:38 ` Ingo Molnar
2009-01-28 1:18 ` Andrew Morton
0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2009-01-26 13:38 UTC (permalink / raw)
To: Rakib Mullick; +Cc: Valdis.Kletnieks, LKML, Andrew Morton
* Rakib Mullick <rakib.mullick@gmail.com> wrote:
> On 1/25/09, Valdis.Kletnieks@vt.edu <Valdis.Kletnieks@vt.edu> wrote:
> >
> > Blech. That's abuse of inline. Can you find some other, more kernel-y
> > way to address the issue? (Possibly make it an actual inline up in a .h
> > file, with a #ifdef wrapping around it, or do something matching what's
> > done at the call site (apparently #ifdef'ing code is accepted in that .c
> > file, adding another #ifdef around that function to match all the *other*
> > '#ifdef CONFIG_AUDIT_TREE' would be less ugly than 'inline'.
>
> Hi, Valdis. Thanks for your suggestions. Actually without inline-ing
> the warning doesn't resolves. So, as you said I moved
> audit_set_auditable function into kernel/audit.h, but not #ifdef
> wrapping around it rather #ifdef into it. Since wrapping #ifdef
> generates same warning few times more. Here is the patch. If I miss
> anything, then please notice.
>
> -----
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
>
> --- linux-2.6-orig/kernel/audit.h 2009-01-23 18:28:45.000000000 +0600
> +++ linux-2.6/kernel/audit.h 2009-01-25 11:50:22.520217272 +0600
> @@ -121,6 +121,16 @@ extern struct mutex audit_filter_mutex;
> extern void audit_free_rule_rcu(struct rcu_head *);
> extern struct list_head audit_filter_list[];
>
> +static inline void audit_set_auditable(struct audit_context *ctx)
> +{
> +#ifdef CONFIG_AUDIT_TREE
> + if (!ctx->prio) {
> + ctx->prio = 1;
> + ctx->current_state = AUDIT_RECORD_CONTEXT;
> + }
> +#endif
> +}
> +
> #ifdef CONFIG_AUDIT_TREE
> extern struct audit_chunk *audit_tree_lookup(const struct inode *);
> extern void audit_put_chunk(struct audit_chunk *);
>
> ------
> Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
>
> --- linux-2.6-orig/kernel/auditsc.c 2009-01-23 18:28:45.000000000 +0600
> +++ linux-2.6/kernel/auditsc.c 2009-01-25 11:50:25.712731936 +0600
> @@ -741,14 +741,6 @@ void audit_filter_inodes(struct task_str
> rcu_read_unlock();
> }
>
> -static void audit_set_auditable(struct audit_context *ctx)
> -{
> - if (!ctx->prio) {
> - ctx->prio = 1;
> - ctx->current_state = AUDIT_RECORD_CONTEXT;
> - }
> -}
> -
i dont see how this is an improvement in code quality. A non-oneliner
function got inlined and an ugly #ifdef got added.
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] audit: Fix compile time warning on kernel/auditsc.c
2009-01-26 13:38 ` Ingo Molnar
@ 2009-01-28 1:18 ` Andrew Morton
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2009-01-28 1:18 UTC (permalink / raw)
To: Ingo Molnar; +Cc: rakib.mullick, Valdis.Kletnieks, linux-kernel
On Mon, 26 Jan 2009 14:38:54 +0100
Ingo Molnar <mingo@elte.hu> wrote:
> > +static inline void audit_set_auditable(struct audit_context *ctx)
> > +{
> > +#ifdef CONFIG_AUDIT_TREE
> > + if (!ctx->prio) {
> > + ctx->prio = 1;
> > + ctx->current_state = AUDIT_RECORD_CONTEXT;
> > + }
> > +#endif
> > +}
> > +
> > #ifdef CONFIG_AUDIT_TREE
> > extern struct audit_chunk *audit_tree_lookup(const struct inode *);
> > extern void audit_put_chunk(struct audit_chunk *);
> >
> > ------
> > Signed-off-by: Rakib Mullick <rakib.mullick@gmail.com>
> >
> > --- linux-2.6-orig/kernel/auditsc.c 2009-01-23 18:28:45.000000000 +0600
> > +++ linux-2.6/kernel/auditsc.c 2009-01-25 11:50:25.712731936 +0600
> > @@ -741,14 +741,6 @@ void audit_filter_inodes(struct task_str
> > rcu_read_unlock();
> > }
> >
> > -static void audit_set_auditable(struct audit_context *ctx)
> > -{
> > - if (!ctx->prio) {
> > - ctx->prio = 1;
> > - ctx->current_state = AUDIT_RECORD_CONTEXT;
> > - }
> > -}
> > -
>
> i dont see how this is an improvement in code quality. A non-oneliner
> function got inlined and an ugly #ifdef got added.
We can just move the function inside an existing ifdef block.
--- a/kernel/auditsc.c~a
+++ a/kernel/auditsc.c
@@ -364,6 +364,15 @@ static int grow_tree_refs(struct audit_c
ctx->tree_count = 31;
return 1;
}
+
+static void audit_set_auditable(struct audit_context *ctx)
+{
+ if (!ctx->prio) {
+ ctx->prio = 1;
+ ctx->current_state = AUDIT_RECORD_CONTEXT;
+ }
+}
+
#endif
static void unroll_tree_refs(struct audit_context *ctx,
@@ -741,14 +750,6 @@ void audit_filter_inodes(struct task_str
rcu_read_unlock();
}
-static void audit_set_auditable(struct audit_context *ctx)
-{
- if (!ctx->prio) {
- ctx->prio = 1;
- ctx->current_state = AUDIT_RECORD_CONTEXT;
- }
-}
-
static inline struct audit_context *audit_get_context(struct task_struct *tsk,
int return_valid,
int return_code)
_
That file takes a rather unpleasing approach to this problem. Things
like
static void unroll_tree_refs(struct audit_context *ctx,
struct audit_tree_refs *p, int count)
{
#ifdef CONFIG_AUDIT_TREE
struct audit_tree_refs *q;
int n;
if (!p) {
/* we started with empty chain */
p = ctx->first_trees;
count = 31;
/* if the very first allocation has failed, nothing to do */
if (!p)
return;
}
n = count;
for (q = p; q != ctx->trees; q = q->next, n = 31) {
while (n--) {
audit_put_chunk(q->c[n]);
q->c[n] = NULL;
}
}
while (n-- > ctx->tree_count) {
audit_put_chunk(q->c[n]);
q->c[n] = NULL;
}
ctx->trees = p;
ctx->tree_count = count;
#endif
}
all over the place. It needs help.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-01-28 1:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-01-23 19:50 [PATCH] audit: Fix compile time warning on kernel/auditsc.c Rakib Mullick
2009-01-25 3:18 ` Valdis.Kletnieks
2009-01-25 6:01 ` Rakib Mullick
2009-01-26 13:38 ` Ingo Molnar
2009-01-28 1:18 ` Andrew Morton
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).