Add script usage_of_is_enabled_check to print any configs that use CONFIG_IS_ENABLED instead of IS_ENABLED and vice versa.
Add usage_of_is_enabled_commit.sh to generate commits to fix the above issues.
You can remove entries from test/usage_of_is_enabled_todo.txt or the entire file and then run test/usage_of_is_enabled_commit.sh to convert to suggested usage of CONFIG_IS_ENABLED/IS_ENABLED
or run test/usage_of_is_enabled_check.sh to see which configs are still todo.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Troy Kisky troykiskyboundary@gmail.com ---
(no changes since v1)
.azure-pipelines.yml | 11 ++ .gitlab-ci.yml | 5 + test/usage_of_is_enabled_check.sh | 19 +++ test/usage_of_is_enabled_commit.sh | 12 ++ test/usage_of_is_enabled_correct.sh | 50 +++++++ test/usage_of_is_enabled_exempt.txt | 9 ++ test/usage_of_is_enabled_list.sh | 86 +++++++++++ test/usage_of_is_enabled_splcfg.txt | 21 +++ test/usage_of_is_enabled_todo.txt | 213 ++++++++++++++++++++++++++++ 9 files changed, 426 insertions(+) create mode 100755 test/usage_of_is_enabled_check.sh create mode 100755 test/usage_of_is_enabled_commit.sh create mode 100755 test/usage_of_is_enabled_correct.sh create mode 100644 test/usage_of_is_enabled_exempt.txt create mode 100755 test/usage_of_is_enabled_list.sh create mode 100644 test/usage_of_is_enabled_splcfg.txt create mode 100644 test/usage_of_is_enabled_todo.txt
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 3c1846a5bc3..9b5a0ab6459 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -67,6 +67,17 @@ stages: :^doc/ :^arch/arm/dts/ :^scripts/kconfig/lkc.h :^include/linux/kconfig.h :^tools/ && exit 1 || exit 0
+ - job: check_usage_of_is_enabled + displayName: 'Check usage of CONFIG_IS_ENABLED vs IS_ENABLED' + pool: + vmImage: $(ubuntu_vm) + container: + image: $(ci_runner_image) + options: $(container_option) + steps: + # generate list of SPL configs + - script: test/usage_of_is_enabled_check.sh + - job: cppcheck displayName: 'Static code analysis with cppcheck' pool: diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e6c6ab3586e..2591feabfe6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -148,6 +148,11 @@ check for new CONFIG symbols outside Kconfig: :^doc/ :^arch/arm/dts/ :^scripts/kconfig/lkc.h :^include/linux/kconfig.h :^tools/ && exit 1 || exit 0
+check usage of CONFIG_IS_ENABLED vs IS_ENABLED: + stage: testsuites + script: + - ./test/usage_of_is_enabled_check.sh + # QA jobs for code analytics # static code analysis with cppcheck (we can add --enable=all later) cppcheck: diff --git a/test/usage_of_is_enabled_check.sh b/test/usage_of_is_enabled_check.sh new file mode 100755 index 00000000000..6bd5d9c1ac7 --- /dev/null +++ b/test/usage_of_is_enabled_check.sh @@ -0,0 +1,19 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0+ +# +# Written by Troy Kisky troykiskyboundary@gmail.com + +scriptdir=`dirname "$0"`; +${scriptdir}/usage_of_is_enabled_list.sh | grep -vw FOO; +if [ $? -eq 0 ] ; then + echo "The above may have incorrect usage of IS_ENABLED/"\ +"CONFIG_IS_ENABLED" + echo "Run test/usage_of_is_enabled_commit.sh and "\ +"squash with appropriate commit" + ret=1; +else + ret=0; +fi + +rm ${scriptdir}/splcfg.tmp ${scriptdir}/exclude.tmp +exit ${ret} diff --git a/test/usage_of_is_enabled_commit.sh b/test/usage_of_is_enabled_commit.sh new file mode 100755 index 00000000000..593dbd1428c --- /dev/null +++ b/test/usage_of_is_enabled_commit.sh @@ -0,0 +1,12 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0+ +# +# Written by Troy Kisky troykiskyboundary@gmail.com + +scriptdir=`dirname "$0"`; +${scriptdir}/usage_of_is_enabled_list.sh | \ +xargs -I {} sh -c "${scriptdir}/usage_of_is_enabled_correct.sh {}; \ +git commit -a -m"CONFIG_{}: correct usage of CONFIG_IS_ENABLED/IS_ENABLED";" + + +rm ${scriptdir}/splcfg.tmp ${scriptdir}/exclude.tmp diff --git a/test/usage_of_is_enabled_correct.sh b/test/usage_of_is_enabled_correct.sh new file mode 100755 index 00000000000..8724747beed --- /dev/null +++ b/test/usage_of_is_enabled_correct.sh @@ -0,0 +1,50 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0+ +# +# Written by Troy Kisky troykiskyboundary@gmail.com + +scriptdir=`dirname "$0"`; + +if [ -z "$1" ] ; then + echo missing config + exit 1; +fi +if [ ! -f "${scriptdir}/splcfg.tmp" ] ; then + echo missing splcfg.tmp + exit 1; +fi + + +grep -qw $1 ${scriptdir}/splcfg.tmp +if [ $? -ne 0 ] ; then + # not splcfg + # change CONFIG_IS_ENABLED to IS_ENABLED + git grep -l \ + -e "CONFIG_IS_ENABLED($1)" \ + | \ + xargs -IFile sh -c \ + " \ + sed -i -E "\ +s/CONFIG_IS_ENABLED($1)/IS_ENABLED(CONFIG_$1)/g; \ +" File"; +else + # splcfg + # change IS_ENABLED to CONFIG_IS_ENABLED + # change ifdef to CONFIG_IS_ENABLED + # change ifndef to !CONFIG_IS_ENABLED + # change defined to CONFIG_IS_ENABLED + git grep -l \ + -e "IS_ENABLED(CONFIG_$1)" \ + -e "^#ifdef[ \t]+CONFIG_$1>" \ + -e "^#ifndef[ \t]+CONFIG_$1>" \ + -e "defined(CONFIG_$1)" \ + | \ + xargs -IFile sh -c \ + " \ + sed -i -E "\ +s/([^_])IS_ENABLED(CONFIG_$1)/\1CONFIG_IS_ENABLED($1)/g; \ +s/^#ifdef[ \t]+CONFIG_$1>/#if CONFIG_IS_ENABLED($1)/; \ +s/^#ifndef[ \t]+CONFIG_$1>/#if !CONFIG_IS_ENABLED($1)/; \ +s/defined(CONFIG_$1)/CONFIG_IS_ENABLED($1)/; \ +" File"; +fi diff --git a/test/usage_of_is_enabled_exempt.txt b/test/usage_of_is_enabled_exempt.txt new file mode 100644 index 00000000000..d9fefd6cb6c --- /dev/null +++ b/test/usage_of_is_enabled_exempt.txt @@ -0,0 +1,9 @@ +BLOBLIST +BLOBLIST_FIXED +DM_PMIC_PFUZE100 +FOO +NAND_BOOT +OF_CONTROL +SYS_L2_PL310 +WATCHDOG +X86_64 diff --git a/test/usage_of_is_enabled_list.sh b/test/usage_of_is_enabled_list.sh new file mode 100755 index 00000000000..0f51d3602ca --- /dev/null +++ b/test/usage_of_is_enabled_list.sh @@ -0,0 +1,86 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0+ +# +# Written by Troy Kisky troykiskyboundary@gmail.com + +scriptdir=`dirname "$0"`; +# generate list of excluded configs +{ +# 1. ignore configs that have a number or string for a value +git grep -h -A2 -E "^config " '*Kconfig*' | \ +sed -En '/depends on/!p' | \ +sed -En '/^config/{h;$!d} ;H;x; s/config[ \t]+(.*)\n[ \t]*/config \1 #/p' | \ +sed -E "/#bool/d; /#def_bool/d; /#tristate/d; \ +/#default y/d; /#select/d; /#prompt/d; /#imply/d" | +sed -n -r "s/^config[[:space:]]+([0-9a-zA-Z_]+)/\n{\1}\n/p" | \ +sed -n -r 's/^{([0-9a-zA-Z_]+)}/\1/p' | sort -u; +# 2. configs that are exempt for other reasons +cat ${scriptdir}/usage_of_is_enabled_exempt.txt; +# 3. configs that need converted later +[ -f ${scriptdir}/usage_of_is_enabled_todo.txt ] && \ +cat ${scriptdir}/usage_of_is_enabled_todo.txt +} | sort -u > ${scriptdir}/exclude.tmp + +# generate list of CONFIGs that should use CONFIG_IS_ENABLED +{ +# 1. all obj-$(CONFIG_$(SPL_)xxx in Makefiles +git grep -h 'obj-$(CONFIG_$(SPL_' '*Makefile' | sed -e "s/SPL_TPL_/SPL_/"| \ +sed -n -r 's/obj-$(CONFIG_$(SPL_)([0-9a-zA-Z_]+))/\n{\1}\n/gp'| \ +sed -n -r 's/{([0-9a-zA-Z_]+)}/\1/p'; + +# 2. all SPL_xxx in Kconfig files +git grep -h -E 'config [ST]PL_' '*Kconfig*' | \ +sed -n -r "s/config [ST]PL_([0-9a-zA-Z_]+)/\n{\1}\n/p" | \ +sed -n -r 's/{([0-9a-zA-Z_]+)}/\1/p'; + +# 3. all CONFIG_CMD_xxx which already use CONFIG_IS_ENABLED +# The Makefile for most if these use ifndef CONFIG_SPL_BUILD +# instead of obj-$(CONFIG_$(SPL_)xxx +git grep -h -E 'CONFIG_IS_ENABLED(CMD_' | \ +sed -n -e "s/(CONFIG_IS_ENABLED(CMD_[0-9a-zA-Z_]*))/\n\1\n/gp"| \ +sed -n -r "s/CONFIG_IS_ENABLED((CMD_[0-9a-zA-Z_]+))/\1/p"; + +# 4. A list of other configs that should use CONFIG_IS_ENABLED +# This list could be reduced if obj-$(CONFIG_$(SPL_)xxx was used instead of +# ifndef CONFIG_SPL_BUILD in Makefiles +# usage_of_is_enabled_splcfg.txt mostly contains configs that should always +# be undefined in SPL/TPL +# Note: CONFIG_CLK was included to prevent a change in test_checkpatch.py +# which is checking for an error. +cat ${scriptdir}/usage_of_is_enabled_splcfg.txt; +} | sort -u | \ +comm -23 - ${scriptdir}/exclude.tmp >${scriptdir}/splcfg.tmp + +{ +# generate list of CONFIGs that incorrectly use CONFIG_IS_ENABLED +git grep -h CONFIG_IS_ENABLED | \ +sed -n -e "s/(CONFIG_IS_ENABLED([0-9a-zA-Z_]*))/\n\1\n/gp"| \ +sed -n -r "s/CONFIG_IS_ENABLED(([0-9a-zA-Z_]+))/\1/p" |sort -u| \ +comm -23 - ${scriptdir}/exclude.tmp | \ +comm -23 - ${scriptdir}/splcfg.tmp ; + +# generate list of CONFIGs that incorrectly use IS_ENABLED +git grep -h -w IS_ENABLED | \ +sed -n -e "s/(IS_ENABLED(CONFIG_[0-9a-zA-Z_]*))/\n\1\n/gp"| \ +sed -n -r "s/IS_ENABLED(CONFIG_([0-9a-zA-Z_]+))/\1/p" |sort -u| \ +join - ${scriptdir}/splcfg.tmp; + +# generate list of CONFIGs that incorrectly use ifdef +git grep -h -E "^#ifdef[ \t]+CONFIG_" | \ +sed -n -E "s/(ifdef[ \t]+CONFIG_[0-9a-zA-Z_]+)/\n\1\n/p"| \ +sed -n -E "s/ifdef[ \t]+CONFIG_([0-9a-zA-Z_]+)/\1/p" |sort -u| \ +join - ${scriptdir}/splcfg.tmp ; + +# generate list of CONFIGs that incorrectly use ifndef +git grep -h -E "^#ifndef[ \t]+CONFIG_" | \ +sed -n -E "s/(ifndef[ \t]+CONFIG_[0-9a-zA-Z_]+)/\n\1\n/p"| \ +sed -n -E "s/ifndef[ \t]+CONFIG_([0-9a-zA-Z_]+)/\1/p" |sort -u| \ +join - ${scriptdir}/splcfg.tmp ; + +# generate list of CONFIGs that incorrectly use defined +git grep -h -E "defined(CONFIG_" | \ +sed -n -E "s/(defined(CONFIG_[0-9a-zA-Z_]+))/\n\1\n/gp"| \ +sed -n -E "s/defined(CONFIG_([0-9a-zA-Z_]+))/\1/p" |sort -u| \ +join - ${scriptdir}/splcfg.tmp ; + +} | sort -u; diff --git a/test/usage_of_is_enabled_splcfg.txt b/test/usage_of_is_enabled_splcfg.txt new file mode 100644 index 00000000000..29d6257c5c7 --- /dev/null +++ b/test/usage_of_is_enabled_splcfg.txt @@ -0,0 +1,21 @@ +BZIP2 +CONFIG_CLK +CONSOLE_MUX +DM_EVENT +DM_HWSPINLOCK +DM_RNG +DM_STDIO +EFI_DEVICE_PATH_TO_TEXT +EFI_LOADER +ERRNO_STR +EVENT_DYNAMIC +GENERATE_SMBIOS_TABLE +IOMMU +MMC_HW_PARTITIONING +NAND_CS_INIT +OFNODE_MULTI_TREE +PINCTRL_ARMADA_38X +PRE_CONSOLE_BUFFER +RESET_MEDIATEK +RESET_ROCKCHIP +UT_DM diff --git a/test/usage_of_is_enabled_todo.txt b/test/usage_of_is_enabled_todo.txt new file mode 100644 index 00000000000..652637650dd --- /dev/null +++ b/test/usage_of_is_enabled_todo.txt @@ -0,0 +1,213 @@ +ACPIGEN +ARCH_MVEBU +ARCH_VERSAL_NET +ARM_PSCI_FW +ARMV8_SEC_FIRMWARE_SUPPORT +ATMEL_PIT_TIMER +BLK +BLOCK_CACHE +BOOTCOUNT_LIMIT +BOOTDEV_ETH +BOOTDEV_SPI_FLASH +BOOTSTAGE +BOOTSTD +BZIP2 +CLK +CLK_CCF +CLK_IMX6Q +CMD_DHCP +CMDLINE +CMD_PXE +CONSOLE_MUX +COREBOOT_SYSINFO +CPU +CRC32_VERIFY +CROS_EC_KEYB +DFU_SF_PART +DFU_VIRT +DISPLAY_AER_FULL +DM +DMA +DM_DMA +DM_ETH +DM_GPIO +DM_I2C +DM_KEYBOARD +DM_MMC +DM_PMIC +DM_PMIC_DA9063 +DM_REGULATOR +DM_RNG +DM_RTC +DM_SERIAL +DM_SPI +DM_SPI_FLASH +DM_USB +DM_USB_GADGET +DOS_PARTITION +DWC_AHSATA_AHCI +EFI_DT_FIXUP +EFI_EBBR_2_1_CONFORMANCE +EFI_LOADER +EFI_PARTITION +EFI_SCROLL_ON_CLEAR_SCREEN +EFI_TCG2_PROTOCOL_MEASURE_DTB +EFI_UNICODE_CAPITALIZATION +ENV_APPEND +ENV_IS_IN_EXT4 +ENV_IS_IN_FAT +ENV_IS_IN_FLASH +ENV_IS_IN_MMC +ENV_IS_IN_NAND +ENV_IS_IN_SPI_FLASH +ENV_IS_NOWHERE +ENV_WRITEABLE_LIST +ERRNO_STR +EVENT_DEBUG +EXPO +EXYNOS7870 +EXYNOS7880 +FASTBOOT_UUU_SUPPORT +FAT_WRITE +FIT +FIT_CIPHER +FIT_IMAGE_POST_PROCESS +FIT_SIGNATURE +FIT_VERBOSE +FPGA +FRU_SC +FSL_ISBC_KEY_EXT +FSL_LS_PPA +FS_LOADER +FSP_VERSION2 +GENERATE_ACPI_TABLE +GENERATE_SMBIOS_TABLE +GMAC_ROCKCHIP +GZIP +I2C_EEPROM +I8259_PIC +IMX_RDC +LED +LEGACY_IMAGE_FORMAT +LIB_UUID +LOG +LZ4 +LZMA +LZO +MALTA +MARY +MEMSIZE_IN_BYTES +MIPS_BOOT_CMDLINE_LEGACY +MIPS_BOOT_ENV_LEGACY +MIPS_BOOT_FDT +MMC +MMC_IO_VOLTAGE +MMC_VERBOSE +MULTI_DTB_FIT +MULTIPLEXER +MXC_OCOTP +NAND_DENALI +NET +NO_FB_CLEAR +NXP_FSPI +OF_LIBFDT +OF_LIVE +OFNODE_MULTI_TREE +OF_REAL +OF_TRANSLATE +OPTEE +OPTEE_IMAGE +PARTITIONS +PARTITION_TYPE_GUID +PARTITION_UUIDS +PCI +PCI_PNP +PG_WCOM_UBOOT_UPDATE_SUPPORTED +PHY +PHY_ATHEROS +PHY_CADENCE_SIERRA +PHY_CADENCE_TORRENT +PHY_FIXED +PINCTRL +PKCS7_MESSAGE_PARSER +PLATDATA +POWER_DOMAIN +POWER_I2C +QFW +QFW_PIO +RAM +RANDOM_UUID +RESET_MEDIATEK +RESTORE_EXCEPTION_VECTOR_BASE +RISCV_SMODE +ROCKCHIP_RK8XX_DISABLE_BOOT_ON_POWERON +RSA_PUBLIC_KEY_PARSER +RSA_VERIFY_WITH_PKEY +RTC_SANDBOX +SANDBOX +SATA +SEC_FIRMWARE_ARMV8_PSCI +SEMIHOSTING +SERIAL +SERIAL_PUTS +SERIAL_RX_BUFFER +SHA1 +SHA384 +SHA512 +SHA512_HW_ACCEL +SHA_HW_ACCEL +SHOW_BOOT_PROGRESS +SILENT_CONSOLE +SILENT_CONSOLE_UPDATE_ON_RELOC +SILENT_CONSOLE_UPDATE_ON_SET +SIMPLE_BUS_CORRECT_RANGE +SKIP_LOWLEVEL_INIT +SMC911X_32_BIT +SMP +SPI +SPI_BOOT +SPI_DIRMAP +SPI_FLASH_BAR +SPI_FLASH_MACRONIX +SPI_FLASH_MTD +SPI_FLASH_SFDP_SUPPORT +SPI_NOR_BOOT_SOFT_RESET_EXT_INVERT +STM32_ETZPC +SYS_CONSOLE_IS_IN_ENV +SYS_DCACHE_OFF +SYS_DEVICE_NULLDEV +SYS_ICACHE_OFF +SYSINFO +SYSRESET +SYS_THUMB_BUILD +SYS_WHITE_ON_BLACK +TARGET_DENEB +TARGET_EVB_RK3399 +TARGET_GIEDI +TARGET_KMCOGE5NE +TARGET_KMETER1 +TARGET_ST_STM32MP15x +TEST_KCONFIG +TIMER +_UNDEFINED +UNIT_TEST +USB_CDNS3_GADGET +USB_CDNS3_HOST +USB_ETHER +USB_GADGET +USB_GADGET_OS_DESCRIPTORS +USB_HOST +USB_STORAGE +USE_ARCH_MEMSET +UT_DM +UT_UNICODE +VID +VIRTIO +WDT +X509_CERTIFICATE_PARSER +X86_16BIT_INIT +XILINX_MICROBLAZE0_DELAY_SLOT_EXCEP +XILINX_MICROBLAZE0_USR_EXCEP +ZLIB +ZSTD +ZYNQMP_PSU_INIT_ENABLED