From mboxrd@z Thu Jan 1 00:00:00 1970 From: Steve Grubb Subject: Re: [PATCH] [AUDIT] auditfilter.c cleanup/const-ification Date: Mon, 3 Apr 2006 16:56:05 -0400 Message-ID: <200604031656.05141.sgrubb@redhat.com> References: <20060403125128.GG3157@gaz.sfgoth.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20060403125128.GG3157@gaz.sfgoth.com> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-audit-bounces@redhat.com Errors-To: linux-audit-bounces@redhat.com To: linux-audit@redhat.com List-Id: linux-audit@redhat.com Hi Mitchell, Thanks for looking at this.=20 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 'st= ate' > would have been initialized) =A0However the code was needlessly complex= -- > audit_filter_user_rules() carefully populated the "enum audit_state *st= ate" > with various value but it's only caller just cares if it's AUDIT_DISABL= ED > or not. IIRC, this was done to mirror the filtering of syscalls. I think we discu= ssed=20 this last June/July. Anyways it was a long time ago. > It's shorter and simpler to just let audit_filter_user_rules()=20 > modify its caller's return value more directly. =A0As an added bonus th= is > also removes the warning. Changes to the rule matcher have to be carefully tested just in case some= thing=20 obscure needs it. In this case, I don't think so since its a user space=20 originating message. > While I was looking at auditfilter.c I did some other minor cleanup > > =A0 * const-ified pointers where possible > > =A0 * both audit_data_to_entry() and audit_krule_to_data() had an unuse= d > =A0 =A0 variable called "void *bufp" which I removed > > =A0 * [minor] I changed some variables from "int" to "unsigned int" if > =A0 =A0 they can't be negative. =A0Since ->field_count is unsigned I th= ink it's > =A0 =A0 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.=20 I'd take all these cleanups immediately. The other one I'd probably want = to=20 put in the test kernel for a week or two just to make sure nothing relied= on=20 the state. Thanks, -Steve