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

[Wolfgang Denk]

Dear Simon Glass,

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.
> 
> 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.

As mentioned before, I'm really interested in seeing something like
this going into mainline, but I have some doubts about the actual
implementation.

To summarize:  Your current proposal was to convert code snippets
like this:

	#ifdef CONFIG_VERSION_VARIABLE
		setenv("ver", version_string);  /* set version variable */
	#endif

into

	if (autoconf_version_variable())
		setenv("ver", version_string);  /* set version variable */

By chance I ran about "include/linux/kconfig.h" in the Linux kernel
tree, which provides (among other things) the IS_ENABLED() macro that
implements essentially the very same feature.  Using this, the same
code would be written as:

	if (IS_ENABLED(CONFIG_VERSION_VARIABLE))
		setenv("ver", version_string);  /* set version variable */

I agree that this does not solve some of the isses that have been
raised about this change (indentation level increses - which may in
turn require reformatting of bigger parts of the code; code becomes
less readable), but on the other hand it avoids the need for a new
autoconf header file, and it should be possible to introduce this
easily step by step.

And I really like the idea of re-using existing code that is already
known to Linux hackers, especially as we we are currently having our
eyes on the Kconfig stuff anyway.


What do you think?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
PROGRAM - n.  A magic spell cast over a computer  allowing it to turn
one's input into error messages.
v. tr. - To engage in a pastime similar to banging one's head against
a wall, but with fewer opportunities for reward.