[U-Boot] [PATCH v4 0/8] Provide a mechanism to avoid using #ifdef everywhere

[Simon Glass]

Hi Tom,

On Mon, Oct 28, 2013 at 9:32 AM, Tom Rini <trini at ti.com> wrote:
> On Mon, Oct 28, 2013 at 03:44:01PM +0100, Wolfgang Denk wrote:
>> Dear Simon,
>>
>> In message <1382800457-26608-1-git-send-email-sjg at chromium.org> you wrote:
>> > Many parts of the U-Boot code base are sprinkled with #ifdefs. This makes
>> > different boards compile different versions of the source code, meaning
>> > that we must build all boards to check for failures. It is easy to misspell
>> > an #ifdef and there is not as much checking of this by the compiler. Multiple
>> > dependent #ifdefs are harder to do than with if..then..else. Variable
>> > declarations must be #idefed as well as the code that uses them, often much
>> > later in the file/function. #ifdef indents don't match code indents and
>> > have their own separate indent feature. Overall, excessive use of #idef
>> > hurts readability and makes the code harder to modify and refactor. For
>> > people coming newly into the code base, #ifdefs can be a big barrier.
>>
>> While I agree in general with the goal and the implementation, there
>> is an implementation detail I dislike - this is that the new code
>> is harder to type and to read - I mean, something like
>> CONFIG_SYS_HUSH_PARSER is already clumsy enough, but making this
>> autoconf_sys_hush_parser() appears to be worse.  Also, the connection
>> to a CONFIG_* option is not easily visible.
>>
>>
>> I also had feedback from Detlev (who is unfortunately on a business
>> trip again so he didn't find time [yet] to comment himself); he
>> commented that he really likes the idea, but does not like that we now
>> have to access the well-known contants using a new name.
>>
>>
>> Maybe we can find a shorter / easier to read way to do this - I think
>> it would be really nice if we could see the well-known names.
>
> I guess this is what I get for not being like Linus sometimes[1].  I think
> what this series highlights is that we have a lot of code in
> common/main.c that needs either re-thinking, re-factoring or splitting
> out into difference functions and files.  And when we're turning
> #ifdef CONFIG_FOO
>         ... whatever ...
> #endif
> into
>         if (autoconf_foo()) {
>                 ... whatever ...
>         }
>
> The code starts looking worse in some cases since we're already 3
> indents in.  The problem is we have lots of ifdef code, in some areas of
> the code.  This hides the problem, not fixes the problem.

While I agree with this, the question is more whether using the
compiler instead of the preprocessor helps with reducing the number of
code paths and the number of boards we must build to test things. In
many cases the CONFIG options hide features that we don't want to
enable.

I did a series that improved main.c in various ways including
splitting the code differently - there are a few more things that
could be done, but it will still be a mountain of #ifdefs.

I would quite like to split the command editing stuff into its own
file - in fact main.c could be split into several files:

- command line
- command editing
- parser

But does this help? I wasn't entirely sure.

>
> Looking over the first parts of the series, weak functions for example
> would help in a number of cases, especially if we split things out of
> main.c and into other files too.

I am not a huge fan of weak functions since it isn't clear what
happens when they are called. But again if you have a specific example
it would help me understand your intent here.

Regards,
Simon

>
> --
> Tom
>
> [1]: By which I mean being very "forceful" when saying I don't like an
> idea.

BTW I am not sure about this idea either - let's figure out exactly
what it can help with and whether it is worth it. Perhaps main.c was
not the best choice - but board files have loads of #ifdefs also.