[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20110513080909.GO18952@game.jcrosoft.org>
Date: Fri, 13 May 2011 10:09:09 +0200
From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>
To: Michal Marek <mmarek@...e.cz>
Cc: Arnaud Lacombe <lacombar@...il.com>, linux-kernel@...r.kernel.org,
linux-kbuild@...r.kernel.org, x86@...nel.org,
Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro
On 10:52 Mon 09 May , Michal Marek wrote:
> On 7.5.2011 03:50, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >On 12:19 Fri 06 May , Arnaud Lacombe wrote:
> >>Why would it be a good thing ?
> >>
> >>Most configuration-dependent code inside functions tends to be moved
> >>to a static inline already, which get conditionally defined based on
> >>the CONFIG_<foo>. If it is not, then the code is badly architectured
> >>(-> bad). Using that if(xxx) notation would also lead to yet more
> >>heavily indented function (-> bad). Moreover, this introduces
> >>yet-another way to check for an information (-> bad), and you will end
> >>up with mixing the config_is_<xxx> notation inside a function
> >>declaration, and CONFIG_<xxx> when not inside a function (-> bad)
> >>
> >>Actually, this is even worse than that as you'll not be able to hide
> >>structure (or structure members) inside CONFIG_<xxx> and use that
> >>structure (or structure members) in config_is_<xxx> protected block
> >>without causing compile-time failure.
> >sorry but conditionnal structure members is bad practice
> >you save nearly no space nut for the test of the code in multiple
> >configuration. Use union for this.
> >
> >the compile-time failure is good here. it's means your code is not generic.
> >
> >specially when you want to keep code running on multiple soc/arch keep compiling
> >no matter the configuration
> >
> >#ifdef in the code is a really bad habit
>
> Do you have proof of concept patches that make use of the
> config_is_xxx macros? Acked by the respective subsystem maintainers?
> It would be a good idea to send them along to show that this feature
> is going to be actually used.
I've seen thousands of place in the kernel we can use
so I'll just take one example on x86
the patch attached is just an example
Best Regards,
J.
View attachment "p.patch" of type "text/x-diff" (2419 bytes)
Powered by blists - more mailing lists