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

Simon Glass sjg at chromium.org
Tue Feb 26 17:10:53 CET 2013


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 an example of how to use this, this series replaces all but two #ifdefs
from the main code body of common/main.c, which has the dubious distinction
of having the most #ifdefs by at least one measure:

$ for f in $(find . -name *.c); do echo $(grep -c "ifdef" $f) $f; done \
	|sort -nr |head
57 ./common/main.c
57 ./arch/powerpc/cpu/mpc83xx/cpu_init.c
48 ./arch/powerpc/lib/board.c
46 ./drivers/video/cfb_console.c
40 ./drivers/mtd/cfi_flash.c
38 ./net/tftp.c
38 ./common/cmd_bootm.c
37 ./drivers/usb/host/ohci-hcd.c
36 ./drivers/fpga/ivm_core.c
35 ./drivers/usb/gadget/ether.c

Code size for this series seems to be roughly neutral (below numbers are
average change in byte size for each region:

       x86: (for 1/1 boards)  all -12.0  data +4.0  text -16.0
      m68k: (for 41/50 boards)  all -23.7  text -23.7
   powerpc: (for 633/635 boards)  all -4.4  bss +2.0  data +0.0  text -6.4
   sandbox: (for 1/1 boards)  all +16.0  bss +16.0
        sh: (for 16/21 boards)  all -4.8  bss +3.2  text -8.0
     nios2: (for 3/3 boards)  all +24.0  bss +1.3  data -1.3  text +24.0
       arm: (for 292/292 boards)  all -11.1  bss +6.0  spl/u-boot-spl:all +0.4  spl/u-boot-spl:text +0.4  text -17.1
     nds32: (for 3/3 boards)  all -42.7  bss +10.7  text -53.3

Note that a config_drop.h file is added - this defines all the CONFIGs
which are not used in any board config file. Without this, autoconf cannot
define the macros for this CONFIGs.

Compile time for main.c does not seem to be any different in my tests. The
time to perform the 'dep' step (which now creates autoconf.h) increases,
from about 2.8s to about 4.6s. This additional time is used to grep, sed
and sort the contents of all the header file in U-Boot. The time for an
incremental build is not affected.

It would be much more efficient to maintain a list of all available CONFIG
defines, but no such list exists at present.

Changes in v3:
- Update config_xxx() to autoconf_xxx() in comments/README/sed
- Update config_xxx_enabled() to autoconf_has_xxx() in comments/README/sed
- Add comment as to why we use [A-Za-z0-9_][A-Za-z0-9_]*
- Rename sed scripts to more useful names
- Fix commit message which said autoboot() instead of abortboot()
- Add note about CONFIG_MENU not being needed in main.c anymore
- Fix missing && in if() statement
- Remove unneeded retry_min variable
- Simplify code for finding out bootdelay from config or environment
- Separate out checkpatch fixes in command line reading code into new patch
- Remove the extra config_of_libfdt() condition in main_loop()

Changes in v2:
- Split out changes to main.c into separate patches
- Fix up a few errors and comments in the original RFC
- Use autoconf_...() instead of config_...()
- Use autoconf_has_...() instead of config_..._enabled()
- Add a grep to the sed/sort pipe to speed up processing

Simon Glass (16):
  Implement autoconf header file
  at91: Correct CONFIG_AUTOBOOT_PROMPT definition for pm9263
  net: Add prototype for update_tftp, and use autoconf
  main: Separate out the two abortboot() functions
  main: Move boot_delay code into its own function
  main: Use autoconf for boot retry feature
  main: Remove CONFIG #ifdefs from the abortboot() code
  main: Use get/setenv_ulong()
  main: Use autoconf for boot_delay code
  main: Use autoconf for parser selection
  main: Fix typos and checkpatch warnings in command line reading
  main: Use autoconf in command line reading
  main: Use autoconf in main_loop()
  main: Correct header order
  main: Add debug_parser() to avoid #ifdefs
  main: Add debug_bootkeys to avoid #ifdefs

 Makefile                       |  43 ++-
 README                         |  87 ++++-
 common/cmd_fitupd.c            |   3 +-
 common/main.c                  | 809 ++++++++++++++++++-----------------------
 common/update.c                |  24 +-
 include/command.h              |   2 -
 include/common.h               |   9 +-
 include/config_drop.h          |  17 +
 include/configs/pm9263.h       |   2 +-
 include/fdt_support.h          |   4 +-
 include/hush.h                 |   2 -
 include/menu.h                 |   2 -
 include/net.h                  |   3 +
 tools/scripts/define2value.sed |  37 ++
 tools/scripts/define2zero.sed  |  32 ++
 15 files changed, 580 insertions(+), 496 deletions(-)
 create mode 100644 include/config_drop.h
 create mode 100644 tools/scripts/define2value.sed
 create mode 100644 tools/scripts/define2zero.sed

-- 
1.8.1.3



More information about the U-Boot mailing list