Re: [PATCH] audit: Fix compile time warning on kernel/auditsc.c

[Andrew Morton]

On Mon, 26 Jan 2009 14:38:54 +0100
Ingo Molnar <mingo@xxxxxxx> 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@xxxxxxxxx>
> > 
> > --- 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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/