Re: 2.4.0-test8-pre1 is quite bad / how about integrating Rik's VM

From: Alexander Viro (aviro@redhat.com)
Date: Tue Sep 05 2000 - 16:51:53 EST


On Wed, 6 Sep 2000, Chris Wedgwood wrote:

> On Tue, Sep 05, 2000 at 04:15:29PM -0400, Alexander Viro wrote:
>
> 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.
>
> * simple #define foo(x,y) definitions should be reimplemented as inline
> functions?
>
> In general -- make the compile more aware of the surroundings by
> utilising the preprocessor less and less?

Exactly. Look: cpp(1) is at least Homsky-1 complete. You really don't want to
emulate all possible expansions. OTOH, C parsers are easily available.
Moreover, the parser used by normal build process can be asked to give the
intermediate results, so attaching tags-like tools to it is feasible. So if
we can get full source seen by gcc - we've won the hardest part of the battle.
In most cases it's relatively easy (garden-variety conditional compilation is
not a problem, since if (CONSTANT) { } else { } is recognized by gcc). In
some... Well, just look at the text below and see if it deserves fixing:

static void __init cyrix_arr_init(void)
{
    struct set_mtrr_context ctxt;
    unsigned char ccr[7];
    int ccrc[7] = { 0, 0, 0, 0, 0, 0, 0 };
#ifdef CONFIG_SMP
    int i;
#endif

[snip]

#ifdef CONFIG_SMP
    for(i=0; i<7; i++) ccr_state[i] = ccr[i];
    for(i=0; i<8; i++)
      cyrix_get_arr(i,
        &arr_state[i].base, &arr_state[i].size, &arr_state[i].type);
#endif

[snip]

}

Do you like it? Me neither. Now, if the thing turns into (preserving
ugly indent style)

static void __init cyrix_arr_init(void)
{
    struct set_mtrr_context ctxt;
    unsigned char ccr[7];
    int ccrc[7] = { 0, 0, 0, 0, 0, 0, 0 };

[snip]

    if (CONFIG_SMP) {
        int i;
        for(i=0; i<7; i++) ccr_state[i] = ccr[i];
        for(i=0; i<8; i++)
            cyrix_get_arr(i,
                &arr_state[i].base, &arr_state[i].size, &arr_state[i].type);
    }

[snip]

}

- much more readable. And that's relatively sane example, BTW. There are
much uglier ones. The worst I remember was for (...) vs. if (...) before
the body, depending on value of the constant and bunch of #ifs on the same
constant in said body. IMO unknown author deserves rectal insertion of K&P
(The Practice of Programming). Rectal since that's apparently the shortest
way to his head...

I also remember "it doesn't compile - compiles fine here" threads. Lots of
them. In many cases it boiled down to the trivial syntax error ifdefed out
in almost all configurations.

Yes, some use of #ifdef is probably necessary. But 99% of the current use
is completely gratitious.

Don't get me wrong, I'm not calling for throwing cpp(1) away. But we are
using the thing in places where we have better replacements and we are
paying for that.

-
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:23 EST