From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752927AbcGAKee (ORCPT ); Fri, 1 Jul 2016 06:34:34 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:34706 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752893AbcGAKec (ORCPT ); Fri, 1 Jul 2016 06:34:32 -0400 Date: Fri, 1 Jul 2016 12:20:18 +0200 From: Ingo Molnar To: Borislav Petkov Cc: Linus Torvalds , Steven Rostedt , Greg Kroah-Hartman , Peter Zijlstra , Andrew Morton , Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= , Franck Bui , LKML Subject: Re: [PATCH -v2 2/2] printk: Add kernel parameter to control writes to /dev/kmsg Message-ID: <20160701102018.GB8787@gmail.com> References: <1467194161-1472-1-git-send-email-bp@alien8.de> <1467194161-1472-3-git-send-email-bp@alien8.de> <20160701090413.GB27709@gmail.com> <20160701091710.GC4593@pd.tnic> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160701091710.GC4593@pd.tnic> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Borislav Petkov wrote: > On Fri, Jul 01, 2016 at 11:04:13AM +0200, Ingo Molnar wrote: > > ... > > > So the most robust way to define such bitfields is via a pattern like this: > > > > enum devkmsg_log_bits { > > __DEVKMSG_LOG_BIT_ON, > > __DEVKMSG_LOG_BIT_OFF, > > __DEVKMSG_LOG_BIT_LOCK, > > }; > > > > enum devkmsg_log_masks { > > DEVKMSG_LOG_MASK_ON = BIT(__DEVKMSG_LOG_BIT_ON), > > DEVKMSG_LOG_MASK_OFF = BIT(__DEVKMSG_LOG_BIT_OFF), > > DEVKMSG_LOG_MASK_LOCK = BIT(__DEVKMSG_LOG_BIT_LOCK), > > Agreed with so far, I'd only drop the "_MASK" thing and make it even > easier on the eyes: > > enum devkmsg_log_state { > DEVKMSG_LOG_ON = BIT(__DEVKMSG_LOG_BIT_ON), > DEVKMSG_LOG_OFF = BIT(__DEVKMSG_LOG_BIT_OFF), > DEVKMSG_LOCK = BIT(__DEVKMSG_LOG_BIT_LOCK), > }; It's just a nit, but generally it's nice to know the character of such values - i.e. in case that it's a bit mask that has to be used with bit ops. That's more important IMHO than brevity. This means that possibly buggy code like this stands out immediately: if (devkmgs_log == DEVKMSG_LOG_MASK_ON) while this one: if (devkmgs_log == DEVKMSG_LOG_ON) might slip through. But no strong feelings either way! Thanks, Ingo