Re: [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(a)redhat.com&gt; wrote: > 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. Like I said, I'm don't really care for the boolean-like approach of this first patch.

paul moore