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

[Detlev Zundel]

Hi Simon,

[...]

>> I think the Linux code has two big advantages - for one, we increase the
>> overlap with Linux kernel proper and secondly we keep the 'grep'ability
>> of the names which I really missed in your proposal.
>
> Yes that seems reasonable.

Ok, I'm glad we are in agreement.

>>> Many of U-Boot's options are not just yes/no, so I'm not sure what will
>>> happen with those. Perhaps they just can't be used with this macro? Part of
>>> my intent was to allow any option to be used.
>>
>> In those cases the defines then are a "shortcut" to using a boolean + a
>> value and we can make it work by introducing this boolean?  I have no
>> idea of how much work this would be, but it might be worthwhile getting
>> some real numbers to that.
>>
>>> So for example you can do this:
>>>
>>> struct {
>>>> const char *str;
>>>> u_int len;
>>>> int retry;
>>>> const char *conf; /* Configuration value */
>>>> }
>>>> delaykey [] = {
>>>> { str: getenv("bootdelaykey"),  retry: 1,
>>>> conf: autoconf_autoboot_delay_str() },
>>>> { str: getenv("bootdelaykey2"), retry: 1,
>>>> conf: autoconf_autoboot_delay_str2() },
>>>> { str: getenv("bootstopkey"),   retry: 0,
>>>> conf: autoconf_autoboot_stop_str() },
>>>> { str: getenv("bootstopkey2"),  retry: 0,
>>>> conf: autoconf_autoboot_stop_str2() },
>>>> };
>>>
>>>
>>>
>>> or this:
>>>
>>> /* don't retry auto boot? */
>>>> if (autoconf_boot_retry_time() &&
>>>>     !delaykey[i].retry)
>>>> retry_time = -1;
>>>
>>>
>>> Note that autoconf_boot_retry_time() will return 0 if the CONFIG is not
>>> defined, or if its value is 0.
>>
>> I'm having real trouble figuring out what this would do on first sight.
>> Of course you could call me lazy, but by experience I tend to favour
>> solutions that do not need geniuses to understand ;)
>
> Well it is simply that we currently have options which may or may not
> be defined. If they are not defined, then it is an error to refer to
> them, so they must be guarded by an #ifdef. By defining them to 0 when
> not defined, we can avoid that guard.

Ok, I see.  But in this way we are actually shutting up the compiler on
code paths that we did not have before.  This in effect is a "rather be
quiet than error" strategy and I'm not sure that I want to use that
when doing such changes.  Call me old-fashioned, but I'd prefer to throw
an error and fix it after thinking it through from todays perspective

(I can say this easily if I'm not the one who has to do the fixes ;)

>>> It seems to me we should provide the Linux feature in U-Boot as part of the
>>> Kconfig stuff, and see where it takes us. Combined with a bit more
>>> rationalisation of things like main.c it might be enough.
>>
>> Why not reimplement your patch set along those lines?  I still really
>> would _love_ to see us using the compiler more to check for errors and
>> reduce the number of "potential source code combination" that we have.
>
> Well certainly I could, but I did not want to do this while
> Kbuild/Kconfig is in progress, and we are not quite clear on what to
> do. I think Kbuild is done - we will probably get the Linux autoconf
> stuff for free with Kconfig which I understand is coming very soon.

This makes a lot of sense yes.  Actually I'm pretty excited about this
excellent and continuous work from Masahiro-san on that topic!

> After that I can certainly take another look at main.c and see how it
> can be improved.

Sure, its only that I wanted to keep the ball rolling as I really
believe the wins to be had from such a change are substantial for our
codebase.

Cheers
  Detlev

-- 
Golden rule #12:   When the comments do not match the code, they probably are
both wrong. -- Steven Rostedt <1300126962.9910.128.camel at gandalf.stny.rr.com>
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de