public inbox for linux-audit@redhat.com
 help / color / mirror / Atom feed
From: Steve Grubb <sgrubb@redhat.com>
To: linux-audit@redhat.com
Subject: Re: [PATCH] [AUDIT] auditfilter.c cleanup/const-ification
Date: Mon, 3 Apr 2006 16:56:05 -0400	[thread overview]
Message-ID: <200604031656.05141.sgrubb@redhat.com> (raw)
In-Reply-To: <20060403125128.GG3157@gaz.sfgoth.com>

Hi Mitchell,

Thanks for looking at this. 

On Monday 03 April 2006 08:51, Mitchell Blank Jr wrote:
> The gcc warning isn't correct (since audit_filter_user() only looked at
> state if audit_filter_user_rules() returned non-zero, in which case 'state'
> would have been initialized)  However the code was needlessly complex --
> audit_filter_user_rules() carefully populated the "enum audit_state *state"
> with various value but it's only caller just cares if it's AUDIT_DISABLED
> or not.

IIRC, this was done to mirror the filtering of syscalls. I think we discussed 
this last June/July. Anyways it was a long time ago.

> It's shorter and simpler to just let audit_filter_user_rules() 
> modify its caller's return value more directly.  As an added bonus this
> also removes the warning.

Changes to the rule matcher have to be carefully tested just in case something 
obscure needs it. In this case, I don't think so since its a user space 
originating message.

> While I was looking at auditfilter.c I did some other minor cleanup
>
>   * const-ified pointers where possible
>
>   * both audit_data_to_entry() and audit_krule_to_data() had an unused
>     variable called "void *bufp" which I removed
>
>   * [minor] I changed some variables from "int" to "unsigned int" if
>     they can't be negative.  Since ->field_count is unsigned I think it's
>     a little cleaner to use an unsigned type to iterate through it

These are good cleanups. In a way, I wished this was 2 patches instead of 1. 
I'd take all these cleanups immediately. The other one I'd probably want to 
put in the test kernel for a week or two just to make sure nothing relied on 
the state.

Thanks,
-Steve

  reply	other threads:[~2006-04-03 20:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-04-03 12:51 [PATCH] [AUDIT] auditfilter.c cleanup/const-ification Mitchell Blank Jr
2006-04-03 20:56 ` Steve Grubb [this message]
2006-04-03 23:46   ` Mitchell Blank Jr
2006-04-04 14:37 ` Amy Griffis
2006-04-05 11:41   ` Mitchell Blank Jr
2006-04-05 12:29     ` Steve Grubb
2006-04-05 13:50       ` Mitchell Blank Jr
2006-04-06 13:43         ` Mitchell Blank Jr
2006-04-06 15:41           ` Alexander Viro
2006-04-05 13:30     ` Amy Griffis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200604031656.05141.sgrubb@redhat.com \
    --to=sgrubb@redhat.com \
    --cc=linux-audit@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox