lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 17 Jun 2014 10:53:22 +0200
From:	Paul Bolle <pebolle@...cali.nl>
To:	Doug Anderson <dianders@...omium.org>
Cc:	Bill Richardson <wfrichar@...omium.org>,
	Simon Glass <sjg@...omium.org>,
	Lee Jones <lee.jones@...aro.org>,
	Stephen Warren <swarren@...dia.com>,
	Wolfram Sang <wsa@...-dreams.de>,
	Andrew Bresticker <abrestic@...omium.org>,
	Dylan Reid <dgreid@...omium.org>,
	Olof Johansson <olof@...om.net>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	linux-samsung-soc <linux-samsung-soc@...r.kernel.org>,
	"linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 5/7] mfd: cros_ec: Sync to the latest
 cros_ec_commands.h from EC sources

Doug,

On Fri, 2014-06-13 at 08:22 -0700, Doug Anderson wrote:
> On Fri, Jun 13, 2014 at 1:08 AM, Paul Bolle <pebolle@...cali.nl> wrote:
> > On Wed, 2014-06-11 at 08:11 -0700, Doug Anderson wrote:
> >> This is a config option on the ChromeOS EC
> >> <https://chromium.googlesource.com/chromiumos/platform/ec>.  Doing a
> >> grep there:
> >>
> >> board/samus/board.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE
> >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> common/charge_state_v2.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> driver/battery/samus.c:#ifdef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> driver/battery/samus.c:#endif   /* CONFIG_CHARGER_PROFILE_OVERRIDE */
> >> include/config.h:#undef CONFIG_CHARGER_PROFILE_OVERRIDE
> >> include/ec_commands.h:  /* Range for CONFIG_CHARGER_PROFILE_OVERRIDE params */
> >> test/test_config.h:#define CONFIG_CHARGER_PROFILE_OVERRIDE
> >
> > I see. So this is not a Kconfig macro but a general macro with a CONFIG_
> > prefix. There are quite a bit of those in the tree already, but still,
> > would another prefix also do?
> 
> Given that it's an entirely separate project and this is a valid
> CONFIG option in that project, it seems a lot to ask them not to use
> the CONFIG_ prefix.  Also: the part you are objecting to is only a
> comment, right?

So all the hits you quoted above are actually from code that's never
going to be included in the kernel tree, right? If so, then yes, we're
only discussing a single comment.

> We could certainly add extra wording in the comment to make it obvious
> that this is a CONFIG option for the EC and not the kernel.  Would
> that be enough?  ...or are you trying to use some scripts to
> automatically process files to look for CONFIG options?

Yes, I'm using a script to check for Kconfig macros, among other things.
It doesn't care about comments (because every now and then mistakes are
made in comments too, and some of those can get surprisingly confusing).

Anyhow, the CONFIG_ prefix used in the kernel tree is quite generic, but
we're stuck with it. Would it be bothersome to drop it in that comment?
Mentioning a preprocessor macro from a separate project is a bit
confusing to begin with. How is one supposed to know that this is a
reference to something out of tree?

So, in summary, while we're apparently only discussing a single comment,
I would appreciate it if it could be reworded, preferably by dropping
that the CONFIG_ prefix. But other people might care very little, as
they don't share this particular pet peeve.

Thanks,


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists