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

[Simon Glass]

Hi Tom,

On Tue, May 14, 2013 at 3:28 PM, Tom Rini <trini at ti.com> wrote:
> On Tue, May 14, 2013 at 03:22:04PM -0700, Vadim Bendebury wrote:
>> On Tue, May 14, 2013 at 3:15 PM, Tom Rini <trini at ti.com> wrote:
>> > On Tue, May 14, 2013 at 02:27:32PM -0700, Simon Glass wrote:
>> >> Hi Tom,
>> >>
>> >> On Tue, May 14, 2013 at 2:21 PM, Tom Rini <trini at ti.com> wrote:
>> >> > On Mon, May 13, 2013 at 03:12:07PM -0700, Simon Glass wrote:
>> >> >> Hi Tom,
>> >> >>
>> >> >> On Thu, Mar 21, 2013 at 7:38 AM, Tom Rini <trini at ti.com> wrote:
>> >> >> > On Tue, Feb 26, 2013 at 08:10:53AM -0800, Simon Glass 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.
>> >> >> >>
>> >> >> >> The use of #ifdef in U-Boot has possibly got a little out of hand. In an
>> >> >> >> attempt to turn the tide, this series includes a patch which provides a way
>> >> >> >> to make CONFIG macros available to C code without using the preprocessor.
>> >> >> >> This makes it possible to use standard C conditional features such as
>> >> >> >> if/then instead of #ifdef. A README update exhorts compliance.
>> >> >> >
>> >> >> > OK, this is true.  Looking over the series, a number of the patches are
>> >> >> > just general fixes / improvements that don't depend on the autoconf_...
>> >> >> > work.  Lets split this out now and take them in now as they seem like
>> >> >> > reviewable by inspection code.
>> >> >>
>> >> >> Well sorry I didn't make time to get this done last time. I can do
>> >> >> this now or...
>> >> >>
>> >> >> >
>> >> >> > For the approach itself, I'm not sure which is best here.  In a lot of
>> >> >> > cases we're trading an #ifdef for adding a level of indent to already
>> >> >> > pretty indented code and that feels like a wash, in terms of readability
>> >> >> > to me.  We probably need to re-factor some of the code in question there
>> >> >> > too for readability, then see about using autoconf_... type things, or
>> >> >> > maybe something else.
>> >> >>
>> >> >> I think you are saying to do the rearrangement and clean-up first,
>> >> >> then add autoconf afterwards. I can do that but really I am wondering
>> >> >> what you think of the autoconf concept? The Kconfig stuff is related
>> >> >> here too, but first I would like to decide on what to do with the
>> >> >> #ifdefs.
>> >> >
>> >> > I think a lot of our #ifdefery is a problem of code that's in need of
>> >> > some love and re-org and cleaning and updating.  One of the old style
>> >> > rules I still try and follow is that after a few levels of indent code
>> >> > doesn't read well.  Also big nested #ifdefs don't read well.  So we're
>> >> > trading one in for the other.  But your series showed a lot of places
>> >> > where we can re-factor things to improve readability.  So lets go that
>> >> > way.  Then we can see if there's still things to improve on, and what
>> >> > dead code we still have around.
>> >>
>> >> So are you saying that you are keen on the autoconf idea?
>> >
>> > I'm saying lets clean up the code and see if we still need something
>> > like autoconf.  It seems to provide the most benefit in terms of
>> > readability in places that could read a lot better with some clean up
>> > and refactoring before we add new tools to the pile.
>> >
>>
>> Yet another great advantage of autoconf is that it ensures a
>> consistent combination of the configuration options, with the status
>> quo it is so easy to make a mistake and create a deficient
>> configuration.
>
> There are things I like about the concept, but I really want to see the
> problem areas in question made more readable as it will both help in
> general, and if we do make this conversion later, make the conversion
> easier as we'll hopefully kill off some of the nested and tricky ifdefs.

I've brought over the patches that I can that don't depend on autoconf:

6c0e6c9 (HEAD, ws/us-config5, us-config5) main: Add debug_bootkeys to
avoid #ifdefs
9777b9f main: Add debug_parser() to avoid #ifdefs
2fc85b6 main: Correct header order
24e80be main: Fix typos and checkpatch warnings in command line reading
9e9e3b9 main: Use get/setenv_ulong()
1290cb7 main: Move boot_delay code into its own function
cd8f13e main: Separate out the two abortboot() functions
ca2451c net: Add prototype for update_tftp
4a2a802 at91: Correct CONFIG_AUTOBOOT_PROMPT definition for pm9263

I will run these through the builder and send an interim series
without autoconf. We still have a lot of inline #ifdefs, and static
functions and local variables must also be #ifdefed out if not used.
Still, there is some improvement.

Regards,
Simon