[U-Boot] doing anything about "bad" Kbuild configuration options?

Fri Apr 12 16:32:12 UTC 2019

  only a few months ago, i jumped into a discussion regarding Kbuild
config options that were defined in some Kconfig* file, but were
unused anywhere in the source tree:

http://u-boot.10912.n7.nabble.com/policy-regarding-unused-code-td351802.html#a351835

  many years ago, i wrote a script to locate such entries in the linux
kernel source tree and, once u-boot adoopted the Kbuild build system,
i could obviously use the same script on the u-boot source -- so far,
so good. but i wrote some associated scripts to locate other oddities,
and one of them was to find what i called "badref" options.

  now, "unused" options are typically not serious -- a Kbuild option
is defined that is simply not tested anywhere in the source. normally,
that's because someone removed some source and just forgot to remove
the associated Kconfig option.

  "badref" options, on the other hand, are exactly the opposite --
options that *are* tested somewhere in the code, but are not defined
anywhere in a Kconfig file. in short, the code is testing some
CONFIG_* option that will never, ever succeed as it isn't defined,
which should typically be treated as a potentially more serious issue.

  i wrote up an example of this here:

https://crashcourse.ca/dokuwiki/doku.php?id=u-boot_badref

where you can see the result of running the script and asking it to
scan the entire u-boot "drivers" directory once upon a time -- the
output consists of a string for which the corresponding
CONFIG_<string> is being tested somewhere in the argument directory,
but for which there is no apparent Kconfig definition; the output for
each string consists of a tree-wide search for that string.

  as an example, note the very first example found:

>>>>> ACX517AKN
drivers/video/pxa_lcd.c:201:#ifdef CONFIG_ACX517AKN
drivers/video/pxa_lcd.c:231:#endif /* CONFIG_ACX517AKN */
drivers/video/pxa_lcd.c:297:#endif /* CONFIG_ACX517AKN */
scripts/config_whitelist.txt:15:CONFIG_ACX517AKN

so CONFIG_ACX517AKN is clearly being tested, while there is no
definition for that option *anywhere* in the tree. (the script might
not be perfect, there might be the occasional false negative or
positive, but one can always verify manually if there's any doubt.)

  one of the worst culprits appears to be CONFIG_SPL_BUILD, which
appears all over the tree, but one can see a recent commit that takes
that into account:

  commit 0ef692084363f2de8547db93397c6a69123d26ca
  Author: Baruch Siach <baruch at tkos.co.il>
  Date:   Thu Feb 7 13:21:16 2019 +0200

    Kconfig: fix BUILD_TARGET for ARCH_MVEBU

    Commit dc146ca11187 ("Kconfig: Migrate CONFIG_BUILD_TARGET") made the
    mvebu default build target depend on CONFIG_SPL_BUILD. Unfortunately,
    there is no such Kconfig symbol. Use the CONFIG_SPL symbol instead to
    fix that.

    Cc: Jagan Teki <jagan at amarulasolutions.com>
    Signed-off-by: Baruch Siach <baruch at tkos.co.il>
    Reviewed-by: Stefan Roese <sr at denx.de>
    Reviewed-by: Jagan Teki <jagan at amarulasolutions.com>
    Signed-off-by: Stefan Roese <sr at denx.de>

  the output for that example in my wiki page is a bit overwhelming,
which is why my badref script takes an optional directory to search,
so you can focus your attention as finely as you want. as an example,
check only drivers/mmc, which appears to find exactly four bad
references:

$ find_badref_configs.sh drivers/mmc
>>>>> HSMMC3_8BIT
drivers/mmc/omap_hsmmc.c:1577:#if defined(CONFIG_DRA7XX) && defined(CONFIG_HSMMC3_8BIT)
>>>>> MMC_RPMB_TRACE
drivers/mmc/rpmb.c:98:#ifdef CONFIG_MMC_RPMB_TRACE
drivers/mmc/rpmb.c:115:#ifdef CONFIG_MMC_RPMB_TRACE
drivers/mmc/rpmb.c:131:#ifdef CONFIG_MMC_RPMB_TRACE
drivers/mmc/rpmb.c:147:#ifdef CONFIG_MMC_RPMB_TRACE
drivers/mmc/rpmb.c:154:#ifdef CONFIG_MMC_RPMB_TRACE
scripts/config_whitelist.txt:1225:CONFIG_MMC_RPMB_TRACE
>>>>> MMC_SPI_CRC_ON
drivers/mmc/mmc_spi.c:94:#ifdef CONFIG_MMC_SPI_CRC_ON
drivers/mmc/mmc_spi.c:123:#ifdef CONFIG_MMC_SPI_CRC_ON
drivers/mmc/mmc.c:2259:#ifdef CONFIG_MMC_SPI_CRC_ON
scripts/config_whitelist.txt:1228:CONFIG_MMC_SPI_CRC_ON
arch/arm/include/asm/arch-pxa/regs-mmc.h:66:#define MMC_SPI_CRC_ON			(1 << 1)
>>>>> ZYNQ_HISPD_BROKEN
drivers/mmc/zynq_sdhci.c:263:#ifdef CONFIG_ZYNQ_HISPD_BROKEN
scripts/config_whitelist.txt:4637:CONFIG_ZYNQ_HISPD_BROKEN
$

  in any event, i am not planning to do anything about this, i just
thought others might want to check whatever part of the code base for
which they are a maintainer, and see if any such things exist. and as
forrest gump once said, "that's all i have to say about that."

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                         http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================