Re: if (CONFIG_FOO) Re: 2.4.0-test8-pre1 is quite bad / how about

From: Russell King (rmk@arm.linux.org.uk)
Date: Wed Sep 06 2000 - 16:32:37 EST


Martin MaD Douda writes:
> On Tue, 5 Sep 2000, Alexander Viro wrote:
> > The _real_ problem is preprocessor abuse. BTW, could we schedule for
> > 2.5 the following?
> > * things like CONFIG_FOO are _always_ defined. As 0 or 1, that is.
> > * #ifdef CONFIG_FOO => if (CONFIG_FOO) in *.c. gcc will kill the unused
> > branches just fine.
> > * Yes, Virginia, it means that controlflow-affecting expansion has to
> > go. Good riddance, IMO.
> >
> > Goal: making sure that every bloody line of the files we choose to
> > compile goes through the parser. Will do wonders with test coverage and will
> > make analysis tools like tags viable. Then we can just use the gcc frontend
> > output as input for such beasts.
>
> I can understand advantages the if(CONFIG_FOO) approach has, but I'm not
> sure it is worth compilation slowdown.

In response to Al (I don't think I've got his mail, still got 150 emails
to look at since last night):

Been there, done that, maybe you'd like to look at the ARM code which contains
the machine_is_xxx()? These essentially reduce down to one of three possible
values:

1. (0)
2. (MACH_TYPE_ASSABET == MACH_TYPE_ASSABET)
3. (__machine_type == MACH_TYPE_ASSABET)

The relevent definition is chosen by a set of pre-processor generated out of
arch/arm/tools/gen-mach-types.

Whilst it is good to remove redunant code, you still need #ifdefs...#endifs,
so I wouldn't recommend this style of configuration management for the
generic kernel.

Think about this:

struct bar foo_specific = {
        ...
};

struct bar *goo()
{
        if (CONFIG_FOO)
                return &foo_specific;
        else
                return NULL;
}

Unfortunately, the compiler always generates dataspace for both structures,
even if CONFIG_FOO is 0. So, how do we get rid of them? Simple, surround
them with #if CONFIG_FOO...#endif. Oh dear, the symbol is now undefined.
I guess we must add #if CONFIG_FOO around the if (CONFIG_FOO) as well. Ug,
I suppose we might as well just do:

struct bar *goo()
{
#if CONFIG_FOO
        return &foo_specific;
#else
        return NULL;
#endif
}

and be done with it.

Yes, I'd agree a good idea, and may apply to some small bits of code, but
not as a general kernel-wide thing. And converting the #ifdefs all to #ifs
is going to be a nightmarish large patch (before even thinking of making
configure produce the "define CONFIG_FOO as 0 or 1" you have first got to
make sure that there are no #ifdef CONFIG_FOO statements in existance.

(Note that the reason we do it on the ARM is NOT because we have a never
or always condition, but because we have never, maybe or always conditions,
and this is the easiest way to fix them up. But. We still need #ifdefs
#endifs).
   _____
  |_____| ------------------------------------------------- ---+---+-
  | | Russell King rmk@arm.linux.org.uk --- ---
  | | | | http://www.arm.linux.org.uk/personal/aboutme.html / / |
  | +-+-+ --- -+-
  / | THE developer of ARM Linux |+| /|\
 / | | | --- |
    +-+-+ ------------------------------------------------- /\\\ |
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Sep 07 2000 - 21:00:28 EST