[PATCH] Re: 2.4.6p6: dep_{bool,tristate} $CONFIG_ARCH_xxx bugs

From: Riley Williams (rhw@MemAlpha.CX)
Date: Sun Jul 01 2001 - 18:04:22 EST

Hi Keith.

I don't agree with your analysis in the first block quoted below, but
that's not relevant as I feel there's a better solution than either of
the ones proposed so far. See below for details...

>> 1. Adam's point is that there are dep_* statements in the config
>> setup that have been used to say that a particular option is
>> dependant upon a particular architecture, but this doesn't work.
>> 3. MY understanding of the situation is that ALL architecture
>> specific config lines are EXPECTED to be in the arch/*/config.in
>> files, where they will only even be seen when the relevant
>> architecture is being compiled for.
>> As a result of this, I would summarise this discussion as saying
>> that there is a bug in the kernel config scripts in that some
>> options that should be located in the architecture-specific
>> config files are in the all-architecture config files instead.

> (1) and (3) are correct but your conclusion is not. The problem is
> dep_tristate CONFIG_some_driver $CONFIG_some_arch
> where the intention is to allow the driver only if some_arch is
> set.

Can I suggest you re-read your comment and the points you quoted and
said are correct. As a DIRECT logical consequence of "(1) and (3) are
correct" (as you phrased it), we get the conclusion that any config
lines that depend on "if some arch is set" (as you phrased it) are in
the architecture-specific config files. This leads DIRECTLY to the
conclusion that ANY lines dependant on a particular architecture that
are not in the architecture-specific config files are a bug in the
kernel config scripts.


Personally, I would suggest that the bug in this case is the actual
assumption that (3) is true, and the correct course of action is to
remove that assumption as it leads directly to the spaghetti in the
configuration system that we currently have.

> When you compile for some_other_arch, CONFIG_some_arch is
> undefined. dep_tristate treats undefined variables as "don't
> care"...

Agreed - and whoever did the config lines where the dependancy is an
arch variable made the mistake of misunderstanding that.

> ...and we cannot fix that without changing bash or a major
> rewrite of the shell scripts.

Why is either needed? As I see it, the cure is to define a pair of new
statement types in the config language, as follows:

 1. dep_arch_bool CONFIG_var CONFIG_arch [CONFIG_other_var...]

    This states that CONFIG_var is a boolean var that depends both on
    CONFIG_arch being specifically "y" and on CONFIG_other_var being
    such as to satisfy dep_bool as currently.

 2. dep_arch_tristate CONFIG_var CONFIG_arch [CONFIG_other_var...]

    This is the tristate version of dep_arch_bool as expected.

When we've done that, ALL of the problem lines can be trivially
converted from dep_ to dep_arch_ and the problem vanishes. As a free
bonus, the need for the arch-specific config files vanishes as well,
and each option can be documented as to what architectures it is valid
for. This thus removes a possible source of bugs from the equation.

The enclosed patch (against 2.4.5 raw) adds these two statements to
both the `make config` and `make menuconfig` scripts. I don't
understand TCL/TK so can't add them to the `make xconfig` script, and
as I also don't understand PERL, I can't add it to `make checkconfig`
so somebody else will need to deal with those two scripts.

> There are two solutions, either change all such lines to
> if [ "$CONFIG_some_arch" = "y" ];then
> tristate CONFIG_some_driver
> fi
> or
> define_bool CONFIG_some_arch n
> for all architectures at the start, followed by turning on the
> one that is required.

Both are messy, and best described as kludges. I wouldn't support

> Lots of if statements are messy and they will not prevent
> somebody adding new options with exactly the same problem.


> Explicitly setting all but one arch variable to 'n' results in
> cleaner config scripts and new arch dependent driver settings
> will automatically work.

Until somebody adds a new port and fails to upgrade any one of the
various files doing this. That's an open recipe for bugs IMHO, and
should be avoided at all costs.

Best wishes from Riley.

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/

This archive was generated by hypermail 2b29 : Sat Jul 07 2001 - 21:00:09 EST