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

[Vadim Bendebury]

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.

--vb

> --
> Tom