lkml.org 
[lkml]   [2018]   [Jun]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [RFC PATCH ghak86 V1] audit: use audit_enabled as a boolean where convenient
On Thu, May 31, 2018 at 12:38 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2018-05-31 11:48, Paul Moore wrote:
>> On Thu, May 31, 2018 at 11:13 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > Most uses of audit_enabled don't care about the distinction between
>> > AUDIT_ON and AUDIT_LOCKED, so using audit_enabled as a boolean makes
>> > more sense and is easier to read. Most uses of audit_enabled treat it as
>> > a boolean, so switch the remaining AUDIT_OFF usage to simply use
>> > audit_enabled as a boolean where applicable.
>> >
>> > See: https://github.com/linux-audit/audit-kernel/issues/86
>> >
>> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
>> > ---
>> > drivers/tty/tty_audit.c | 2 +-
>> > include/net/xfrm.h | 2 +-
>> > kernel/audit.c | 8 ++++----
>> > net/netfilter/xt_AUDIT.c | 2 +-
>> > net/netlabel/netlabel_user.c | 2 +-
>> > 5 files changed, 8 insertions(+), 8 deletions(-)
>>
>> I'm not sure I like this idea. Yes, technically this change is
>> functionally equivalent but I worry that this will increase the chance
>> that non-audit folks will mistake audit_enabled as a true/false value
>> when it is not. It might work now, but I worry about some subtle
>> problem in the future.
>
> Would you prefer a patch to change the majority (18) of uses of
> audit_enabled to be of the form "audit_enabled == AUDIT_OFF" (or
> "audit_enabled != AUDIT_OFF")?
>
> I prefer the approach in this patch because it makes the code smaller
> and significantly easier to read, but either way, I'd like all uses to
> be consistent so that it is easier to read all the code similarly.
>
>> If you are bothered by the comparison to 0 (magic numbers), you could
>> move the AUDIT_OFF/AUDIT_ON/AUDIT_LOCKED definitions into
>> include/linux/audit.h and convert the "audit_enabled == 0" to
>> "audit_enabled == AUDIT_OFF".
>
> I'd be fine doing that if you really dislike this patch's approach.

Like I said, I'm don't really care for the boolean-like approach of
this first patch.

--
paul moore
www.paul-moore.com

\
 
 \ /
  Last update: 2018-06-02 00:16    [W:0.063 / U:0.420 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site