Re: [RFC PATCH ghak86 V1] audit: use audit_enabled as a boolean where convenient

On 2018-05-31 11:48, Paul Moore wrote: > On Thu, May 31, 2018 at 11:13 AM, Richard Guy Briggs <rgb(a)redhat.com&gt; 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(a)redhat.com&gt; > > --- > > 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.