All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Ravnborg <sam@ravnborg.org>
To: Michal Marek <mmarek@suse.cz>
Cc: Arnaud Lacombe <lacombar@gmail.com>,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>,
	Valdis.Kletnieks@vt.edu
Subject: Re: [PATCH] kconfig: Only generate config_is_xxx for bool and tristate options
Date: Wed, 13 Jul 2011 22:08:13 +0200	[thread overview]
Message-ID: <20110713200813.GA11538@merkur.ravnborg.org> (raw)
In-Reply-To: <4E1D9C25.8080300@suse.cz>

On Wed, Jul 13, 2011 at 03:22:45PM +0200, Michal Marek wrote:
> On 18.5.2011 08:23, Arnaud Lacombe wrote:
>> Hi,
>>
>> [added Valdis.Kletnieks@vt.edu to CC:]
>>
>> On Tue, May 17, 2011 at 3:53 PM, Sam Ravnborg<sam@ravnborg.org>  wrote:
>>> On Tue, May 17, 2011 at 05:35:32PM +0200, Michal Marek wrote:
>>>> For strings and integers, the config_is_xxx macros are useless and
>>>> sometimes misleading:
>>>>
>>>>    #define CONFIG_INITRAMFS_SOURCE ""
>>>>    #define config_is_initramfs_source() 1
>>>
>>> I'm late with this comment....
>>> Could we introduce "config_is_foo" using a syntax that
>>> does not break grepability?
>>>
>>> Maybe a syntax like this?
>>>
>>>     #ifdef CONFIG_FOO
>>>
>>> and
>>>
>>>     if (KCONFIG_FOO())
>>>
>>> Grepping for the use of a symbol is a very typical thing,
>>> so we should try to keep this.
>>> And with the suggested syntax above I expect fixdep to
>>> catch up both usage types.
>>>
>> Actually, there is already an issue, on a much smaller scale, in the
>> current tree with NUMA_BUILD and COMPACTION_BUILD. The real way to fix
>> this would be to always defines CONFIG_FOO, its value being 1 or 0
>> depending on whether or not the symbol is selected. This is a
>> +35k/-35k change.
>>
>> Also, I find KCONFIG_FOO() is too specific to be in the kernel source,
>> and redundant with CONFIG_FOO.
>>
>> I've been playing a bit with the preprocessor, and reached that point:
>>
>> #define EXPAND(x)       __ ## x
>> #define CONFIGURED(x)   \
>>          ({  int __1 __maybe_unused = 1;         \
>>              int __ ## x __maybe_unused = 0;     \
>>              EXPAND(x); })
>>
>> I am not specifically proud of that, use case would be:
>>
>> extern func(void);
>> int fn()
>> {
>>          if(CONFIGURED(CONFIG_FOO))
>>                  func();
>> }
>
> I finally got round to revisit this. Your approach inspired me to a much  
> simpler scheme: Instead of generating the config_is_xxx macros for  
> direct use in the code, name them __enabled_CONFIG_XXX or similar and  
> have a macro that expands given CONFIG_XXX symbol to the other form:

But then we clutter the namesapce with another set of variables that will
be misued. If not in the kernel then by other kconfig users.

Arnaud approach is nice as it does not require any additional
symbols to be generated.
We can always discuss tha naming - but the approach looks good.

To look at actual usage first:

int foo()
{
	if (KCONFIG(CONFIG_FOO)) {
		// this should be true for both module and non-module case
	}

	if (KCONFIG_NOT_MODULE(CONFIG_FOO)) {
		// this should be true only if this is not a module
	}



	if (KCONFIG_MODULE(CONFIG_FOO)) {
		// this should only be true for the module case
	}

}

Using the above even the naive reader is aware this is a kconfig generate
symbol.
Using "KCONFIG" in the name gives a stong hint that this is a kconfig thing.

We could stuff it all into a kconfig.h header like the attached (not tested)
but based on Arnauds sketch above - so creadits goes to him.

	Sam

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
new file mode 100644
index 0000000..3ccdd73
--- /dev/null
+++ b/include/linux/kconfig.h
@@ -0,0 +1,30 @@
+#ifndef _LINUX_KCONFIG_H
+#define _LINUX_KCONFIG_H
+
+/*
+ * Helper macros for dealing with kconfig symbols
+ */
+
+#include <linux/compiler.h>
+
+#define __symbol_value(sym)       __ ## x
+#define __symbol_module(sym)      __symbol_value(__ ## sym ## _MODULE)
+
+/* return 1 if x is defined and not a module */
+#define KCONFIG_NON_MODULE(sym)					\
+	({ int __1 __maybe_unused = 1;				\
+	   int __ ## sym __maybe_unused = 0;			\
+	   __symbol_value(sym); })
+
+/* return 1 if sym is a module symbol */
+#define KCONFIG_MODULE(sym)					\
+	({ int __1 __maybe_unused = 1;				\
+	   int __ ## sym ## _MODULE __maybe_unused = 0;	\
+	   __symbol_value(sym); })
+
+/* return 1 if sym is defined - module or not */
+#define KCONFIG(sym) (KCONFIG_NON_MODULE(sym) || KCONFIG_MODULE(sym))
+
+#undef __symbol_value
+#undef __symbol_module
+#endif

  parent reply	other threads:[~2011-07-13 20:08 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-17 15:35 [PATCH] kconfig: Only generate config_is_xxx for bool and tristate options Michal Marek
2011-05-17 18:05 ` Jean-Christophe PLAGNIOL-VILLARD
2011-05-17 19:44   ` Michal Marek
2011-05-17 19:53 ` Sam Ravnborg
2011-05-18  5:16   ` Jean-Christophe PLAGNIOL-VILLARD
2011-05-18  6:19     ` Sam Ravnborg
2011-05-18  6:27       ` Arnaud Lacombe
2011-05-18  6:23   ` Arnaud Lacombe
2011-07-13 13:22     ` Michal Marek
2011-07-13 15:20       ` Arnaud Lacombe
2011-07-13 20:08       ` Sam Ravnborg [this message]
2011-07-19 13:45         ` Michal Marek
2011-07-25 22:58       ` [RFC][PATCH 1/2] kconfig: Introduce KCONFIG(), KCONFIG_BUILTIN() and KCONFIG_MODULE() Michal Marek
2011-07-25 22:58         ` [RFC][PATCH 2/2] mm: Switch NUMA_BUILD and COMPACTION_BUILD to new KCONFIG() syntax Michal Marek
2011-07-25 22:58           ` Michal Marek
2011-07-26 15:19           ` Michal Hocko
2011-07-26 15:19             ` Michal Hocko
2011-07-26 18:34             ` Michal Marek
2011-07-26 18:34               ` Michal Marek
2011-07-26 18:52               ` Michal Hocko
2011-07-26 18:52                 ` Michal Hocko
2011-07-26 13:01         ` [RFC][PATCH 1/2] kconfig: Introduce KCONFIG(), KCONFIG_BUILTIN() and KCONFIG_MODULE() Américo Wang
2011-07-26 13:21           ` Michal Marek
2011-07-26 15:04             ` Randy Dunlap
2011-07-26 18:28               ` Michal Marek
2011-07-26 18:28                 ` Randy Dunlap
2011-07-26 18:48                   ` Arnaud Lacombe
2011-07-27  0:42                     ` Arnaud Lacombe
2011-07-27  4:35                       ` Randy Dunlap
2011-07-27  8:36                         ` Michal Marek
2011-07-27 13:31                           ` Arnaud Lacombe
2011-07-27 13:38                             ` Michal Marek
2011-07-27 15:11                               ` Arnaud Lacombe
2011-07-27 15:18                                 ` Michal Marek
2011-07-27 16:36                           ` Américo Wang
2011-07-27 15:09         ` Arnaud Lacombe
2011-07-27 15:16           ` Michal Marek
2011-07-27 15:18             ` Arnaud Lacombe
2011-07-29 13:51         ` [PATCH] kconfig: Introduce IS_ENABLED(), IS_BUILTIN() and IS_MODULE() Michal Marek
2011-07-29 17:43           ` Arnaud Lacombe
2011-07-29 18:25           ` Randy Dunlap
2011-07-29 18:58             ` Arnaud Lacombe
2011-08-02 17:33           ` Sam Ravnborg
2011-08-02 17:50             ` Arnaud Lacombe
2011-08-02 19:26               ` Sam Ravnborg
2011-08-02 19:33                 ` Michal Marek
2011-08-02 19:33                   ` Arnaud Lacombe
2011-05-25 13:35 ` [PATCH] kconfig: Only generate config_is_xxx for bool and tristate options Michal Marek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110713200813.GA11538@merkur.ravnborg.org \
    --to=sam@ravnborg.org \
    --cc=Valdis.Kletnieks@vt.edu \
    --cc=lacombar@gmail.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mmarek@suse.cz \
    --cc=plagnioj@jcrosoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.