Mailing List Archive

Andi Kleen wrote:
> The whole idea was pretty bad. Ifdefs are not ugly because the syntax
> looks ugly, but because it's a semantically ugly construct with bad
> maintainability impact.
>
> Trying to put syntactical sugar around that is a doomed exercise. It
> will be still ugly, no matter what you do.

Not true. Using C rather than CPP to control the compilation of config
options has the big win that all code paths are still visible to the
compiler. In some cases that's not what you want, but it often is, and
it would avoid some degree if inadvertent breakage of options. It can
also be syntactically a lot more pleasant.

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

On Sun, 18 May 2008, Adrian Bunk wrote:

> Even more important:
> How do you want to handle kconfig variables set to "m"?
>
> Expand them to 0.5 ? ;-)

:-)

Modules use a separate set of variables with _MODULE appended to their
names. Thus for an option FOO, you'll get:

#undef CONFIG_FOO
#undef CONFIG_FOO_MODULE

if it's set to "n",

#define CONFIG_FOO 1
#undef CONFIG_FOO_MODULE

if it's set to "y", and

#undef CONFIG_FOO
#define CONFIG_FOO_MODULE 1

if it's set to "m". Individual tests may check these both as they find
appropriate.

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

Jeremy Fitzhardinge wrote:
> Andi Kleen wrote:
>> The whole idea was pretty bad. Ifdefs are not ugly because the syntax
>> looks ugly, but because it's a semantically ugly construct with bad
>> maintainability impact.
>>
>> Trying to put syntactical sugar around that is a doomed exercise. It
>> will be still ugly, no matter what you do.
>
> Not true. Using C rather than CPP to control the compilation of config
> options has the big win that all code paths are still visible to the
> compiler.

A small win. Still lots of other problems, including testing.

In some cases that's not what you want, but it often is, and
> it would avoid some degree if inadvertent breakage of options. It can
> also be syntactically a lot more pleasant.

Well it's still an unnecessary different code path and making
it look nicer is just an excuse from properly cleaning it up.

-Andi

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

On Sun, May 18, 2008 at 07:33:47PM +0100, Jeremy Fitzhardinge wrote:
> Adrian Bunk wrote:
>> Even more important:
>> How do you want to handle kconfig variables set to "m"?
>>
>> Expand them to 0.5 ? ;-)
>>
>
> Doesn't much matter. We can't do "#ifdef CONFIG_FOO == m" anyway.

Yes, we can:

#ifdef CONFIG_FOO_MODULE

> J

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

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

Adrian Bunk wrote:
> On Sun, May 18, 2008 at 07:33:47PM +0100, Jeremy Fitzhardinge wrote:
>
>> Adrian Bunk wrote:
>>
>>> Even more important:
>>> How do you want to handle kconfig variables set to "m"?
>>>
>>> Expand them to 0.5 ? ;-)
>>>
>>>
>> Doesn't much matter. We can't do "#ifdef CONFIG_FOO == m" anyway.
>>
>
> Yes, we can:
>
> #ifdef CONFIG_FOO_MODULE
>

Well then, that's your answer.

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

On Sun, May 18, 2008 at 07:38:41PM +0100, Maciej W. Rozycki wrote:
> On Sun, 18 May 2008, Adrian Bunk wrote:
>
> > Even more important:
> > How do you want to handle kconfig variables set to "m"?
> >
> > Expand them to 0.5 ? ;-)
>
> :-)
>
> Modules use a separate set of variables with _MODULE appended to their
> names. Thus for an option FOO, you'll get:
>
> #undef CONFIG_FOO
> #undef CONFIG_FOO_MODULE
>
> if it's set to "n",
>
> #define CONFIG_FOO 1
> #undef CONFIG_FOO_MODULE
>
> if it's set to "y", and
>
> #undef CONFIG_FOO
> #define CONFIG_FOO_MODULE 1
>
> if it's set to "m". Individual tests may check these both as they find
> appropriate.

I do know that.


But your suggestion was:

static inline unsigned int get_nmi_count(int cpu)
{
return cpu_x86_64 ? cpu_pda(cpu)->__nmi_count : nmi_count(cpu);
}


And Jeremy said "It would be nice if CONFIG_* expanded to 0/1".


My point was simply that like most of the fun^Wproblems we already have
in kconfig also here the tristate logic we need would make stuff more
tricky.


> Maciej

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

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

[Andi Kleen - Sun, May 18, 2008 at 09:13:04PM +0200]
| Jeremy Fitzhardinge wrote:
| > Andi Kleen wrote:
| >> The whole idea was pretty bad. Ifdefs are not ugly because the syntax
| >> looks ugly, but because it's a semantically ugly construct with bad
| >> maintainability impact.
| >>
| >> Trying to put syntactical sugar around that is a doomed exercise. It
| >> will be still ugly, no matter what you do.
| >
| > Not true. Using C rather than CPP to control the compilation of config
| > options has the big win that all code paths are still visible to the
| > compiler.
|
| A small win. Still lots of other problems, including testing.
|
| In some cases that's not what you want, but it often is, and
| > it would avoid some degree if inadvertent breakage of options. It can
| > also be syntactically a lot more pleasant.
|
| Well it's still an unnecessary different code path and making
| it look nicer is just an excuse from properly cleaning it up.
|
| -Andi
|

ok, thanks to all reviewers for the look being taken. I will try to
eliminate as much #ifdefs as possible, make them straight and understandable
and will send the next patch as only get it done. Thanks to all!

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

Btw, if someone is still watching this thread - I've found a bit strange
behaviour of nmi on 32bit platform. Look, we have

nmi_watchdog = NMI_DEFAULT

by default which is the alias to NMI_DISABLED. Then lets assume that
user put some option on command line, say for example he passes something
like that

nmi_watchdog=2

which set it to

nmi_watchdog = NMI_LOCAL_APIC

with only that option passed we have sysfs entry created
but I can't figure out why in proc_nmi_enabled() we have this
code

if (nmi_watchdog == NMI_DEFAULT) {
if (lapic_watchdog_ok())
nmi_watchdog = NMI_LOCAL_APIC;
else
nmi_watchdog = NMI_IO_APIC;
}

it seems it just _dont need_ at and could be safetly removed.
Did I miss something?

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

[Cyrill Gorcunov - Mon, May 19, 2008 at 10:07:53PM +0400]
| Btw, if someone is still watching this thread - I've found a bit strange
| behaviour of nmi on 32bit platform. Look, we have
|
| nmi_watchdog = NMI_DEFAULT
|
| by default which is the alias to NMI_DISABLED. Then lets assume that
| user put some option on command line, say for example he passes something
| like that
|
| nmi_watchdog=2
|
| which set it to
|
| nmi_watchdog = NMI_LOCAL_APIC
|
| with only that option passed we have sysfs entry created
| but I can't figure out why in proc_nmi_enabled() we have this
| code
|
| if (nmi_watchdog == NMI_DEFAULT) {
| if (lapic_watchdog_ok())
| nmi_watchdog = NMI_LOCAL_APIC;
| else
| nmi_watchdog = NMI_IO_APIC;
| }
|
| it seems it just _dont need_ at and could be safetly removed.
| Did I miss something?
|
| - Cyrill -

Ah, I found... sorry for noise, I'm shutting up ;)

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

well, here is a second version of patch attached (i'm in office now so
can't use my
lovely mutt as usual). Please take a look. Any objections are quite welcome!

- Cyrill -