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

Simon Glass sjg at chromium.org
Mon Jun 17 16:44:52 CEST 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 is one of the largest users
of #ifdef, even after a recent cleanup:

for f in $(find . -name *.c); do echo $(grep -c "ifdef" $f) $f; do
ne |sort -nr |head
81 ./common/board_r.c
57 ./arch/powerpc/cpu/mpc83xx/cpu_init.c
55 ./common/board_f.c
49 ./common/main.c
48 ./arch/powerpc/lib/board.c
47 ./drivers/video/cfb_console.c
40 ./drivers/mtd/cfi_flash.c
38 ./net/tftp.c
37 ./drivers/usb/host/ohci-hcd.c
36 ./drivers/fpga/ivm_core.c

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

  blackfin: (for 24/35 boards)  all -11.0  text -11.0
       x86: (for 1/1 boards)  bss +20.0  data +4.0  text -24.0
     avr32: (for 10/10 boards)  all -8.4  text -8.4
   sandbox: (for 1/1 boards)  all +16.0  bss +16.0
      m68k: (for 41/50 boards)  all -31.9  text -31.9
   powerpc: (for 639/641 boards)  all -20.5  bss +0.0  rodata -0.5  text -20.0
     sparc: (for 5/5 boards)  all -28.8  text -28.8
        sh: (for 16/21 boards)  all -78.2  bss +3.2  rodata -15.5  text -66.0
     nios2: (for 3/3 boards)  all +24.0  bss +1.3  data -1.3  text +24.0
       arm: (for 307/327 boards)  all -41.0  bss +3.5  data +0.1  rodata -3.6
		spl/u-boot-spl:all -0.1  spl/u-boot-spl:bss -0.1  text -41.0

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.

Buildman output shows no additional failures from mainline:
01: Merge branch 'master' of git://www.denx.de/git/u-boot-mmc
  blackfin: +   bf561-acvilon cm-bf561 blackstamp br4 bct-brettl2 cm-bf527 dnp5370 bf506f-ezkit ip04 bf527-sdp bf609-ezkit bf537-stamp bf527-ezkit-v2 cm-bf537e tcm-bf518 cm-bf537u bf527-ezkit bf537-pnav cm-bf533 pr1 bf533-ezkit ibf-dsp561 bf537-srv1 cm-bf548 bf537-minotaur bf538f-ezkit bf548-ezkit bf525-ucr2 blackvme tcm-bf537 bf533-stamp bf518f-ezbrd bf527-ad7160-eval bf526-ezbrd bf561-ezkit
      m68k: +   M54455EVB_a66 M5329AFEE M5249EVB idmr M5208EVBE M5475FFE M54451EVB astro_mcf5373l M54418TWR_serial_rmii M54455EVB_intel M5282EVB M54455EVB_i66 M5475GFE M5253DEMO M54455EVB_stm33 M5485BFE M5485DFE M5329BFEE M52277EVB M5475EFE M5475CFE M5485AFE M53017EVB M5475AFE M5485HFE M5235EVB M5253EVBE M54418TWR_nand_mii M54418TWR_nand_rmii_lowfreq TASREG cobra5272 M5475BFE M5475DFE M5275EVB M52277EVB_stmicro eb_cpu5282 eb_cpu5282_internal M54451EVB_stmicro M5271EVB M5485GFE M5485EFE M5485FFE M54418TWR M5235EVB_Flash32 M5373EVB M54418TWR_nand_rmii M54418TWR_serial_mii M5485CFE M54455EVB M5272C3
   powerpc: +   MVBLM7 MVSMR lcd4_lwmon5
        sh: +   rsk7269 rsk7264 sh7757lcr sh7752evb rsk7203
microblaze: +   microblaze-generic
  openrisc: +   openrisc-generic
       arm: +   palmtc zipitz2 VCMA9 lubbock zynq_dcc vpac270_nor_128 colibri_pxa270 kzm9g zynq xaeniax polaris pxa255_idp lp8x4x vpac270_ond_256 vpac270_nor_256 smdk2410 h2200 balloon3 palmld trizepsiv
     nds32: +   adp-ag101p adp-ag102 adp-ag101
02: Implement autoconf header file
03: main: Use autoconf for boot retry feature
04: main: Remove CONFIG #ifdefs from the abortboot() code
05: main: Use autoconf to remove #ifdefs around process_boot_delay()
06: main: Use autoconf for boot_delay code
07: main: Use autoconf for parser selection
08: main: Use autoconf in command line reading
09: main: Use autoconf in main_loop()


Changes in v4:
- Rebase on current master
- Split out new patch to remove #ifdefs around process_boot_delay()
- Tidy up code style nits with new checkpatch

Changes in v3:
- Add comment as to why we use [A-Za-z0-9_][A-Za-z0-9_]*
- Fix missing && in if() statement
- Remove the extra config_of_libfdt() condition in main_loop()
- Remove unneeded retry_min variable
- Rename sed scripts to more useful names
- Simplify code for finding out bootdelay from config or environment
- Update config_xxx() to autoconf_xxx() in comments/README/sed
- Update config_xxx_enabled() to autoconf_has_xxx() in comments/README/sed

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

Simon Glass (8):
  Implement autoconf header file
  main: Use autoconf for boot retry feature
  main: Remove CONFIG #ifdefs from the abortboot() code
  main: Use autoconf to remove #ifdefs around process_boot_delay()
  main: Use autoconf for boot_delay code
  main: Use autoconf for parser selection
  main: Use autoconf in command line reading
  main: Use autoconf in main_loop()

 Makefile                       |  43 ++-
 README                         |  87 +++++-
 common/main.c                  | 629 ++++++++++++++++++-----------------------
 include/command.h              |   2 -
 include/common.h               |   6 +-
 include/config_drop.h          |  17 ++
 include/fdt_support.h          |   4 +-
 include/hush.h                 |   2 -
 include/menu.h                 |   2 -
 tools/scripts/define2value.sed |  37 +++
 tools/scripts/define2zero.sed  |  32 +++
 11 files changed, 492 insertions(+), 369 deletions(-)
 create mode 100644 include/config_drop.h
 create mode 100644 tools/scripts/define2value.sed
 create mode 100644 tools/scripts/define2zero.sed

-- 
1.8.3



More information about the U-Boot mailing list