diff options
33 files changed, 3194 insertions, 1549 deletions
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c index 24ddb5d991..bbf8bba112 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/ppa.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/ppa.c @@ -107,9 +107,6 @@ int ppa_init(void) return -EIO; } - /* flush cache after read */ - flush_cache((ulong)fitp, cnt * 512); - ret = fdt_check_header(fitp); if (ret) { free(fitp); @@ -134,9 +131,6 @@ int ppa_init(void) } debug("Read PPA header to 0x%p\n", ppa_hdr_ddr); - /* flush cache after read */ - flush_cache((ulong)ppa_hdr_ddr, cnt * 512); - ppa_esbc_hdr = (uintptr_t)ppa_hdr_ddr; #endif @@ -164,9 +158,6 @@ int ppa_init(void) return -EIO; } - /* flush cache after read */ - flush_cache((ulong)ppa_fit_addr, cnt * 512); - #elif defined(CONFIG_SYS_LS_PPA_FW_IN_NAND) struct fdt_header fit; @@ -208,9 +199,6 @@ int ppa_init(void) } debug("Read PPA header to 0x%p\n", ppa_hdr_ddr); - /* flush cache after read */ - flush_cache((ulong)ppa_hdr_ddr, fw_length); - ppa_esbc_hdr = (uintptr_t)ppa_hdr_ddr; #endif @@ -232,9 +220,6 @@ int ppa_init(void) CONFIG_SYS_LS_PPA_FW_ADDR); return -EIO; } - - /* flush cache after read */ - flush_cache((ulong)ppa_fit_addr, fw_length); #else #error "No CONFIG_SYS_LS_PPA_FW_IN_xxx defined" #endif diff --git a/arch/arm/dts/dra7-evm-u-boot.dtsi b/arch/arm/dts/dra7-evm-u-boot.dtsi new file mode 100644 index 0000000000..62ef83047e --- /dev/null +++ b/arch/arm/dts/dra7-evm-u-boot.dtsi @@ -0,0 +1,15 @@ +/* + * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/ + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include "omap5-u-boot.dtsi" + +&pcf_gpio_21{ + u-boot,i2c-offset-len = <0>; +}; + +&pcf_hdmi{ + u-boot,i2c-offset-len = <0>; +}; diff --git a/arch/arm/include/asm/arch-omap5/mux_dra7xx.h b/arch/arm/include/asm/arch-omap5/mux_dra7xx.h index 5eed98ca27..55f49c7848 100644 --- a/arch/arm/include/asm/arch-omap5/mux_dra7xx.h +++ b/arch/arm/include/asm/arch-omap5/mux_dra7xx.h @@ -12,20 +12,6 @@ #include <asm/types.h> -#define FSC (1 << 19) -#define SSC (0 << 19) - -#define IEN (1 << 18) -#define IDIS (0 << 18) - -#define PTU (1 << 17) -#define PTD (0 << 17) -#define PEN (1 << 16) -#define PDIS (0 << 16) - -#define WKEN (1 << 24) -#define WKDIS (0 << 24) - #define PULL_ENA (0 << 16) #define PULL_DIS (1 << 16) #define PULL_UP (1 << 17) diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile index 7d67191de8..d4c593d25b 100644 --- a/arch/arm/mach-davinci/Makefile +++ b/arch/arm/mach-davinci/Makefile @@ -13,7 +13,6 @@ obj-$(CONFIG_SOC_DM355) += dm355.o obj-$(CONFIG_SOC_DM365) += dm365.o obj-$(CONFIG_SOC_DM644X) += dm644x.o obj-$(CONFIG_SOC_DM646X) += dm646x.o -obj-$(CONFIG_SOC_DA830) += da830_pinmux.o obj-$(CONFIG_SOC_DA850) += da850_pinmux.o obj-$(CONFIG_DRIVER_TI_EMAC) += lxt972.o dp83848.o et1011c.o ksz8873.o diff --git a/arch/arm/mach-davinci/da830_pinmux.c b/arch/arm/mach-davinci/da830_pinmux.c deleted file mode 100644 index 4182bb7960..0000000000 --- a/arch/arm/mach-davinci/da830_pinmux.c +++ /dev/null @@ -1,139 +0,0 @@ -/* - * Pinmux configurations for the DA830 SoCs - * - * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com/ - * - * SPDX-License-Identifier: GPL-2.0+ - */ - -#include <common.h> -#include <asm/arch/davinci_misc.h> -#include <asm/arch/hardware.h> -#include <asm/arch/pinmux_defs.h> - -/* SPI0 pin muxer settings */ -const struct pinmux_config spi0_pins_base[] = { - { pinmux(7), 1, 3 }, /* SPI0_SOMI */ - { pinmux(7), 1, 4 }, /* SPI0_SIMO */ - { pinmux(7), 1, 6 } /* SPI0_CLK */ -}; - -const struct pinmux_config spi0_pins_scs0[] = { - { pinmux(7), 1, 7 } /* SPI0_SCS[0] */ -}; - -const struct pinmux_config spi0_pins_ena[] = { - { pinmux(7), 1, 5 } /* SPI0_ENA */ -}; - -/* NAND pin muxer settings */ -const struct pinmux_config emifa_pins_cs0[] = { - { pinmux(18), 1, 2 } /* EMA_CS[0] */ -}; - -const struct pinmux_config emifa_pins_cs2[] = { - { pinmux(18), 1, 3 } /* EMA_CS[2] */ -}; - -const struct pinmux_config emifa_pins_cs3[] = { - { pinmux(18), 1, 4 } /* EMA_CS[3] */ -}; - -#ifdef CONFIG_USE_NAND -const struct pinmux_config emifa_pins[] = { - { pinmux(13), 1, 6 }, /* EMA_D[0] */ - { pinmux(13), 1, 7 }, /* EMA_D[1] */ - { pinmux(14), 1, 0 }, /* EMA_D[2] */ - { pinmux(14), 1, 1 }, /* EMA_D[3] */ - { pinmux(14), 1, 2 }, /* EMA_D[4] */ - { pinmux(14), 1, 3 }, /* EMA_D[5] */ - { pinmux(14), 1, 4 }, /* EMA_D[6] */ - { pinmux(14), 1, 5 }, /* EMA_D[7] */ - { pinmux(14), 1, 6 }, /* EMA_D[8] */ - { pinmux(14), 1, 7 }, /* EMA_D[9] */ - { pinmux(15), 1, 0 }, /* EMA_D[10] */ - { pinmux(15), 1, 1 }, /* EMA_D[11] */ - { pinmux(15), 1, 2 }, /* EMA_D[12] */ - { pinmux(15), 1, 3 }, /* EMA_D[13] */ - { pinmux(15), 1, 4 }, /* EMA_D[14] */ - { pinmux(15), 1, 5 }, /* EMA_D[15] */ - { pinmux(15), 1, 6 }, /* EMA_A[0] */ - { pinmux(15), 1, 7 }, /* EMA_A[1] */ - { pinmux(16), 1, 0 }, /* EMA_A[2] */ - { pinmux(16), 1, 1 }, /* EMA_A[3] */ - { pinmux(16), 1, 2 }, /* EMA_A[4] */ - { pinmux(16), 1, 3 }, /* EMA_A[5] */ - { pinmux(16), 1, 4 }, /* EMA_A[6] */ - { pinmux(16), 1, 5 }, /* EMA_A[7] */ - { pinmux(16), 1, 6 }, /* EMA_A[8] */ - { pinmux(16), 1, 7 }, /* EMA_A[9] */ - { pinmux(17), 1, 0 }, /* EMA_A[10] */ - { pinmux(17), 1, 1 }, /* EMA_A[11] */ - { pinmux(17), 1, 2 }, /* EMA_A[12] */ - { pinmux(17), 1, 3 }, /* EMA_BA[1] */ - { pinmux(17), 1, 4 }, /* EMA_BA[0] */ - { pinmux(17), 1, 5 }, /* EMA_CLK */ - { pinmux(17), 1, 6 }, /* EMA_SDCKE */ - { pinmux(17), 1, 7 }, /* EMA_CAS */ - { pinmux(18), 1, 0 }, /* EMA_CAS */ - { pinmux(18), 1, 1 }, /* EMA_WE */ - { pinmux(18), 1, 5 }, /* EMA_OE */ - { pinmux(18), 1, 6 }, /* EMA_WE_DQM[1] */ - { pinmux(18), 1, 7 }, /* EMA_WE_DQM[0] */ - { pinmux(10), 1, 0 } /* Tristate */ -}; -#endif - -/* EMAC PHY interface pins */ -const struct pinmux_config emac_pins_rmii[] = { - { pinmux(10), 2, 1 }, /* RMII_TXD[0] */ - { pinmux(10), 2, 2 }, /* RMII_TXD[1] */ - { pinmux(10), 2, 3 }, /* RMII_TXEN */ - { pinmux(10), 2, 4 }, /* RMII_CRS_DV */ - { pinmux(10), 2, 5 }, /* RMII_RXD[0] */ - { pinmux(10), 2, 6 }, /* RMII_RXD[1] */ - { pinmux(10), 2, 7 } /* RMII_RXER */ -}; - -const struct pinmux_config emac_pins_mdio[] = { - { pinmux(11), 2, 0 }, /* MDIO_CLK */ - { pinmux(11), 2, 1 } /* MDIO_D */ -}; - -const struct pinmux_config emac_pins_rmii_clk_source[] = { - { pinmux(9), 0, 5 } /* ref.clk from external source */ -}; - -/* UART2 pin muxer settings */ -const struct pinmux_config uart2_pins_txrx[] = { - { pinmux(8), 2, 7 }, /* UART2_RXD */ - { pinmux(9), 2, 0 } /* UART2_TXD */ -}; - -/* I2C0 pin muxer settings */ -const struct pinmux_config i2c0_pins[] = { - { pinmux(8), 2, 3 }, /* I2C0_SDA */ - { pinmux(8), 2, 4 } /* I2C0_SCL */ -}; - -/* USB0_DRVVBUS pin muxer settings */ -const struct pinmux_config usb_pins[] = { - { pinmux(9), 1, 1 } /* USB0_DRVVBUS */ -}; - -#ifdef CONFIG_MMC_DAVINCI -/* MMC0 pin muxer settings */ -const struct pinmux_config mmc0_pins_8bit[] = { - { pinmux(15), 2, 7 }, /* MMCSD0_CLK */ - { pinmux(16), 2, 0 }, /* MMCSD0_CMD */ - { pinmux(13), 2, 6 }, /* MMCSD0_DAT_0 */ - { pinmux(13), 2, 7 }, /* MMCSD0_DAT_1 */ - { pinmux(14), 2, 0 }, /* MMCSD0_DAT_2 */ - { pinmux(14), 2, 1 }, /* MMCSD0_DAT_3 */ - { pinmux(14), 2, 2 }, /* MMCSD0_DAT_4 */ - { pinmux(14), 2, 3 }, /* MMCSD0_DAT_5 */ - { pinmux(14), 2, 4 }, /* MMCSD0_DAT_6 */ - { pinmux(14), 2, 5 } /* MMCSD0_DAT_7 */ - /* DA830 supports 8-bit mode */ -}; -#endif diff --git a/arch/arm/mach-omap2/sec-common.c b/arch/arm/mach-omap2/sec-common.c index 2d54a3165e..52e1785b4a 100644 --- a/arch/arm/mach-omap2/sec-common.c +++ b/arch/arm/mach-omap2/sec-common.c @@ -305,12 +305,8 @@ int secure_tee_install(u32 addr) if ((hdr->magic != OPTEE_MAGIC) || (hdr->version != OPTEE_VERSION) || - (hdr->init_load_addr_hi != 0) || - (hdr->init_load_addr_lo < (sec_mem_start + sizeof(struct optee_header))) || - (tee_file_size > size) || - ((hdr->init_load_addr_lo + tee_file_size - 1) > - (sec_mem_start + size - 1))) { - printf("Error in TEE header. Check load address and sizes\n"); + (tee_file_size > size)) { + printf("Error in TEE header. Check firewall and TEE sizes\n"); unmap_sysmem(hdr); return CMD_RET_FAILURE; } diff --git a/board/compulab/cl-som-am57x/mux.c b/board/compulab/cl-som-am57x/mux.c index 0db0609727..21449ca029 100644 --- a/board/compulab/cl-som-am57x/mux.c +++ b/board/compulab/cl-som-am57x/mux.c @@ -12,97 +12,98 @@ /* Serial console */ static const struct pad_conf_entry cl_som_am57x_padconf_console[] = { - {UART3_RXD, (FSC | IEN | PDIS | PTU | M0)}, /* UART3_RXD */ - {UART3_TXD, (FSC | IEN | PDIS | PTU | M0)}, /* UART3_TXD */ + {UART3_RXD, (M0 | PIN_INPUT_PULLUP | SLEWCONTROL)}, /* UART3_RXD */ + {UART3_TXD, (M0 | PIN_INPUT_PULLUP | SLEWCONTROL)}, /* UART3_TXD */ }; /* PMIC I2C */ static const struct pad_conf_entry cl_som_am57x_padconf_pmic[] = { - {MCASP1_ACLKR, (IEN | PEN | M10)}, /* MCASP1_ACLKR.I2C4_SDA */ - {MCASP1_FSR, (IEN | PEN | M10)}, /* MCASP1_FSR.I2C4_SCL */ + {MCASP1_ACLKR, (M10 | PIN_INPUT)}, /* MCASP1_ACLKR.I2C4_SDA */ + {MCASP1_FSR, (M10 | PIN_INPUT)}, /* MCASP1_FSR.I2C4_SCL */ }; /* Green GPIO led */ static const struct pad_conf_entry cl_som_am57x_padconf_green_led[] = { - {GPMC_A15, (IDIS | PDIS | PTD | M14)}, /* GPMC_A15.GPIO2_5 */ + {GPMC_A15, (M14 | PIN_OUTPUT_PULLDOWN)}, /* GPMC_A15.GPIO2_5 */ }; /* MMC/SD Card */ static const struct pad_conf_entry cl_som_am57x_padconf_sd_card[] = { - {MMC1_CLK, (IEN | PDIS | PTU | M0) }, /* MMC1_CLK */ - {MMC1_CMD, (IEN | PDIS | PTU | M0) }, /* MMC1_CMD */ - {MMC1_DAT0, (IEN | PDIS | PTU | M0) }, /* MMC1_DAT0 */ - {MMC1_DAT1, (IEN | PDIS | PTU | M0) }, /* MMC1_DAT1 */ - {MMC1_DAT2, (IEN | PDIS | PTU | M0) }, /* MMC1_DAT2 */ - {MMC1_DAT3, (IEN | PDIS | PTU | M0) }, /* MMC1_DAT3 */ - {MMC1_SDCD, (IEN | PEN | M14)}, /* MMC1_SDCD */ - {MMC1_SDWP, (IEN | PEN | M14)}, /* MMC1_SDWP */ + {MMC1_CLK, (M0 | PIN_INPUT_PULLUP)}, /* MMC1_CLK */ + {MMC1_CMD, (M0 | PIN_INPUT_PULLUP)}, /* MMC1_CMD */ + {MMC1_DAT0, (M0 | PIN_INPUT_PULLUP)}, /* MMC1_DAT0 */ + {MMC1_DAT1, (M0 | PIN_INPUT_PULLUP)}, /* MMC1_DAT1 */ + {MMC1_DAT2, (M0 | PIN_INPUT_PULLUP)}, /* MMC1_DAT2 */ + {MMC1_DAT3, (M0 | PIN_INPUT_PULLUP)}, /* MMC1_DAT3 */ + {MMC1_SDCD, (M14 | PIN_INPUT) }, /* MMC1_SDCD */ + {MMC1_SDWP, (M14 | PIN_INPUT) }, /* MMC1_SDWP */ }; /* WiFi - must be in the safe mode on boot */ static const struct pad_conf_entry cl_som_am57x_padconf_wifi[] = { - {UART1_CTSN, (IEN | M15)}, /* UART1_CTSN */ - {UART1_RTSN, (IEN | M15)}, /* UART1_RTSN */ - {UART2_RXD, (IEN | M15)}, /* UART2_RXD */ - {UART2_TXD, (IEN | M15)}, /* UART2_TXD */ - {UART2_CTSN, (IEN | M15)}, /* UART2_CTSN */ - {UART2_RTSN, (IEN | M15)}, /* UART2_RTSN */ + {UART1_CTSN, (M15 | PIN_INPUT_PULLDOWN)}, /* UART1_CTSN */ + {UART1_RTSN, (M15 | PIN_INPUT_PULLDOWN)}, /* UART1_RTSN */ + {UART2_RXD, (M15 | PIN_INPUT_PULLDOWN)}, /* UART2_RXD */ + {UART2_TXD, (M15 | PIN_INPUT_PULLDOWN)}, /* UART2_TXD */ + {UART2_CTSN, (M15 | PIN_INPUT_PULLDOWN)}, /* UART2_CTSN */ + {UART2_RTSN, (M15 | PIN_INPUT_PULLDOWN)}, /* UART2_RTSN */ }; /* QSPI */ static const struct pad_conf_entry cl_som_am57x_padconf_qspi[] = { - {GPMC_A13, (IEN | PEN | M1)}, /* GPMC_A13.QSPI1_RTCLK */ - {GPMC_A18, (IEN | PEN | M1)}, /* GPMC_A18.QSPI1_SCLK */ - {GPMC_A16, (IEN | PEN | M1)}, /* GPMC_A16.QSPI1_D0 */ - {GPMC_A17, (IEN | PEN | M1)}, /* GPMC_A17.QSPI1_D1 */ - {GPMC_CS2, (IEN | PDIS | PTU | M1)}, /* GPMC_CS2.QSPI1_CS0 */ + {GPMC_A13, (M1 | PIN_INPUT) }, /* GPMC_A13.QSPI1_RTCLK */ + {GPMC_A18, (M1 | PIN_INPUT) }, /* GPMC_A18.QSPI1_SCLK */ + {GPMC_A16, (M1 | PIN_INPUT) }, /* GPMC_A16.QSPI1_D0 */ + {GPMC_A17, (M1 | PIN_INPUT) }, /* GPMC_A17.QSPI1_D1 */ + {GPMC_CS2, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_CS2.QSPI1_CS0 */ }; /* GPIO Expander I2C */ static const struct pad_conf_entry cl_som_am57x_padconf_i2c_gpio[] = { - {MCASP1_AXR0, (IEN | PEN | M10)}, /* MCASP1_AXR0.I2C5_SDA */ - {MCASP1_AXR1, (IEN | PEN | M10)}, /* MCASP1_AXR1.I2C5_SCL */ + {MCASP1_AXR0, (M10 | PIN_INPUT)}, /* MCASP1_AXR0.I2C5_SDA */ + {MCASP1_AXR1, (M10 | PIN_INPUT)}, /* MCASP1_AXR1.I2C5_SCL */ }; /* eMMC internal storage */ static const struct pad_conf_entry cl_som_am57x_padconf_emmc[] = { - {GPMC_A19, (IEN | PDIS | PTU | M1)}, /* GPMC_A19.MMC2_DAT4 */ - {GPMC_A20, (IEN | PDIS | PTU | M1)}, /* GPMC_A20.MMC2_DAT5 */ - {GPMC_A21, (IEN | PDIS | PTU | M1)}, /* GPMC_A21.MMC2_DAT6 */ - {GPMC_A22, (IEN | PDIS | PTU | M1)}, /* GPMC_A22.MMC2_DAT7 */ - {GPMC_A23, (IEN | PDIS | PTU | M1)}, /* GPMC_A23.MMC2_CLK */ - {GPMC_A24, (IEN | PDIS | PTU | M1)}, /* GPMC_A24.MMC2_DAT0 */ - {GPMC_A25, (IEN | PDIS | PTU | M1)}, /* GPMC_A25.MMC2_DAT1 */ - {GPMC_A26, (IEN | PDIS | PTU | M1)}, /* GPMC_A26.MMC2_DAT2 */ - {GPMC_A27, (IEN | PDIS | PTU | M1)}, /* GPMC_A27.MMC2_DAT3 */ - {GPMC_CS1, (IEN | PDIS | PTU | M1)}, /* GPMC_CS1.MMC2_CMD */ + {GPMC_A19, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A19.MMC2_DAT4 */ + {GPMC_A20, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A20.MMC2_DAT5 */ + {GPMC_A21, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A21.MMC2_DAT6 */ + {GPMC_A22, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A22.MMC2_DAT7 */ + {GPMC_A23, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A23.MMC2_CLK */ + {GPMC_A24, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A24.MMC2_DAT0 */ + {GPMC_A25, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A25.MMC2_DAT1 */ + {GPMC_A26, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A26.MMC2_DAT2 */ + {GPMC_A27, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_A27.MMC2_DAT3 */ + {GPMC_CS1, (M1 | PIN_INPUT_PULLUP)}, /* GPMC_CS1.MMC2_CMD */ }; /* usb1_drvvbus */ static const struct pad_conf_entry cl_som_am57x_padconf_usb[] = { - {USB1_DRVVBUS, (M0 | FSC) }, /* USB1_DRVVBUS.USB1_DRVVBUS */ + /* USB1_DRVVBUS.USB1_DRVVBUS */ + {USB1_DRVVBUS, (M0 | PIN_OUTPUT_PULLDOWN | SLEWCONTROL) }, }; /* Ethernet */ static const struct pad_conf_entry cl_som_am57x_padconf_ethernet[] = { /* MDIO bus */ - {VIN2A_D10, (PDIS | PTU | M3) }, /* VIN2A_D10.MDIO_MCLK */ - {VIN2A_D11, (IEN | PDIS | PTU | M3) }, /* VIN2A_D11.MDIO_D */ + {VIN2A_D10, (M3 | PIN_OUTPUT_PULLUP) }, /* VIN2A_D10.MDIO_MCLK */ + {VIN2A_D11, (M3 | PIN_INPUT_PULLUP) }, /* VIN2A_D11.MDIO_D */ /* EMAC Slave 1 at addr 0x1 - Default interface */ - {VIN2A_D12, (IDIS | PEN | M3) }, /* VIN2A_D12.RGMII1_TXC */ - {VIN2A_D13, (IDIS | PEN | M3) }, /* VIN2A_D13.RGMII1_TXCTL */ - {VIN2A_D14, (IDIS | PEN | M3) }, /* VIN2A_D14.RGMII1_TXD3 */ - {VIN2A_D15, (IDIS | PEN | M3) }, /* VIN2A_D15.RGMII1_TXD2 */ - {VIN2A_D16, (IDIS | PEN | M3) }, /* VIN2A_D16.RGMII1_TXD1 */ - {VIN2A_D17, (IDIS | PEN | M3) }, /* VIN2A_D17.RGMII1_TXD0 */ - {VIN2A_D18, (IEN | PDIS | PTD | M3) }, /* VIN2A_D18.RGMII1_RXC */ - {VIN2A_D19, (IEN | PDIS | PTD | M3) }, /* VIN2A_D19.RGMII1_RXCTL */ - {VIN2A_D20, (IEN | PDIS | PTD | M3) }, /* VIN2A_D20.RGMII1_RXD3 */ - {VIN2A_D21, (IEN | PDIS | PTD | M3) }, /* VIN2A_D21.RGMII1_RXD2 */ - {VIN2A_D22, (IEN | PDIS | PTD | M3) }, /* VIN2A_D22.RGMII1_RXD1 */ - {VIN2A_D23, (IEN | PDIS | PTD | M3) }, /* VIN2A_D23.RGMII1_RXD0 */ + {VIN2A_D12, (M3 | PIN_OUTPUT) }, /* VIN2A_D12.RGMII1_TXC */ + {VIN2A_D13, (M3 | PIN_OUTPUT) }, /* VIN2A_D13.RGMII1_TXCTL */ + {VIN2A_D14, (M3 | PIN_OUTPUT) }, /* VIN2A_D14.RGMII1_TXD3 */ + {VIN2A_D15, (M3 | PIN_OUTPUT) }, /* VIN2A_D15.RGMII1_TXD2 */ + {VIN2A_D16, (M3 | PIN_OUTPUT) }, /* VIN2A_D16.RGMII1_TXD1 */ + {VIN2A_D17, (M3 | PIN_OUTPUT) }, /* VIN2A_D17.RGMII1_TXD0 */ + {VIN2A_D18, (M3 | PIN_INPUT_PULLDOWN) }, /* VIN2A_D18.RGMII1_RXC */ + {VIN2A_D19, (M3 | PIN_INPUT_PULLDOWN) }, /* VIN2A_D19.RGMII1_RXCTL */ + {VIN2A_D20, (M3 | PIN_INPUT_PULLDOWN) }, /* VIN2A_D20.RGMII1_RXD3 */ + {VIN2A_D21, (M3 | PIN_INPUT_PULLDOWN) }, /* VIN2A_D21.RGMII1_RXD2 */ + {VIN2A_D22, (M3 | PIN_INPUT_PULLDOWN) }, /* VIN2A_D22.RGMII1_RXD1 */ + {VIN2A_D23, (M3 | PIN_INPUT_PULLDOWN) }, /* VIN2A_D23.RGMII1_RXD0 */ /* Eth PHY1 reset GPIOs*/ - {VIN2A_CLK0, (IDIS | PDIS | PTD | M14)}, /* VIN2A_CLK0.GPIO3_28 */ + {VIN2A_CLK0, (M14 | PIN_OUTPUT_PULLDOWN)}, /* VIN2A_CLK0.GPIO3_28 */ }; #define SET_MUX(mux_array) do_set_mux32((*ctrl)->control_padconf_core_base, \ diff --git a/board/sandbox/README.sandbox b/board/sandbox/README.sandbox index 2e2c819384..9bc13e142b 100644 --- a/board/sandbox/README.sandbox +++ b/board/sandbox/README.sandbox @@ -337,6 +337,11 @@ $> lodev=`sudo losetup -P -f --show ./disk.raw` $> sudo mkfs.vfat -n EFI -v ${lodev}p1 $> sudo mkfs.ext4 -L ROOT -v ${lodev}p2 +or utilize the device described in test/py/make_test_disk.py: + + #!/usr/bin/python + import make_test_disk + make_test_disk.makeDisk() Writing Sandbox Drivers ----------------------- diff --git a/board/toradex/common/tdx-cfg-block.c b/board/toradex/common/tdx-cfg-block.c index 328c4c0200..f850a3c98e 100644 --- a/board/toradex/common/tdx-cfg-block.c +++ b/board/toradex/common/tdx-cfg-block.c @@ -129,8 +129,6 @@ static int tdx_cfg_block_mmc_storage(u8 *config_block, int write) ret = -EIO; goto out; } - /* Flush cache after read */ - flush_cache((ulong)(unsigned char *)config_block, 512); } else { /* Just writing one 512 byte block */ if (blk_dwrite(mmc_get_blk_desc(mmc), blk_start, 1, diff --git a/cmd/blk_common.c b/cmd/blk_common.c index 86c75e78d8..0c0c23eb37 100644 --- a/cmd/blk_common.c +++ b/cmd/blk_common.c @@ -68,9 +68,8 @@ int blk_common_cmd(int argc, char * const argv[], enum if_type if_type, ulong cnt = simple_strtoul(argv[4], NULL, 16); ulong n; - printf("\n%s read: device %d block # %lld, count %ld ... ", - if_name, *cur_devnump, (unsigned long long)blk, - cnt); + printf("\n%s read: device %d block # "LBAFU", count %lu ... ", + if_name, *cur_devnump, blk, cnt); n = blk_read_devnum(if_type, *cur_devnump, blk, cnt, (ulong *)addr); @@ -84,9 +83,8 @@ int blk_common_cmd(int argc, char * const argv[], enum if_type if_type, ulong cnt = simple_strtoul(argv[4], NULL, 16); ulong n; - printf("\n%s write: device %d block # %lld, count %ld ... ", - if_name, *cur_devnump, (unsigned long long)blk, - cnt); + printf("\n%s write: device %d block # "LBAFU", count %lu ... ", + if_name, *cur_devnump, blk, cnt); n = blk_write_devnum(if_type, *cur_devnump, blk, cnt, (ulong *)addr); @@ -293,8 +293,6 @@ static int do_mmc_read(cmd_tbl_t *cmdtp, int flag, curr_device, blk, cnt); n = blk_dread(mmc_get_blk_desc(mmc), blk, cnt, addr); - /* flush cache after read */ - flush_cache((ulong)addr, cnt * 512); /* FIXME */ printf("%d blocks read: %s\n", n, (n == cnt) ? "OK" : "ERROR"); return (n == cnt) ? CMD_RET_SUCCESS : CMD_RET_FAILURE; @@ -334,6 +334,7 @@ int ubi_volume_read(char *volume, char *buf, size_t size) unsigned long long tmp; struct ubi_volume *vol; loff_t offp = 0; + size_t len_read; vol = ubi_find_volume(volume); if (vol == NULL) @@ -373,6 +374,7 @@ int ubi_volume_read(char *volume, char *buf, size_t size) tmp = offp; off = do_div(tmp, vol->usable_leb_size); lnum = tmp; + len_read = size; do { if (off + len >= vol->usable_leb_size) len = vol->usable_leb_size - off; @@ -398,6 +400,9 @@ int ubi_volume_read(char *volume, char *buf, size_t size) len = size > tbuf_size ? tbuf_size : size; } while (size); + if (!size) + env_set_hex("filesize", len_read); + free(tbuf); return err; } diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 54120c2cc6..72600afea8 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -34,6 +34,7 @@ CONFIG_CMD_MX_CYCLIC=y CONFIG_CMD_DEMO=y CONFIG_CMD_GPIO=y CONFIG_CMD_GPT=y +CONFIG_CMD_GPT_RENAME=y CONFIG_CMD_IDE=y CONFIG_CMD_I2C=y CONFIG_CMD_PCI=y diff --git a/disk/part.c b/disk/part.c index c67fdacc79..aa9183d696 100644 --- a/disk/part.c +++ b/disk/part.c @@ -331,6 +331,24 @@ int part_get_info(struct blk_desc *dev_desc, int part, return -1; } +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info) +{ + info->start = 0; + info->size = dev_desc->lba; + info->blksz = dev_desc->blksz; + info->bootable = 0; + strcpy((char *)info->type, BOOT_PART_TYPE); + strcpy((char *)info->name, "Whole Disk"); +#if CONFIG_IS_ENABLED(PARTITION_UUIDS) + info->uuid[0] = 0; +#endif +#ifdef CONFIG_PARTITION_TYPE_GUID + info->type_guid[0] = 0; +#endif + + return 0; +} + int blk_get_device_by_str(const char *ifname, const char *dev_hwpart_str, struct blk_desc **dev_desc) { @@ -523,18 +541,7 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str, (*dev_desc)->log2blksz = LOG2((*dev_desc)->blksz); - info->start = 0; - info->size = (*dev_desc)->lba; - info->blksz = (*dev_desc)->blksz; - info->bootable = 0; - strcpy((char *)info->type, BOOT_PART_TYPE); - strcpy((char *)info->name, "Whole Disk"); -#if CONFIG_IS_ENABLED(PARTITION_UUIDS) - info->uuid[0] = 0; -#endif -#ifdef CONFIG_PARTITION_TYPE_GUID - info->type_guid[0] = 0; -#endif + part_get_info_whole_disk(*dev_desc, info); ret = 0; goto cleanup; diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index aee2a50d62..537cf5f0bb 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -294,9 +294,6 @@ ulong blk_read_devnum(enum if_type if_type, int devnum, lbaint_t start, if (IS_ERR_VALUE(n)) return n; - /* flush cache after read */ - flush_cache((ulong)buffer, blkcnt * desc->blksz); - return n; } diff --git a/drivers/block/blk_legacy.c b/drivers/block/blk_legacy.c index 981872ecb3..16d3bfe7f2 100644 --- a/drivers/block/blk_legacy.c +++ b/drivers/block/blk_legacy.c @@ -232,9 +232,6 @@ ulong blk_read_devnum(enum if_type if_type, int devnum, lbaint_t start, if (IS_ERR_VALUE(n)) return n; - /* flush cache after read */ - flush_cache((ulong)buffer, blkcnt * desc->blksz); - return n; } diff --git a/drivers/i2c/muxes/pca954x.c b/drivers/i2c/muxes/pca954x.c index 383f72f552..01ca1ff48d 100644 --- a/drivers/i2c/muxes/pca954x.c +++ b/drivers/i2c/muxes/pca954x.c @@ -1,5 +1,6 @@ /* * Copyright (C) 2015 - 2016 Xilinx, Inc. + * Copyright (C) 2017 National Instruments Corp * Written by Michal Simek * * SPDX-License-Identifier: GPL-2.0+ @@ -9,7 +10,8 @@ #include <dm.h> #include <errno.h> #include <i2c.h> -#include <asm/gpio.h> + +#include <asm-generic/gpio.h> DECLARE_GLOBAL_DATA_PTR; @@ -30,6 +32,7 @@ struct chip_desc { struct pca954x_priv { u32 addr; /* I2C mux address */ u32 width; /* I2C mux width - number of busses */ + struct gpio_desc gpio_mux_reset; }; static const struct chip_desc chips[] = { @@ -105,10 +108,45 @@ static int pca954x_ofdata_to_platdata(struct udevice *dev) return 0; } +static int pca954x_probe(struct udevice *dev) +{ + if (IS_ENABLED(CONFIG_DM_GPIO)) { + struct pca954x_priv *priv = dev_get_priv(dev); + int err; + + err = gpio_request_by_name(dev, "reset-gpios", 0, + &priv->gpio_mux_reset, GPIOD_IS_OUT); + + /* it's optional so only bail if we get a real error */ + if (err && (err != -ENOENT)) + return err; + + /* dm will take care of polarity */ + if (dm_gpio_is_valid(&priv->gpio_mux_reset)) + dm_gpio_set_value(&priv->gpio_mux_reset, 0); + } + + return 0; +} + +static int pca954x_remove(struct udevice *dev) +{ + if (IS_ENABLED(CONFIG_DM_GPIO)) { + struct pca954x_priv *priv = dev_get_priv(dev); + + if (dm_gpio_is_valid(&priv->gpio_mux_reset)) + dm_gpio_free(dev, &priv->gpio_mux_reset); + } + + return 0; +} + U_BOOT_DRIVER(pca954x) = { .name = "pca954x", .id = UCLASS_I2C_MUX, .of_match = pca954x_ids, + .probe = pca954x_probe, + .remove = pca954x_remove, .ops = &pca954x_ops, .ofdata_to_platdata = pca954x_ofdata_to_platdata, .priv_auto_alloc_size = sizeof(struct pca954x_priv), diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c index 451dfded77..261f1b911b 100644 --- a/drivers/net/fm/fm.c +++ b/drivers/net/fm/fm.c @@ -405,8 +405,6 @@ int fm_init_common(int index, struct ccsr_fman *reg) mmc_init(mmc); (void)mmc->block_dev.block_read(&mmc->block_dev, blk, cnt, addr); - /* flush cache after read */ - flush_cache((ulong)addr, cnt * 512); } #elif defined(CONFIG_SYS_QE_FMAN_FW_IN_REMOTE) void *addr = (void *)CONFIG_SYS_FMAN_FW_ADDR; diff --git a/drivers/net/phy/cortina.c b/drivers/net/phy/cortina.c index e0e9ed9767..637d89a1e1 100644 --- a/drivers/net/phy/cortina.c +++ b/drivers/net/phy/cortina.c @@ -177,8 +177,6 @@ void cs4340_upload_firmware(struct phy_device *phydev) mmc_init(mmc); (void)mmc->block_dev.block_read(&mmc->block_dev, blk, cnt, addr); - /* flush cache after read */ - flush_cache((ulong)addr, cnt * 512); } #endif diff --git a/drivers/qe/qe.c b/drivers/qe/qe.c index 8151068432..5366a1eb71 100644 --- a/drivers/qe/qe.c +++ b/drivers/qe/qe.c @@ -221,8 +221,6 @@ void u_qe_init(void) mmc_init(mmc); (void)mmc->block_dev.block_read(&mmc->block_dev, blk, cnt, addr); - /* flush cache after read */ - flush_cache((ulong)addr, cnt * 512); } #endif if (!u_qe_upload_firmware(addr)) diff --git a/fs/fat/Makefile b/fs/fat/Makefile index b60e8486c4..3e2a6b01a8 100644 --- a/fs/fat/Makefile +++ b/fs/fat/Makefile @@ -5,7 +5,3 @@ obj-$(CONFIG_FS_FAT) := fat.o obj-$(CONFIG_FAT_WRITE):= fat_write.o - -ifndef CONFIG_SPL_BUILD -obj-$(CONFIG_FS_FAT) += file.o -endif diff --git a/fs/fat/fat.c b/fs/fat/fat.c index 465a6875ed..f0284398b4 100644 --- a/fs/fat/fat.c +++ b/fs/fat/fat.c @@ -14,6 +14,7 @@ #include <config.h> #include <exports.h> #include <fat.h> +#include <fs.h> #include <asm/byteorder.h> #include <part.h> #include <malloc.h> @@ -28,11 +29,13 @@ static const int vfat_enabled = 0; #endif /* - * Convert a string to lowercase. + * Convert a string to lowercase. Converts at most 'len' characters, + * 'len' may be larger than the length of 'str' if 'str' is NULL + * terminated. */ -static void downcase(char *str) +static void downcase(char *str, size_t len) { - while (*str != '\0') { + while (*str != '\0' && len--) { *str = tolower(*str); str++; } @@ -119,22 +122,6 @@ int fat_register_device(struct blk_desc *dev_desc, int part_no) } /* - * Get the first occurence of a directory delimiter ('/' or '\') in a string. - * Return index into string if found, -1 otherwise. - */ -static int dirdelim(char *str) -{ - char *start = str; - - while (*str != '\0') { - if (ISDIRDELIM(*str)) - return str - start; - str++; - } - return -1; -} - -/* * Extract zero terminated short name from a directory entry. */ static void get_name(dir_entry *dirent, char *s_name) @@ -146,10 +133,13 @@ static void get_name(dir_entry *dirent, char *s_name) ptr = s_name; while (*ptr && *ptr != ' ') ptr++; + if (dirent->lcase & CASE_LOWER_BASE) + downcase(s_name, (unsigned)(ptr - s_name)); if (dirent->ext[0] && dirent->ext[0] != ' ') { - *ptr = '.'; - ptr++; + *ptr++ = '.'; memcpy(ptr, dirent->ext, 3); + if (dirent->lcase & CASE_LOWER_EXT) + downcase(ptr, 3); ptr[3] = '\0'; while (*ptr && *ptr != ' ') ptr++; @@ -159,7 +149,6 @@ static void get_name(dir_entry *dirent, char *s_name) *s_name = '\0'; else if (*s_name == aRING) *s_name = DELETED_FLAG; - downcase(s_name); } static int flush_dirty_fat_buffer(fsdata *mydata); @@ -268,8 +257,7 @@ get_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, unsigned long size) int ret; if (clustnum > 0) { - startsect = mydata->data_begin + - clustnum * mydata->clust_size; + startsect = clust_to_sect(mydata, clustnum); } else { startsect = mydata->rootdir_sect; } @@ -468,95 +456,6 @@ static int slot2str(dir_slot *slotptr, char *l_name, int *idx) return 0; } -/* - * Extract the full long filename starting at 'retdent' (which is really - * a slot) into 'l_name'. If successful also copy the real directory entry - * into 'retdent' - * Return 0 on success, -1 otherwise. - */ -static int -get_vfatname(fsdata *mydata, int curclust, __u8 *cluster, - dir_entry *retdent, char *l_name) -{ - dir_entry *realdent; - dir_slot *slotptr = (dir_slot *)retdent; - __u8 *buflimit = cluster + mydata->sect_size * ((curclust == 0) ? - PREFETCH_BLOCKS : - mydata->clust_size); - __u8 counter = (slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff; - int idx = 0; - - if (counter > VFAT_MAXSEQ) { - debug("Error: VFAT name is too long\n"); - return -1; - } - - while ((__u8 *)slotptr < buflimit) { - if (counter == 0) - break; - if (((slotptr->id & ~LAST_LONG_ENTRY_MASK) & 0xff) != counter) - return -1; - slotptr++; - counter--; - } - - if ((__u8 *)slotptr >= buflimit) { - dir_slot *slotptr2; - - if (curclust == 0) - return -1; - curclust = get_fatent(mydata, curclust); - if (CHECK_CLUST(curclust, mydata->fatsize)) { - debug("curclust: 0x%x\n", curclust); - printf("Invalid FAT entry\n"); - return -1; - } - - if (get_cluster(mydata, curclust, get_contents_vfatname_block, - mydata->clust_size * mydata->sect_size) != 0) { - debug("Error: reading directory block\n"); - return -1; - } - - slotptr2 = (dir_slot *)get_contents_vfatname_block; - while (counter > 0) { - if (((slotptr2->id & ~LAST_LONG_ENTRY_MASK) - & 0xff) != counter) - return -1; - slotptr2++; - counter--; - } - - /* Save the real directory entry */ - realdent = (dir_entry *)slotptr2; - while ((__u8 *)slotptr2 > get_contents_vfatname_block) { - slotptr2--; - slot2str(slotptr2, l_name, &idx); - } - } else { - /* Save the real directory entry */ - realdent = (dir_entry *)slotptr; - } - - do { - slotptr--; - if (slot2str(slotptr, l_name, &idx)) - break; - } while (!(slotptr->id & LAST_LONG_ENTRY_MASK)); - - l_name[idx] = '\0'; - if (*l_name == DELETED_FLAG) - *l_name = '\0'; - else if (*l_name == aRING) - *l_name = DELETED_FLAG; - downcase(l_name); - - /* Return the real directory entry */ - memcpy(retdent, realdent, sizeof(dir_entry)); - - return 0; -} - /* Calculate short name checksum */ static __u8 mkcksum(const char name[8], const char ext[3]) { @@ -573,169 +472,13 @@ static __u8 mkcksum(const char name[8], const char ext[3]) } /* - * Get the directory entry associated with 'filename' from the directory - * starting at 'startsect' + * TODO these should go away once fat_write is reworked to use the + * directory iterator */ __u8 get_dentfromdir_block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN); - -static dir_entry *get_dentfromdir(fsdata *mydata, int startsect, - char *filename, dir_entry *retdent, - int dols) -{ - __u16 prevcksum = 0xffff; - __u32 curclust = START(retdent); - int files = 0, dirs = 0; - - debug("get_dentfromdir: %s\n", filename); - - while (1) { - dir_entry *dentptr; - - int i; - - if (get_cluster(mydata, curclust, get_dentfromdir_block, - mydata->clust_size * mydata->sect_size) != 0) { - debug("Error: reading directory block\n"); - return NULL; - } - - dentptr = (dir_entry *)get_dentfromdir_block; - - for (i = 0; i < DIRENTSPERCLUST; i++) { - char s_name[14], l_name[VFAT_MAXLEN_BYTES]; - - l_name[0] = '\0'; - if (dentptr->name[0] == DELETED_FLAG) { - dentptr++; - continue; - } - if ((dentptr->attr & ATTR_VOLUME)) { - if (vfat_enabled && - (dentptr->attr & ATTR_VFAT) == ATTR_VFAT && - (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) { - prevcksum = ((dir_slot *)dentptr)->alias_checksum; - get_vfatname(mydata, curclust, - get_dentfromdir_block, - dentptr, l_name); - if (dols) { - int isdir; - char dirc; - int doit = 0; - - isdir = (dentptr->attr & ATTR_DIR); - - if (isdir) { - dirs++; - dirc = '/'; - doit = 1; - } else { - dirc = ' '; - if (l_name[0] != 0) { - files++; - doit = 1; - } - } - if (doit) { - if (dirc == ' ') { - printf(" %8u %s%c\n", - FAT2CPU32(dentptr->size), - l_name, - dirc); - } else { - printf(" %s%c\n", - l_name, - dirc); - } - } - dentptr++; - continue; - } - debug("vfatname: |%s|\n", l_name); - } else { - /* Volume label or VFAT entry */ - dentptr++; - continue; - } - } - if (dentptr->name[0] == 0) { - if (dols) { - printf("\n%d file(s), %d dir(s)\n\n", - files, dirs); - } - debug("Dentname == NULL - %d\n", i); - return NULL; - } - if (vfat_enabled) { - __u8 csum = mkcksum(dentptr->name, dentptr->ext); - if (dols && csum == prevcksum) { - prevcksum = 0xffff; - dentptr++; - continue; - } - } - - get_name(dentptr, s_name); - if (dols) { - int isdir = (dentptr->attr & ATTR_DIR); - char dirc; - int doit = 0; - - if (isdir) { - dirs++; - dirc = '/'; - doit = 1; - } else { - dirc = ' '; - if (s_name[0] != 0) { - files++; - doit = 1; - } - } - - if (doit) { - if (dirc == ' ') { - printf(" %8u %s%c\n", - FAT2CPU32(dentptr->size), - s_name, dirc); - } else { - printf(" %s%c\n", - s_name, dirc); - } - } - - dentptr++; - continue; - } - - if (strcmp(filename, s_name) - && strcmp(filename, l_name)) { - debug("Mismatch: |%s|%s|\n", s_name, l_name); - dentptr++; - continue; - } - - memcpy(retdent, dentptr, sizeof(dir_entry)); - - debug("DentName: %s", s_name); - debug(", start: 0x%x", START(dentptr)); - debug(", size: 0x%x %s\n", - FAT2CPU32(dentptr->size), - (dentptr->attr & ATTR_DIR) ? "(DIR)" : ""); - - return retdent; - } - - curclust = get_fatent(mydata, curclust); - if (CHECK_CLUST(curclust, mydata->fatsize)) { - debug("curclust: 0x%x\n", curclust); - printf("Invalid FAT entry\n"); - return NULL; - } - } - - return NULL; -} +__u8 do_fat_read_at_block[MAX_CLUSTSIZE] + __aligned(ARCH_DMA_MINALIGN); /* * Read boot sector and volume info from a FAT filesystem @@ -808,39 +551,19 @@ exit: return ret; } -__u8 do_fat_read_at_block[MAX_CLUSTSIZE] - __aligned(ARCH_DMA_MINALIGN); - -int do_fat_read_at(const char *filename, loff_t pos, void *buffer, - loff_t maxsize, int dols, int dogetsize, loff_t *size) +static int get_fs_info(fsdata *mydata) { - char fnamecopy[2048]; boot_sector bs; volume_info volinfo; - fsdata datablock; - fsdata *mydata = &datablock; - dir_entry *dentptr = NULL; - __u16 prevcksum = 0xffff; - char *subname = ""; - __u32 cursect; - int idx, isdir = 0; - int files = 0, dirs = 0; - int ret = -1; - int firsttime; - __u32 root_cluster = 0; - __u32 read_blk; - int rootdir_size = 0; - int buffer_blk_cnt; - int do_read; - __u8 *dir_ptr; - - if (read_bootsectandvi(&bs, &volinfo, &mydata->fatsize)) { + int ret; + + ret = read_bootsectandvi(&bs, &volinfo, &mydata->fatsize); + if (ret) { debug("Error: reading boot sector\n"); - return -1; + return ret; } if (mydata->fatsize == 32) { - root_cluster = bs.root_cluster; mydata->fatlength = bs.fat32_length; } else { mydata->fatlength = bs.fat_length; @@ -848,8 +571,7 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer, mydata->fat_sect = bs.reserved; - cursect = mydata->rootdir_sect - = mydata->fat_sect + mydata->fatlength * bs.fats; + mydata->rootdir_sect = mydata->fat_sect + mydata->fatlength * bs.fats; mydata->sect_size = (bs.sector_size[1] << 8) + bs.sector_size[0]; mydata->clust_size = bs.cluster_size; @@ -862,14 +584,17 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer, if (mydata->fatsize == 32) { mydata->data_begin = mydata->rootdir_sect - (mydata->clust_size * 2); + mydata->root_cluster = bs.root_cluster; } else { - rootdir_size = ((bs.dir_entries[1] * (int)256 + - bs.dir_entries[0]) * - sizeof(dir_entry)) / - mydata->sect_size; + mydata->rootdir_size = ((bs.dir_entries[1] * (int)256 + + bs.dir_entries[0]) * + sizeof(dir_entry)) / + mydata->sect_size; mydata->data_begin = mydata->rootdir_sect + - rootdir_size - + mydata->rootdir_size - (mydata->clust_size * 2); + mydata->root_cluster = + sect_to_clust(mydata, mydata->rootdir_sect); } mydata->fatbufnum = -1; @@ -887,355 +612,361 @@ int do_fat_read_at(const char *filename, loff_t pos, void *buffer, mydata->fatsize, mydata->fat_sect, mydata->fatlength); debug("Rootdir begins at cluster: %d, sector: %d, offset: %x\n" "Data begins at: %d\n", - root_cluster, + mydata->root_cluster, mydata->rootdir_sect, mydata->rootdir_sect * mydata->sect_size, mydata->data_begin); debug("Sector size: %d, cluster size: %d\n", mydata->sect_size, mydata->clust_size); - /* "cwd" is always the root... */ - while (ISDIRDELIM(*filename)) - filename++; + return 0; +} - /* Make a copy of the filename and convert it to lowercase */ - strcpy(fnamecopy, filename); - downcase(fnamecopy); -root_reparse: - if (*fnamecopy == '\0') { - if (!dols) - goto exit; +/* + * Directory iterator, to simplify filesystem traversal + * + * Implements an iterator pattern to traverse directory tables, + * transparently handling directory tables split across multiple + * clusters, and the difference between FAT12/FAT16 root directory + * (contiguous) and subdirectories + FAT32 root (chained). + * + * Rough usage: + * + * for (fat_itr_root(&itr, fsdata); fat_itr_next(&itr); ) { + * // to traverse down to a subdirectory pointed to by + * // current iterator position: + * fat_itr_child(&itr, &itr); + * } + * + * For more complete example, see fat_itr_resolve() + */ - dols = LS_ROOT; - } else if ((idx = dirdelim(fnamecopy)) >= 0) { - isdir = 1; - fnamecopy[idx] = '\0'; - subname = fnamecopy + idx + 1; - - /* Handle multiple delimiters */ - while (ISDIRDELIM(*subname)) - subname++; - } else if (dols) { - isdir = 1; - } +typedef struct { + fsdata *fsdata; /* filesystem parameters */ + unsigned clust; /* current cluster */ + int last_cluster; /* set once we've read last cluster */ + int is_root; /* is iterator at root directory */ + int remaining; /* remaining dent's in current cluster */ - buffer_blk_cnt = 0; - firsttime = 1; - while (1) { - int i; + /* current iterator position values: */ + dir_entry *dent; /* current directory entry */ + char l_name[VFAT_MAXLEN_BYTES]; /* long (vfat) name */ + char s_name[14]; /* short 8.3 name */ + char *name; /* l_name if there is one, else s_name */ - if (mydata->fatsize == 32 || firsttime) { - dir_ptr = do_fat_read_at_block; - firsttime = 0; - } else { - /** - * FAT16 sector buffer modification: - * Each loop, the second buffered block is moved to - * the buffer begin, and two next sectors are read - * next to the previously moved one. So the sector - * buffer keeps always 3 sectors for fat16. - * And the current sector is the buffer second sector - * beside the "firsttime" read, when it is the first one. - * - * PREFETCH_BLOCKS is 2 for FAT16 == loop[0:1] - * n = computed root dir sector - * loop | cursect-1 | cursect | cursect+1 | - * 0 | sector n+0 | sector n+1 | none | - * 1 | none | sector n+0 | sector n+1 | - * 0 | sector n+1 | sector n+2 | sector n+3 | - * 1 | sector n+3 | ... - */ - dir_ptr = (do_fat_read_at_block + mydata->sect_size); - memcpy(do_fat_read_at_block, dir_ptr, mydata->sect_size); - } + /* storage for current cluster in memory: */ + u8 block[MAX_CLUSTSIZE] __aligned(ARCH_DMA_MINALIGN); +} fat_itr; - do_read = 1; +static int fat_itr_isdir(fat_itr *itr); - if (mydata->fatsize == 32 && buffer_blk_cnt) - do_read = 0; +/** + * fat_itr_root() - initialize an iterator to start at the root + * directory + * + * @itr: iterator to initialize + * @fsdata: filesystem data for the partition + * @return 0 on success, else -errno + */ +static int fat_itr_root(fat_itr *itr, fsdata *fsdata) +{ + if (get_fs_info(fsdata)) + return -ENXIO; - if (do_read) { - read_blk = (mydata->fatsize == 32) ? - mydata->clust_size : PREFETCH_BLOCKS; + itr->fsdata = fsdata; + itr->clust = fsdata->root_cluster; + itr->dent = NULL; + itr->remaining = 0; + itr->last_cluster = 0; + itr->is_root = 1; - debug("FAT read(sect=%d, cnt:%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n", - cursect, read_blk, mydata->clust_size, DIRENTSPERBLOCK); + return 0; +} - if (disk_read(cursect, read_blk, dir_ptr) < 0) { - debug("Error: reading rootdir block\n"); - goto exit; - } +/** + * fat_itr_child() - initialize an iterator to descend into a sub- + * directory + * + * Initializes 'itr' to iterate the contents of the directory at + * the current cursor position of 'parent'. It is an error to + * call this if the current cursor of 'parent' is pointing at a + * regular file. + * + * Note that 'itr' and 'parent' can be the same pointer if you do + * not need to preserve 'parent' after this call, which is useful + * for traversing directory structure to resolve a file/directory. + * + * @itr: iterator to initialize + * @parent: the iterator pointing at a directory entry in the + * parent directory of the directory to iterate + */ +static void fat_itr_child(fat_itr *itr, fat_itr *parent) +{ + fsdata *mydata = parent->fsdata; /* for silly macros */ + unsigned clustnum = START(parent->dent); - dentptr = (dir_entry *)dir_ptr; - } + assert(fat_itr_isdir(parent)); - for (i = 0; i < DIRENTSPERBLOCK; i++) { - char s_name[14], l_name[VFAT_MAXLEN_BYTES]; - __u8 csum; + itr->fsdata = parent->fsdata; + if (clustnum > 0) { + itr->clust = clustnum; + } else { + itr->clust = parent->fsdata->root_cluster; + } + itr->dent = NULL; + itr->remaining = 0; + itr->last_cluster = 0; + itr->is_root = 0; +} - l_name[0] = '\0'; - if (dentptr->name[0] == DELETED_FLAG) { - dentptr++; - continue; - } +static void *next_cluster(fat_itr *itr) +{ + fsdata *mydata = itr->fsdata; /* for silly macros */ + int ret; + u32 sect; + + /* have we reached the end? */ + if (itr->last_cluster) + return NULL; + + sect = clust_to_sect(itr->fsdata, itr->clust); + + debug("FAT read(sect=%d), clust_size=%d, DIRENTSPERBLOCK=%zd\n", + sect, itr->fsdata->clust_size, DIRENTSPERBLOCK); + + /* + * NOTE: do_fat_read_at() had complicated logic to deal w/ + * vfat names that span multiple clusters in the fat16 case, + * which get_dentfromdir() probably also needed (and was + * missing). And not entirely sure what fat32 didn't have + * the same issue.. We solve that by only caring about one + * dent at a time and iteratively constructing the vfat long + * name. + */ + ret = disk_read(sect, itr->fsdata->clust_size, + itr->block); + if (ret < 0) { + debug("Error: reading block\n"); + return NULL; + } - if (vfat_enabled) - csum = mkcksum(dentptr->name, dentptr->ext); - - if (dentptr->attr & ATTR_VOLUME) { - if (vfat_enabled && - (dentptr->attr & ATTR_VFAT) == ATTR_VFAT && - (dentptr->name[0] & LAST_LONG_ENTRY_MASK)) { - prevcksum = - ((dir_slot *)dentptr)->alias_checksum; - - get_vfatname(mydata, - root_cluster, - dir_ptr, - dentptr, l_name); - - if (dols == LS_ROOT) { - char dirc; - int doit = 0; - int isdir = - (dentptr->attr & ATTR_DIR); - - if (isdir) { - dirs++; - dirc = '/'; - doit = 1; - } else { - dirc = ' '; - if (l_name[0] != 0) { - files++; - doit = 1; - } - } - if (doit) { - if (dirc == ' ') { - printf(" %8u %s%c\n", - FAT2CPU32(dentptr->size), - l_name, - dirc); - } else { - printf(" %s%c\n", - l_name, - dirc); - } - } - dentptr++; - continue; - } - debug("Rootvfatname: |%s|\n", - l_name); - } else { - /* Volume label or VFAT entry */ - dentptr++; - continue; - } - } else if (dentptr->name[0] == 0) { - debug("RootDentname == NULL - %d\n", i); - if (dols == LS_ROOT) { - printf("\n%d file(s), %d dir(s)\n\n", - files, dirs); - ret = 0; - } - goto exit; - } - else if (vfat_enabled && - dols == LS_ROOT && csum == prevcksum) { - prevcksum = 0xffff; - dentptr++; - continue; - } + if (itr->is_root && itr->fsdata->fatsize != 32) { + itr->clust++; + sect = clust_to_sect(itr->fsdata, itr->clust); + if (sect - itr->fsdata->rootdir_sect >= + itr->fsdata->rootdir_size) { + debug("cursect: 0x%x\n", itr->clust); + itr->last_cluster = 1; + } + } else { + itr->clust = get_fatent(itr->fsdata, itr->clust); + if (CHECK_CLUST(itr->clust, itr->fsdata->fatsize)) { + debug("cursect: 0x%x\n", itr->clust); + itr->last_cluster = 1; + } + } - get_name(dentptr, s_name); - - if (dols == LS_ROOT) { - int isdir = (dentptr->attr & ATTR_DIR); - char dirc; - int doit = 0; - - if (isdir) { - dirc = '/'; - if (s_name[0] != 0) { - dirs++; - doit = 1; - } - } else { - dirc = ' '; - if (s_name[0] != 0) { - files++; - doit = 1; - } - } - if (doit) { - if (dirc == ' ') { - printf(" %8u %s%c\n", - FAT2CPU32(dentptr->size), - s_name, dirc); - } else { - printf(" %s%c\n", - s_name, dirc); - } - } - dentptr++; - continue; - } + return itr->block; +} - if (strcmp(fnamecopy, s_name) - && strcmp(fnamecopy, l_name)) { - debug("RootMismatch: |%s|%s|\n", s_name, - l_name); - dentptr++; - continue; - } +static dir_entry *next_dent(fat_itr *itr) +{ + if (itr->remaining == 0) { + struct dir_entry *dent = next_cluster(itr); + unsigned nbytes = itr->fsdata->sect_size * + itr->fsdata->clust_size; - if (isdir && !(dentptr->attr & ATTR_DIR)) - goto exit; + /* have we reached the last cluster? */ + if (!dent) + return NULL; - debug("RootName: %s", s_name); - debug(", start: 0x%x", START(dentptr)); - debug(", size: 0x%x %s\n", - FAT2CPU32(dentptr->size), - isdir ? "(DIR)" : ""); + itr->remaining = nbytes / sizeof(dir_entry) - 1; + itr->dent = dent; + } else { + itr->remaining--; + itr->dent++; + } - goto rootdir_done; /* We got a match */ - } - debug("END LOOP: buffer_blk_cnt=%d clust_size=%d\n", buffer_blk_cnt, - mydata->clust_size); + /* have we reached the last valid entry? */ + if (itr->dent->name[0] == 0) + return NULL; - /* - * On FAT32 we must fetch the FAT entries for the next - * root directory clusters when a cluster has been - * completely processed. - */ - ++buffer_blk_cnt; - int rootdir_end = 0; - if (mydata->fatsize == 32) { - if (buffer_blk_cnt == mydata->clust_size) { - int nxtsect = 0; - int nxt_clust = 0; + return itr->dent; +} + +static dir_entry *extract_vfat_name(fat_itr *itr) +{ + struct dir_entry *dent = itr->dent; + int seqn = itr->dent->name[0] & ~LAST_LONG_ENTRY_MASK; + u8 chksum, alias_checksum = ((dir_slot *)dent)->alias_checksum; + int n = 0; - nxt_clust = get_fatent(mydata, root_cluster); - rootdir_end = CHECK_CLUST(nxt_clust, 32); + while (seqn--) { + char buf[13]; + int idx = 0; - nxtsect = mydata->data_begin + - (nxt_clust * mydata->clust_size); + slot2str((dir_slot *)dent, buf, &idx); - root_cluster = nxt_clust; + /* shift accumulated long-name up and copy new part in: */ + memmove(itr->l_name + idx, itr->l_name, n); + memcpy(itr->l_name, buf, idx); + n += idx; - cursect = nxtsect; - buffer_blk_cnt = 0; - } - } else { - if (buffer_blk_cnt == PREFETCH_BLOCKS) - buffer_blk_cnt = 0; + dent = next_dent(itr); + if (!dent) + return NULL; + } - rootdir_end = (++cursect - mydata->rootdir_sect >= - rootdir_size); - } + itr->l_name[n] = '\0'; - /* If end of rootdir reached */ - if (rootdir_end) { - if (dols == LS_ROOT) { - printf("\n%d file(s), %d dir(s)\n\n", - files, dirs); - *size = 0; - } - goto exit; - } + chksum = mkcksum(dent->name, dent->ext); + + /* checksum mismatch could mean deleted file, etc.. skip it: */ + if (chksum != alias_checksum) { + debug("** chksum=%x, alias_checksum=%x, l_name=%s, s_name=%8s.%3s\n", + chksum, alias_checksum, itr->l_name, dent->name, dent->ext); + return NULL; } -rootdir_done: - firsttime = 1; + return dent; +} + +/** + * fat_itr_next() - step to the next entry in a directory + * + * Must be called once on a new iterator before the cursor is valid. + * + * @itr: the iterator to iterate + * @return boolean, 1 if success or 0 if no more entries in the + * current directory + */ +static int fat_itr_next(fat_itr *itr) +{ + dir_entry *dent; - while (isdir) { - int startsect = mydata->data_begin - + START(dentptr) * mydata->clust_size; - dir_entry dent; - char *nextname = NULL; + itr->name = NULL; - dent = *dentptr; - dentptr = &dent; + while (1) { + dent = next_dent(itr); + if (!dent) + return 0; - idx = dirdelim(subname); + if (dent->name[0] == DELETED_FLAG || + dent->name[0] == aRING) + continue; - if (idx >= 0) { - subname[idx] = '\0'; - nextname = subname + idx + 1; - /* Handle multiple delimiters */ - while (ISDIRDELIM(*nextname)) - nextname++; - if (dols && *nextname == '\0') - firsttime = 0; - } else { - if (dols && firsttime) { - firsttime = 0; + if (dent->attr & ATTR_VOLUME) { + if (vfat_enabled && + (dent->attr & ATTR_VFAT) == ATTR_VFAT && + (dent->name[0] & LAST_LONG_ENTRY_MASK)) { + dent = extract_vfat_name(itr); + if (!dent) + continue; + itr->name = itr->l_name; + break; } else { - isdir = 0; + /* Volume label or VFAT entry, skip */ + continue; } } - if (get_dentfromdir(mydata, startsect, subname, dentptr, - isdir ? 0 : dols) == NULL) { - if (dols && !isdir) - *size = 0; - goto exit; - } + break; + } - if (isdir && !(dentptr->attr & ATTR_DIR)) - goto exit; + get_name(dent, itr->s_name); + if (!itr->name) + itr->name = itr->s_name; - /* - * If we are looking for a directory, and found a directory - * type entry, and the entry is for the root directory (as - * denoted by a cluster number of 0), jump back to the start - * of the function, since at least on FAT12/16, the root dir - * lives in a hard-coded location and needs special handling - * to parse, rather than simply following the cluster linked - * list in the FAT, like other directories. - */ - if (isdir && (dentptr->attr & ATTR_DIR) && !START(dentptr)) { - /* - * Modify the filename to remove the prefix that gets - * back to the root directory, so the initial root dir - * parsing code can continue from where we are without - * confusion. - */ - strcpy(fnamecopy, nextname ?: ""); - /* - * Set up state the same way as the function does when - * first started. This is required for the root dir - * parsing code operates in its expected environment. - */ - subname = ""; - cursect = mydata->rootdir_sect; - isdir = 0; - goto root_reparse; - } + return 1; +} - if (idx >= 0) - subname = nextname; - } +/** + * fat_itr_isdir() - is current cursor position pointing to a directory + * + * @itr: the iterator + * @return true if cursor is at a directory + */ +static int fat_itr_isdir(fat_itr *itr) +{ + return !!(itr->dent->attr & ATTR_DIR); +} - if (dogetsize) { - *size = FAT2CPU32(dentptr->size); - ret = 0; - } else { - ret = get_contents(mydata, dentptr, pos, buffer, maxsize, size); - } - debug("Size: %u, got: %llu\n", FAT2CPU32(dentptr->size), *size); +/* + * Helpers: + */ -exit: - free(mydata->fatbuf); - return ret; -} +#define TYPE_FILE 0x1 +#define TYPE_DIR 0x2 +#define TYPE_ANY (TYPE_FILE | TYPE_DIR) -int do_fat_read(const char *filename, void *buffer, loff_t maxsize, int dols, - loff_t *actread) +/** + * fat_itr_resolve() - traverse directory structure to resolve the + * requested path. + * + * Traverse directory structure to the requested path. If the specified + * path is to a directory, this will descend into the directory and + * leave it iterator at the start of the directory. If the path is to a + * file, it will leave the iterator in the parent directory with current + * cursor at file's entry in the directory. + * + * @itr: iterator initialized to root + * @path: the requested path + * @type: bitmask of allowable file types + * @return 0 on success or -errno + */ +static int fat_itr_resolve(fat_itr *itr, const char *path, unsigned type) { - return do_fat_read_at(filename, 0, buffer, maxsize, dols, 0, actread); + const char *next; + + /* chomp any extra leading slashes: */ + while (path[0] && ISDIRDELIM(path[0])) + path++; + + /* are we at the end? */ + if (strlen(path) == 0) { + if (!(type & TYPE_DIR)) + return -ENOENT; + return 0; + } + + /* find length of next path entry: */ + next = path; + while (next[0] && !ISDIRDELIM(next[0])) + next++; + + while (fat_itr_next(itr)) { + int match = 0; + unsigned n = max(strlen(itr->name), (size_t)(next - path)); + + /* check both long and short name: */ + if (!strncasecmp(path, itr->name, n)) + match = 1; + else if (itr->name != itr->s_name && + !strncasecmp(path, itr->s_name, n)) + match = 1; + + if (!match) + continue; + + if (fat_itr_isdir(itr)) { + /* recurse into directory: */ + fat_itr_child(itr, itr); + return fat_itr_resolve(itr, next, type); + } else if (next[0]) { + /* + * If next is not empty then we have a case + * like: /path/to/realfile/nonsense + */ + debug("bad trailing path: %s\n", next); + return -ENOENT; + } else if (!(type & TYPE_FILE)) { + return -ENOTDIR; + } else { + return 0; + } + } + + return -ENOENT; } int file_fat_detectfs(void) @@ -1300,33 +1031,73 @@ int file_fat_detectfs(void) return 0; } -int file_fat_ls(const char *dir) -{ - loff_t size; - - return do_fat_read(dir, NULL, 0, LS_YES, &size); -} - int fat_exists(const char *filename) { + fsdata fsdata; + fat_itr itrblock, *itr = &itrblock; int ret; - loff_t size; - ret = do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1, &size); + ret = fat_itr_root(itr, &fsdata); + if (ret) + return 0; + + ret = fat_itr_resolve(itr, filename, TYPE_ANY); + free(fsdata.fatbuf); return ret == 0; } int fat_size(const char *filename, loff_t *size) { - return do_fat_read_at(filename, 0, NULL, 0, LS_NO, 1, size); + fsdata fsdata; + fat_itr itrblock, *itr = &itrblock; + int ret; + + ret = fat_itr_root(itr, &fsdata); + if (ret) + return ret; + + ret = fat_itr_resolve(itr, filename, TYPE_FILE); + if (ret) { + /* + * Directories don't have size, but fs_size() is not + * expected to fail if passed a directory path: + */ + free(fsdata.fatbuf); + fat_itr_root(itr, &fsdata); + if (!fat_itr_resolve(itr, filename, TYPE_DIR)) { + *size = 0; + ret = 0; + } + goto out; + } + + *size = FAT2CPU32(itr->dent->size); +out: + free(fsdata.fatbuf); + return ret; } int file_fat_read_at(const char *filename, loff_t pos, void *buffer, loff_t maxsize, loff_t *actread) { + fsdata fsdata; + fat_itr itrblock, *itr = &itrblock; + int ret; + + ret = fat_itr_root(itr, &fsdata); + if (ret) + return ret; + + ret = fat_itr_resolve(itr, filename, TYPE_FILE); + if (ret) + goto out; + printf("reading %s\n", filename); - return do_fat_read_at(filename, pos, buffer, maxsize, LS_NO, 0, - actread); + ret = get_contents(&fsdata, itr->dent, pos, buffer, maxsize, actread); + +out: + free(fsdata.fatbuf); + return ret; } int file_fat_read(const char *filename, void *buffer, int maxsize) @@ -1353,6 +1124,68 @@ int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len, return ret; } +typedef struct { + struct fs_dir_stream parent; + struct fs_dirent dirent; + fsdata fsdata; + fat_itr itr; +} fat_dir; + +int fat_opendir(const char *filename, struct fs_dir_stream **dirsp) +{ + fat_dir *dir = calloc(1, sizeof(*dir)); + int ret; + + if (!dir) + return -ENOMEM; + + ret = fat_itr_root(&dir->itr, &dir->fsdata); + if (ret) + goto fail; + + ret = fat_itr_resolve(&dir->itr, filename, TYPE_DIR); + if (ret) + goto fail; + + *dirsp = (struct fs_dir_stream *)dir; + return 0; + +fail: + free(dir->fsdata.fatbuf); + free(dir); + return ret; +} + +int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp) +{ + fat_dir *dir = (fat_dir *)dirs; + struct fs_dirent *dent = &dir->dirent; + + if (!fat_itr_next(&dir->itr)) + return -ENOENT; + + memset(dent, 0, sizeof(*dent)); + strcpy(dent->name, dir->itr.name); + + if (fat_itr_isdir(&dir->itr)) { + dent->type = FS_DT_DIR; + } else { + dent->type = FS_DT_REG; + dent->size = FAT2CPU32(dir->itr.dent->size); + } + + *dentp = dent; + + return 0; +} + +void fat_closedir(struct fs_dir_stream *dirs) +{ + fat_dir *dir = (fat_dir *)dirs; + free(dir->fsdata.fatbuf); + free(dir); +} + void fat_close(void) { } diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c index 4ca024c208..9d2e0ed74c 100644 --- a/fs/fat/fat_write.c +++ b/fs/fat/fat_write.c @@ -345,7 +345,7 @@ get_long_file_name(fsdata *mydata, int curclust, __u8 *cluster, *l_name = '\0'; else if (*l_name == aRING) *l_name = DELETED_FLAG; - downcase(l_name); + downcase(l_name, INT_MAX); /* Return the real directory entry */ *retdent = realdent; @@ -502,8 +502,7 @@ set_cluster(fsdata *mydata, __u32 clustnum, __u8 *buffer, int ret; if (clustnum > 0) - startsect = mydata->data_begin + - clustnum * mydata->clust_size; + startsect = clust_to_sect(mydata, clustnum); else startsect = mydata->rootdir_sect; @@ -751,8 +750,7 @@ static int check_overflow(fsdata *mydata, __u32 clustnum, loff_t size) __u32 startsect, sect_num, offset; if (clustnum > 0) { - startsect = mydata->data_begin + - clustnum * mydata->clust_size; + startsect = clust_to_sect(mydata, clustnum); } else { startsect = mydata->rootdir_sect; } @@ -791,7 +789,7 @@ static dir_entry *empty_dentptr; static dir_entry *find_directory_entry(fsdata *mydata, int startsect, char *filename, dir_entry *retdent, __u32 start) { - __u32 curclust = (startsect - mydata->data_begin) / mydata->clust_size; + __u32 curclust = sect_to_clust(mydata, startsect); debug("get_dentfromdir: %s\n", filename); @@ -981,7 +979,7 @@ static int do_fat_write(const char *filename, void *buffer, loff_t size, memcpy(l_filename, filename, name_len); l_filename[name_len] = 0; /* terminate the string */ - downcase(l_filename); + downcase(l_filename, INT_MAX); startsect = mydata->rootdir_sect; retdent = find_directory_entry(mydata, startsect, diff --git a/fs/fat/file.c b/fs/fat/file.c deleted file mode 100644 index 89706117b9..0000000000 --- a/fs/fat/file.c +++ /dev/null @@ -1,183 +0,0 @@ -/* - * file.c - * - * Mini "VFS" by Marcus Sundberg - * - * 2002-07-28 - rjones@nexus-tech.net - ported to ppcboot v1.1.6 - * 2003-03-10 - kharris@nexus-tech.net - ported to uboot - * - * SPDX-License-Identifier: GPL-2.0+ - */ - -#include <common.h> -#include <config.h> -#include <malloc.h> -#include <fat.h> -#include <linux/stat.h> -#include <linux/time.h> - -/* Supported filesystems */ -static const struct filesystem filesystems[] = { - { file_fat_detectfs, file_fat_ls, file_fat_read, "FAT" }, -}; -#define NUM_FILESYS (sizeof(filesystems)/sizeof(struct filesystem)) - -/* The filesystem which was last detected */ -static int current_filesystem = FSTYPE_NONE; - -/* The current working directory */ -#define CWD_LEN 511 -char file_cwd[CWD_LEN+1] = "/"; - -const char * -file_getfsname(int idx) -{ - if (idx < 0 || idx >= NUM_FILESYS) - return NULL; - - return filesystems[idx].name; -} - -static void -pathcpy(char *dest, const char *src) -{ - char *origdest = dest; - - do { - if (dest-file_cwd >= CWD_LEN) { - *dest = '\0'; - return; - } - *(dest) = *(src); - if (*src == '\0') { - if (dest-- != origdest && ISDIRDELIM(*dest)) { - *dest = '\0'; - } - return; - } - ++dest; - - if (ISDIRDELIM(*src)) - while (ISDIRDELIM(*src)) src++; - else - src++; - } while (1); -} - -int -file_cd(const char *path) -{ - if (ISDIRDELIM(*path)) { - while (ISDIRDELIM(*path)) path++; - strncpy(file_cwd+1, path, CWD_LEN-1); - } else { - const char *origpath = path; - char *tmpstr = file_cwd; - int back = 0; - - while (*tmpstr != '\0') tmpstr++; - do { - tmpstr--; - } while (ISDIRDELIM(*tmpstr)); - - while (*path == '.') { - path++; - while (*path == '.') { - path++; - back++; - } - if (*path != '\0' && !ISDIRDELIM(*path)) { - path = origpath; - back = 0; - break; - } - while (ISDIRDELIM(*path)) path++; - origpath = path; - } - - while (back--) { - /* Strip off path component */ - while (!ISDIRDELIM(*tmpstr)) { - tmpstr--; - } - if (tmpstr == file_cwd) { - /* Incremented again right after the loop. */ - tmpstr--; - break; - } - /* Skip delimiters */ - while (ISDIRDELIM(*tmpstr)) tmpstr--; - } - tmpstr++; - if (*path == '\0') { - if (tmpstr == file_cwd) { - *tmpstr = '/'; - tmpstr++; - } - *tmpstr = '\0'; - return 0; - } - *tmpstr = '/'; - pathcpy(tmpstr+1, path); - } - - return 0; -} - -int -file_detectfs(void) -{ - int i; - - current_filesystem = FSTYPE_NONE; - - for (i = 0; i < NUM_FILESYS; i++) { - if (filesystems[i].detect() == 0) { - strcpy(file_cwd, "/"); - current_filesystem = i; - break; - } - } - - return current_filesystem; -} - -int -file_ls(const char *dir) -{ - char fullpath[1024]; - const char *arg; - - if (current_filesystem == FSTYPE_NONE) { - printf("Can't list files without a filesystem!\n"); - return -1; - } - - if (ISDIRDELIM(*dir)) { - arg = dir; - } else { - sprintf(fullpath, "%s/%s", file_cwd, dir); - arg = fullpath; - } - return filesystems[current_filesystem].ls(arg); -} - -int file_read(const char *filename, void *buffer, int maxsize) -{ - char fullpath[1024]; - const char *arg; - - if (current_filesystem == FSTYPE_NONE) { - printf("Can't load file without a filesystem!\n"); - return -1; - } - - if (ISDIRDELIM(*filename)) { - arg = filename; - } else { - sprintf(fullpath, "%s/%s", file_cwd, filename); - arg = fullpath; - } - - return filesystems[current_filesystem].read(arg, buffer, maxsize); -} @@ -21,6 +21,7 @@ DECLARE_GLOBAL_DATA_PTR; static struct blk_desc *fs_dev_desc; +static int fs_dev_part; static disk_partition_t fs_partition; static int fs_type = FS_TYPE_ANY; @@ -36,6 +37,35 @@ static inline int fs_ls_unsupported(const char *dirname) return -1; } +/* generic implementation of ls in terms of opendir/readdir/closedir */ +__maybe_unused +static int fs_ls_generic(const char *dirname) +{ + struct fs_dir_stream *dirs; + struct fs_dirent *dent; + int nfiles = 0, ndirs = 0; + + dirs = fs_opendir(dirname); + if (!dirs) + return -errno; + + while ((dent = fs_readdir(dirs))) { + if (dent->type == FS_DT_DIR) { + printf(" %s/\n", dent->name); + ndirs++; + } else { + printf(" %8lld %s\n", dent->size, dent->name); + nfiles++; + } + } + + fs_closedir(dirs); + + printf("\n%d file(s), %d dir(s)\n\n", nfiles, ndirs); + + return 0; +} + static inline int fs_exists_unsupported(const char *filename) { return 0; @@ -69,6 +99,12 @@ static inline int fs_uuid_unsupported(char *uuid_str) return -1; } +static inline int fs_opendir_unsupported(const char *filename, + struct fs_dir_stream **dirs) +{ + return -EACCES; +} + struct fstype_info { int fstype; char *name; @@ -92,6 +128,20 @@ struct fstype_info { loff_t len, loff_t *actwrite); void (*close)(void); int (*uuid)(char *uuid_str); + /* + * Open a directory stream. On success return 0 and directory + * stream pointer via 'dirsp'. On error, return -errno. See + * fs_opendir(). + */ + int (*opendir)(const char *filename, struct fs_dir_stream **dirsp); + /* + * Read next entry from directory stream. On success return 0 + * and directory entry pointer via 'dentp'. On error return + * -errno. See fs_readdir(). + */ + int (*readdir)(struct fs_dir_stream *dirs, struct fs_dirent **dentp); + /* see fs_closedir() */ + void (*closedir)(struct fs_dir_stream *dirs); }; static struct fstype_info fstypes[] = { @@ -102,7 +152,7 @@ static struct fstype_info fstypes[] = { .null_dev_desc_ok = false, .probe = fat_set_blk_dev, .close = fat_close, - .ls = file_fat_ls, + .ls = fs_ls_generic, .exists = fat_exists, .size = fat_size, .read = fat_read_file, @@ -112,6 +162,9 @@ static struct fstype_info fstypes[] = { .write = fs_write_unsupported, #endif .uuid = fs_uuid_unsupported, + .opendir = fat_opendir, + .readdir = fat_readdir, + .closedir = fat_closedir, }, #endif #ifdef CONFIG_FS_EXT4 @@ -131,6 +184,7 @@ static struct fstype_info fstypes[] = { .write = fs_write_unsupported, #endif .uuid = ext4fs_uuid, + .opendir = fs_opendir_unsupported, }, #endif #ifdef CONFIG_SANDBOX @@ -146,6 +200,7 @@ static struct fstype_info fstypes[] = { .read = fs_read_sandbox, .write = fs_write_sandbox, .uuid = fs_uuid_unsupported, + .opendir = fs_opendir_unsupported, }, #endif #ifdef CONFIG_CMD_UBIFS @@ -161,6 +216,7 @@ static struct fstype_info fstypes[] = { .read = ubifs_read, .write = fs_write_unsupported, .uuid = fs_uuid_unsupported, + .opendir = fs_opendir_unsupported, }, #endif { @@ -175,6 +231,7 @@ static struct fstype_info fstypes[] = { .read = fs_read_unsupported, .write = fs_write_unsupported, .uuid = fs_uuid_unsupported, + .opendir = fs_opendir_unsupported, }, }; @@ -228,6 +285,31 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype) if (!info->probe(fs_dev_desc, &fs_partition)) { fs_type = info->fstype; + fs_dev_part = part; + return 0; + } + } + + return -1; +} + +/* set current blk device w/ blk_desc + partition # */ +int fs_set_blk_dev_with_part(struct blk_desc *desc, int part) +{ + struct fstype_info *info; + int ret, i; + + if (part >= 1) + ret = part_get_info(desc, part, &fs_partition); + else + ret = part_get_info_whole_disk(desc, &fs_partition); + if (ret) + return ret; + fs_dev_desc = desc; + + for (i = 0, info = fstypes; i < ARRAY_SIZE(fstypes); i++, info++) { + if (!info->probe(fs_dev_desc, &fs_partition)) { + fs_type = info->fstype; return 0; } } @@ -334,6 +416,59 @@ int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len, return ret; } +struct fs_dir_stream *fs_opendir(const char *filename) +{ + struct fstype_info *info = fs_get_info(fs_type); + struct fs_dir_stream *dirs = NULL; + int ret; + + ret = info->opendir(filename, &dirs); + fs_close(); + if (ret) { + errno = -ret; + return NULL; + } + + dirs->desc = fs_dev_desc; + dirs->part = fs_dev_part; + + return dirs; +} + +struct fs_dirent *fs_readdir(struct fs_dir_stream *dirs) +{ + struct fstype_info *info; + struct fs_dirent *dirent; + int ret; + + fs_set_blk_dev_with_part(dirs->desc, dirs->part); + info = fs_get_info(fs_type); + + ret = info->readdir(dirs, &dirent); + fs_close(); + if (ret) { + errno = -ret; + return NULL; + } + + return dirent; +} + +void fs_closedir(struct fs_dir_stream *dirs) +{ + struct fstype_info *info; + + if (!dirs) + return; + + fs_set_blk_dev_with_part(dirs->desc, dirs->part); + info = fs_get_info(fs_type); + + info->closedir(dirs); + fs_close(); +} + + int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], int fstype) { diff --git a/include/fat.h b/include/fat.h index 71879f01ca..bdeda95e6d 100644 --- a/include/fat.h +++ b/include/fat.h @@ -11,6 +11,7 @@ #define _FAT_H_ #include <asm/byteorder.h> +#include <fs.h> #define CONFIG_SUPPORT_VFAT /* Maximum Long File Name length supported here is 128 UTF-16 code units */ @@ -58,12 +59,6 @@ */ #define LAST_LONG_ENTRY_MASK 0x40 -/* Flags telling whether we should read a file or list a directory */ -#define LS_NO 0 -#define LS_YES 1 -#define LS_DIR 1 -#define LS_ROOT 2 - #define ISDIRDELIM(c) ((c) == '/' || (c) == '\\') #define FSTYPE_NONE (-1) @@ -133,10 +128,14 @@ typedef struct volume_info /* Boot sign comes last, 2 bytes */ } volume_info; +/* see dir_entry::lcase: */ +#define CASE_LOWER_BASE 8 /* base (name) is lower case */ +#define CASE_LOWER_EXT 16 /* extension is lower case */ + typedef struct dir_entry { char name[8],ext[3]; /* Name and extension */ __u8 attr; /* Attribute bits */ - __u8 lcase; /* Case for base and extension */ + __u8 lcase; /* Case for name and ext (CASE_LOWER_x) */ __u8 ctime_ms; /* Creation time, milliseconds */ __u16 ctime; /* Creation time */ __u16 cdate; /* Creation date */ @@ -174,35 +173,26 @@ typedef struct { __u16 clust_size; /* Size of clusters in sectors */ int data_begin; /* The sector of the first cluster, can be negative */ int fatbufnum; /* Used by get_fatent, init to -1 */ + int rootdir_size; /* Size of root dir for non-FAT32 */ + __u32 root_cluster; /* First cluster of root dir for FAT32 */ } fsdata; -typedef int (file_detectfs_func)(void); -typedef int (file_ls_func)(const char *dir); -typedef int (file_read_func)(const char *filename, void *buffer, - int maxsize); - -struct filesystem { - file_detectfs_func *detect; - file_ls_func *ls; - file_read_func *read; - const char name[12]; -}; - -/* FAT tables */ -file_detectfs_func file_fat_detectfs; -file_ls_func file_fat_ls; -file_read_func file_fat_read; - -/* Currently this doesn't check if the dir exists or is valid... */ -int file_cd(const char *path); +static inline u32 clust_to_sect(fsdata *fsdata, u32 clust) +{ + return fsdata->data_begin + clust * fsdata->clust_size; +} + +static inline u32 sect_to_clust(fsdata *fsdata, u32 sect) +{ + return (sect - fsdata->data_begin) / fsdata->clust_size; +} + int file_fat_detectfs(void); -int file_fat_ls(const char *dir); int fat_exists(const char *filename); int fat_size(const char *filename, loff_t *size); int file_fat_read_at(const char *filename, loff_t pos, void *buffer, loff_t maxsize, loff_t *actread); int file_fat_read(const char *filename, void *buffer, int maxsize); -const char *file_getfsname(int idx); int fat_set_blk_dev(struct blk_desc *rbdd, disk_partition_t *info); int fat_register_device(struct blk_desc *dev_desc, int part_no); @@ -210,5 +200,8 @@ int file_fat_write(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actwrite); int fat_read_file(const char *filename, void *buf, loff_t offset, loff_t len, loff_t *actread); +int fat_opendir(const char *filename, struct fs_dir_stream **dirsp); +int fat_readdir(struct fs_dir_stream *dirs, struct fs_dirent **dentp); +void fat_closedir(struct fs_dir_stream *dirs); void fat_close(void); #endif /* _FAT_H_ */ diff --git a/include/fs.h b/include/fs.h index 2f2aca8378..0869ad6e80 100644 --- a/include/fs.h +++ b/include/fs.h @@ -27,6 +27,17 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype); /* + * fs_set_blk_dev_with_part - Set current block device + partition + * + * Similar to fs_set_blk_dev(), but useful for cases where you already + * know the blk_desc and part number. + * + * Returns 0 on success. + * Returns non-zero if invalid partition or error accessing the disk. + */ +int fs_set_blk_dev_with_part(struct blk_desc *desc, int part); + +/* * Print the list of files on the partition previously set by fs_set_blk_dev(), * in directory "dirname". * @@ -79,6 +90,62 @@ int fs_write(const char *filename, ulong addr, loff_t offset, loff_t len, loff_t *actwrite); /* + * Directory entry types, matches the subset of DT_x in posix readdir() + * which apply to u-boot. + */ +#define FS_DT_DIR 4 /* directory */ +#define FS_DT_REG 8 /* regular file */ +#define FS_DT_LNK 10 /* symbolic link */ + +/* + * A directory entry, returned by fs_readdir(). Returns information + * about the file/directory at the current directory entry position. + */ +struct fs_dirent { + unsigned type; /* one of FS_DT_x (not a mask) */ + loff_t size; /* size in bytes */ + char name[256]; +}; + +/* Note: fs_dir_stream should be treated as opaque to the user of fs layer */ +struct fs_dir_stream { + /* private to fs. layer: */ + struct blk_desc *desc; + int part; +}; + +/* + * fs_opendir - Open a directory + * + * @filename: the path to directory to open + * @return a pointer to the directory stream or NULL on error and errno + * set appropriately + */ +struct fs_dir_stream *fs_opendir(const char *filename); + +/* + * fs_readdir - Read the next directory entry in the directory stream. + * + * Works in an analogous way to posix readdir(). The previously returned + * directory entry is no longer valid after calling fs_readdir() again. + * After fs_closedir() is called, the returned directory entry is no + * longer valid. + * + * @dirs: the directory stream + * @return the next directory entry (only valid until next fs_readdir() or + * fs_closedir() call, do not attempt to free()) or NULL if the end of + * the directory is reached. + */ +struct fs_dirent *fs_readdir(struct fs_dir_stream *dirs); + +/* + * fs_closedir - close a directory stream + * + * @dirs: the directory stream + */ +void fs_closedir(struct fs_dir_stream *dirs); + +/* * Common implementation for various filesystem commands, optionally limited * to a specific filesystem type via the fstype parameter. */ diff --git a/include/part.h b/include/part.h index 0d5c99836b..86117a7ce5 100644 --- a/include/part.h +++ b/include/part.h @@ -98,6 +98,12 @@ int host_get_dev_err(int dev, struct blk_desc **blk_devp); /* disk/part.c */ int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t *info); +/** + * part_get_info_whole_disk() - get partition info for the special case of + * a partition occupying the entire disk. + */ +int part_get_info_whole_disk(struct blk_desc *dev_desc, disk_partition_t *info); + void part_print(struct blk_desc *dev_desc); void part_init(struct blk_desc *dev_desc); void dev_print(struct blk_desc *dev_desc); @@ -203,6 +209,9 @@ static inline struct blk_desc *mg_disk_get_dev(int dev) { return NULL; } static inline int part_get_info(struct blk_desc *dev_desc, int part, disk_partition_t *info) { return -1; } +static inline int part_get_info_whole_disk(struct blk_desc *dev_desc, + disk_partition_t *info) +{ return -1; } static inline void part_print(struct blk_desc *dev_desc) {} static inline void part_init(struct blk_desc *dev_desc) {} static inline void dev_print(struct blk_desc *dev_desc) {} diff --git a/lib/strto.c b/lib/strto.c index e93a4f5491..7f6076909a 100644 --- a/lib/strto.c +++ b/lib/strto.c @@ -13,25 +13,30 @@ #include <errno.h> #include <linux/ctype.h> +/* from lib/kstrtox.c */ +static const char *_parse_integer_fixup_radix(const char *s, unsigned int *base) +{ + if (*base == 0) { + if (s[0] == '0') { + if (tolower(s[1]) == 'x' && isxdigit(s[2])) + *base = 16; + else + *base = 8; + } else + *base = 10; + } + if (*base == 16 && s[0] == '0' && tolower(s[1]) == 'x') + s += 2; + return s; +} + unsigned long simple_strtoul(const char *cp, char **endp, unsigned int base) { unsigned long result = 0; unsigned long value; - if (*cp == '0') { - cp++; - if ((*cp == 'x') && isxdigit(cp[1])) { - base = 16; - cp++; - } - - if (!base) - base = 8; - } - - if (!base) - base = 10; + cp = _parse_integer_fixup_radix(cp, &base); while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp-'0' : (islower(*cp) ? toupper(*cp) : *cp)-'A'+10) < base) { @@ -128,19 +133,7 @@ unsigned long long simple_strtoull(const char *cp, char **endp, { unsigned long long result = 0, value; - if (*cp == '0') { - cp++; - if ((*cp == 'x') && isxdigit(cp[1])) { - base = 16; - cp++; - } - - if (!base) - base = 8; - } - - if (!base) - base = 10; + cp = _parse_integer_fixup_radix(cp, &base); while (isxdigit(*cp) && (value = isdigit(*cp) ? *cp - '0' : (islower(*cp) ? toupper(*cp) : *cp) - 'A' + 10) < base) { diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3afc870f0f..4142f5c837 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -1,4 +1,4 @@ -#!/usr/bin/perl -w +#!/usr/bin/env perl # (c) 2001, Dave Jones. (the file handling bit) # (c) 2005, Joel Schopp <jschopp@austin.ibm.com> (the ugly bit) # (c) 2007,2008, Andy Whitcroft <apw@uk.ibm.com> (new conditions, test suite) @@ -6,12 +6,13 @@ # Licensed under the terms of the GNU GPL License version 2 use strict; +use warnings; use POSIX; use File::Basename; use Cwd 'abs_path'; +use Term::ANSIColor qw(:constants); my $P = $0; -$P =~ s@.*/@@g; my $D = dirname(abs_path($P)); my $V = '0.32'; @@ -25,12 +26,17 @@ my $chk_patch = 1; my $tst_only; my $emacs = 0; my $terse = 0; +my $showfile = 0; my $file = 0; +my $git = 0; +my %git_commits = (); my $check = 0; +my $check_orig = 0; my $summary = 1; my $mailback = 0; my $summary_file = 0; my $show_types = 0; +my $list_types = 0; my $fix = 0; my $fix_inplace = 0; my $root; @@ -45,9 +51,14 @@ my $configuration_file = ".checkpatch.conf"; my $max_line_length = 80; my $ignore_perl_version = 0; my $minimum_perl_version = 5.10.0; +my $min_conf_desc_length = 4; my $spelling_file = "$D/spelling.txt"; my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; +my $conststructsfile = "$D/const_structs.checkpatch"; +my $typedefsfile = ""; +my $color = "auto"; +my $allow_c99_comments = 1; sub help { my ($exitcode) = @_; @@ -63,12 +74,25 @@ Options: --patch treat FILE as patchfile (default) --emacs emacs compile window format --terse one line per report + --showfile emit diffed file position, not input file position + -g, --git treat FILE as a single commit or git revision range + single git commit with: + <rev> + <rev>^ + <rev>~n + multiple git commits with: + <rev1>..<rev2> + <rev1>...<rev2> + <rev>-<count> + git merges are ignored -f, --file treat FILE as regular source file --subjective, --strict enable more subjective tests + --list-types list the possible message types --types TYPE(,TYPE2...) show only these comma separated message types --ignore TYPE(,TYPE2...) ignore various comma separated message types + --show-types show the specific message type in the output --max-line-length=n set the maximum line length, if exceeded, warn - --show-types show the message "types" in the output + --min-conf-desc-length=n set the min description length, if shorter, warn --root=PATH PATH to the kernel tree root --no-summary suppress the per-file summary --mailback only produce a report in case of warnings/errors @@ -89,8 +113,11 @@ Options: --ignore-perl-version override checking of perl version. expect runtime errors. --codespell Use the codespell dictionary for spelling/typos - (default:/usr/local/share/codespell/dictionary.txt) + (default:/usr/share/codespell/dictionary.txt) --codespellfile Use this codespell dictionary + --typedefsfile Read additional types from this file + --color[=WHEN] Use colors 'always', 'never', or only when output + is a terminal ('auto'). Default is 'auto'. -h, --help, --version display this help and exit When FILE is - read standard input. @@ -99,6 +126,37 @@ EOM exit($exitcode); } +sub uniq { + my %seen; + return grep { !$seen{$_}++ } @_; +} + +sub list_types { + my ($exitcode) = @_; + + my $count = 0; + + local $/ = undef; + + open(my $script, '<', abs_path($P)) or + die "$P: Can't read '$P' $!\n"; + + my $text = <$script>; + close($script); + + my @types = (); + for ($text =~ /\b(?:(?:CHK|WARN|ERROR)\s*\(\s*"([^"]+)")/g) { + push (@types, $_); + } + @types = sort(uniq(@types)); + print("#\tMessage type\n\n"); + foreach my $type (@types) { + print(++$count . "\t" . $type . "\n"); + } + + exit($exitcode); +} + my $conf = which_conf($configuration_file); if (-f $conf) { my @conf_args; @@ -125,6 +183,14 @@ if (-f $conf) { unshift(@ARGV, @conf_args) if @conf_args; } +# Perl's Getopt::Long allows options to take optional arguments after a space. +# Prevent --color by itself from consuming other arguments +foreach (@ARGV) { + if ($_ eq "--color" || $_ eq "-color") { + $_ = "--color=$color"; + } +} + GetOptions( 'q|quiet+' => \$quiet, 'tree!' => \$tree, @@ -132,13 +198,17 @@ GetOptions( 'patch!' => \$chk_patch, 'emacs!' => \$emacs, 'terse!' => \$terse, + 'showfile!' => \$showfile, 'f|file!' => \$file, + 'g|git!' => \$git, 'subjective!' => \$check, 'strict!' => \$check, 'ignore=s' => \@ignore, 'types=s' => \@use, 'show-types!' => \$show_types, + 'list-types!' => \$list_types, 'max-line-length=i' => \$max_line_length, + 'min-conf-desc-length=i' => \$min_conf_desc_length, 'root=s' => \$root, 'summary!' => \$summary, 'mailback!' => \$mailback, @@ -148,15 +218,22 @@ GetOptions( 'ignore-perl-version!' => \$ignore_perl_version, 'debug=s' => \%debug, 'test-only=s' => \$tst_only, - 'codespell!' => \$codespell, - 'codespellfile=s' => \$codespellfile, + 'codespell!' => \$codespell, + 'codespellfile=s' => \$codespellfile, + 'typedefsfile=s' => \$typedefsfile, + 'color=s' => \$color, + 'no-color' => \$color, #keep old behaviors of -nocolor + 'nocolor' => \$color, #keep old behaviors of -nocolor 'h|help' => \$help, 'version' => \$help ) or help(1); help(0) if ($help); +list_types(0) if ($list_types); + $fix = 1 if ($fix_inplace); +$check_orig = $check; my $exit = 0; @@ -167,9 +244,21 @@ if ($^V && $^V lt $minimum_perl_version) { } } +#if no filenames are given, push '-' to read patch from stdin if ($#ARGV < 0) { - print "$P: no input files\n"; - exit(1); + push(@ARGV, '-'); +} + +if ($color =~ /^[01]$/) { + $color = !$color; +} elsif ($color =~ /^always$/i) { + $color = 1; +} elsif ($color =~ /^never$/i) { + $color = 0; +} elsif ($color =~ /^auto$/i) { + $color = (-t STDOUT); +} else { + die "Invalid color mode: $color\n"; } sub hash_save_array_words { @@ -192,12 +281,12 @@ sub hash_save_array_words { sub hash_show_words { my ($hashRef, $prefix) = @_; - if ($quiet == 0 && keys %$hashRef) { - print "NOTE: $prefix message types:"; + if (keys %$hashRef) { + print "\nNOTE: $prefix message types:"; foreach my $word (sort keys %$hashRef) { print " $word"; } - print "\n\n"; + print "\n"; } } @@ -257,7 +346,8 @@ our $Sparse = qr{ __init_refok| __kprobes| __ref| - __rcu + __rcu| + __private }x; our $InitAttributePrefix = qr{__(?:mem|cpu|dev|net_|)}; our $InitAttributeData = qr{$InitAttributePrefix(?:initdata\b)}; @@ -272,7 +362,7 @@ our $Attribute = qr{ __percpu| __nocast| __safe| - __bitwise__| + __bitwise| __packed__| __packed2__| __naked| @@ -281,6 +371,7 @@ our $Attribute = qr{ __noreturn| __used| __cold| + __pure| __noclone| __deprecated| __read_mostly| @@ -292,7 +383,7 @@ our $Attribute = qr{ __weak }x; our $Modifier; -our $Inline = qr{inline|__always_inline|noinline}; +our $Inline = qr{inline|__always_inline|noinline|__inline|__inline__}; our $Member = qr{->$Ident|\.$Ident|\[[^]]*\]}; our $Lval = qr{$Ident(?:$Member)*}; @@ -300,13 +391,15 @@ our $Int_type = qr{(?i)llu|ull|ll|lu|ul|l|u}; our $Binary = qr{(?i)0b[01]+$Int_type?}; our $Hex = qr{(?i)0x[0-9a-f]+$Int_type?}; our $Int = qr{[0-9]+$Int_type?}; +our $Octal = qr{0[0-7]+$Int_type?}; +our $String = qr{"[X\t]*"}; our $Float_hex = qr{(?i)0x[0-9a-f]+p-?[0-9]+[fl]?}; our $Float_dec = qr{(?i)(?:[0-9]+\.[0-9]*|[0-9]*\.[0-9]+)(?:e-?[0-9]+)?[fl]?}; our $Float_int = qr{(?i)[0-9]+e-?[0-9]+[fl]?}; our $Float = qr{$Float_hex|$Float_dec|$Float_int}; -our $Constant = qr{$Float|$Binary|$Hex|$Int}; +our $Constant = qr{$Float|$Binary|$Octal|$Hex|$Int}; our $Assignment = qr{\*\=|/=|%=|\+=|-=|<<=|>>=|&=|\^=|\|=|=}; -our $Compare = qr{<=|>=|==|!=|<|>}; +our $Compare = qr{<=|>=|==|!=|<|(?<!-)>}; our $Arithmetic = qr{\+|-|\*|\/|%}; our $Operators = qr{ <=|>=|==|!=| @@ -314,10 +407,16 @@ our $Operators = qr{ &&|\|\||,|\^|\+\+|--|&|\||$Arithmetic }x; +our $c90_Keywords = qr{do|for|while|if|else|return|goto|continue|switch|default|case|break}x; + +our $BasicType; our $NonptrType; +our $NonptrTypeMisordered; our $NonptrTypeWithAttr; our $Type; +our $TypeMisordered; our $Declare; +our $DeclareMisordered; our $NON_ASCII_UTF8 = qr{ [\xC2-\xDF][\x80-\xBF] # non-overlong 2-byte @@ -334,19 +433,28 @@ our $UTF8 = qr{ | $NON_ASCII_UTF8 }x; -our $typeTypedefs = qr{(?x: +our $typeC99Typedefs = qr{(?:__)?(?:[us]_?)?int_?(?:8|16|32|64)_t}; +our $typeOtherOSTypedefs = qr{(?x: + u_(?:char|short|int|long) | # bsd + u(?:nchar|short|int|long) # sysv +)}; +our $typeKernelTypedefs = qr{(?x: (?:__)?(?:u|s|be|le)(?:8|16|32|64)| atomic_t )}; +our $typeTypedefs = qr{(?x: + $typeC99Typedefs\b| + $typeOtherOSTypedefs\b| + $typeKernelTypedefs\b +)}; + +our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b}; our $logFunctions = qr{(?x: - printk(?:_ratelimited|_once|)| + printk(?:_ratelimited|_once|_deferred_once|_deferred|)| (?:[a-z0-9]+_){1,2}(?:printk|emerg|alert|crit|err|warning|warn|notice|info|debug|dbg|vdbg|devel|cont|WARN)(?:_ratelimited|_once|)| WARN(?:_RATELIMIT|_ONCE|)| panic| - debug| - printf| - puts| MODULE_[A-Z_]+| seq_vprintf|seq_printf|seq_puts )}; @@ -362,16 +470,36 @@ our $signature_tags = qr{(?xi: Cc: )}; +our @typeListMisordered = ( + qr{char\s+(?:un)?signed}, + qr{int\s+(?:(?:un)?signed\s+)?short\s}, + qr{int\s+short(?:\s+(?:un)?signed)}, + qr{short\s+int(?:\s+(?:un)?signed)}, + qr{(?:un)?signed\s+int\s+short}, + qr{short\s+(?:un)?signed}, + qr{long\s+int\s+(?:un)?signed}, + qr{int\s+long\s+(?:un)?signed}, + qr{long\s+(?:un)?signed\s+int}, + qr{int\s+(?:un)?signed\s+long}, + qr{int\s+(?:un)?signed}, + qr{int\s+long\s+long\s+(?:un)?signed}, + qr{long\s+long\s+int\s+(?:un)?signed}, + qr{long\s+long\s+(?:un)?signed\s+int}, + qr{long\s+long\s+(?:un)?signed}, + qr{long\s+(?:un)?signed}, +); + our @typeList = ( qr{void}, - qr{(?:unsigned\s+)?char}, - qr{(?:unsigned\s+)?short}, - qr{(?:unsigned\s+)?int}, - qr{(?:unsigned\s+)?long}, - qr{(?:unsigned\s+)?long\s+int}, - qr{(?:unsigned\s+)?long\s+long}, - qr{(?:unsigned\s+)?long\s+long\s+int}, - qr{unsigned}, + qr{(?:(?:un)?signed\s+)?char}, + qr{(?:(?:un)?signed\s+)?short\s+int}, + qr{(?:(?:un)?signed\s+)?short}, + qr{(?:(?:un)?signed\s+)?int}, + qr{(?:(?:un)?signed\s+)?long\s+int}, + qr{(?:(?:un)?signed\s+)?long\s+long\s+int}, + qr{(?:(?:un)?signed\s+)?long\s+long}, + qr{(?:(?:un)?signed\s+)?long}, + qr{(?:un)?signed}, qr{float}, qr{double}, qr{bool}, @@ -381,7 +509,31 @@ our @typeList = ( qr{${Ident}_t}, qr{${Ident}_handler}, qr{${Ident}_handler_fn}, + @typeListMisordered, ); + +our $C90_int_types = qr{(?x: + long\s+long\s+int\s+(?:un)?signed| + long\s+long\s+(?:un)?signed\s+int| + long\s+long\s+(?:un)?signed| + (?:(?:un)?signed\s+)?long\s+long\s+int| + (?:(?:un)?signed\s+)?long\s+long| + int\s+long\s+long\s+(?:un)?signed| + int\s+(?:(?:un)?signed\s+)?long\s+long| + + long\s+int\s+(?:un)?signed| + long\s+(?:un)?signed\s+int| + long\s+(?:un)?signed| + (?:(?:un)?signed\s+)?long\s+int| + (?:(?:un)?signed\s+)?long| + int\s+long\s+(?:un)?signed| + int\s+(?:(?:un)?signed\s+)?long| + + int\s+(?:un)?signed| + (?:(?:un)?signed\s+)?int +)}; + +our @typeListFile = (); our @typeListWithAttr = ( @typeList, qr{struct\s+$InitAttribute\s+$Ident}, @@ -391,10 +543,67 @@ our @typeListWithAttr = ( our @modifierList = ( qr{fastcall}, ); +our @modifierListFile = (); + +our @mode_permission_funcs = ( + ["module_param", 3], + ["module_param_(?:array|named|string)", 4], + ["module_param_array_named", 5], + ["debugfs_create_(?:file|u8|u16|u32|u64|x8|x16|x32|x64|size_t|atomic_t|bool|blob|regset32|u32_array)", 2], + ["proc_create(?:_data|)", 2], + ["(?:CLASS|DEVICE|SENSOR|SENSOR_DEVICE|IIO_DEVICE)_ATTR", 2], + ["IIO_DEV_ATTR_[A-Z_]+", 1], + ["SENSOR_(?:DEVICE_|)ATTR_2", 2], + ["SENSOR_TEMPLATE(?:_2|)", 3], + ["__ATTR", 2], +); + +#Create a search pattern for all these functions to speed up a loop below +our $mode_perms_search = ""; +foreach my $entry (@mode_permission_funcs) { + $mode_perms_search .= '|' if ($mode_perms_search ne ""); + $mode_perms_search .= $entry->[0]; +} + +our $mode_perms_world_writable = qr{ + S_IWUGO | + S_IWOTH | + S_IRWXUGO | + S_IALLUGO | + 0[0-7][0-7][2367] +}x; + +our %mode_permission_string_types = ( + "S_IRWXU" => 0700, + "S_IRUSR" => 0400, + "S_IWUSR" => 0200, + "S_IXUSR" => 0100, + "S_IRWXG" => 0070, + "S_IRGRP" => 0040, + "S_IWGRP" => 0020, + "S_IXGRP" => 0010, + "S_IRWXO" => 0007, + "S_IROTH" => 0004, + "S_IWOTH" => 0002, + "S_IXOTH" => 0001, + "S_IRWXUGO" => 0777, + "S_IRUGO" => 0444, + "S_IWUGO" => 0222, + "S_IXUGO" => 0111, +); + +#Create a search pattern for all these strings to speed up a loop below +our $mode_perms_string_search = ""; +foreach my $entry (keys %mode_permission_string_types) { + $mode_perms_string_search .= '|' if ($mode_perms_string_search ne ""); + $mode_perms_string_search .= $entry; +} our $allowed_asm_includes = qr{(?x: irq| - memory + memory| + time| + reboot )}; # memory.h: ARM has a custom one @@ -447,12 +656,54 @@ if ($codespell) { $misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix; +sub read_words { + my ($wordsRef, $file) = @_; + + if (open(my $words, '<', $file)) { + while (<$words>) { + my $line = $_; + + $line =~ s/\s*\n?$//g; + $line =~ s/^\s*//g; + + next if ($line =~ m/^\s*#/); + next if ($line =~ m/^\s*$/); + if ($line =~ /\s/) { + print("$file: '$line' invalid - ignored\n"); + next; + } + + $$wordsRef .= '|' if ($$wordsRef ne ""); + $$wordsRef .= $line; + } + close($file); + return 1; + } + + return 0; +} + +my $const_structs = ""; +read_words(\$const_structs, $conststructsfile) + or warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; + +my $typeOtherTypedefs = ""; +if (length($typedefsfile)) { + read_words(\$typeOtherTypedefs, $typedefsfile) + or warn "No additional types will be considered - file '$typedefsfile': $!\n"; +} +$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne ""); sub build_types { - my $mods = "(?x: \n" . join("|\n ", @modifierList) . "\n)"; - my $all = "(?x: \n" . join("|\n ", @typeList) . "\n)"; + my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; + my $all = "(?x: \n" . join("|\n ", (@typeList, @typeListFile)) . "\n)"; + my $Misordered = "(?x: \n" . join("|\n ", @typeListMisordered) . "\n)"; my $allWithAttr = "(?x: \n" . join("|\n ", @typeListWithAttr) . "\n)"; $Modifier = qr{(?:$Attribute|$Sparse|$mods)}; + $BasicType = qr{ + (?:$typeTypedefs\b)| + (?:${all}\b) + }x; $NonptrType = qr{ (?:$Modifier\s+|const\s+)* (?: @@ -462,6 +713,13 @@ sub build_types { ) (?:\s+$Modifier|\s+const)* }x; + $NonptrTypeMisordered = qr{ + (?:$Modifier\s+|const\s+)* + (?: + (?:${Misordered}\b) + ) + (?:\s+$Modifier|\s+const)* + }x; $NonptrTypeWithAttr = qr{ (?:$Modifier\s+|const\s+)* (?: @@ -473,10 +731,16 @@ sub build_types { }x; $Type = qr{ $NonptrType - (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*|\[\])+|(?:\s*\[\s*\])+)? + (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)? + (?:\s+$Inline|\s+$Modifier)* + }x; + $TypeMisordered = qr{ + $NonptrTypeMisordered + (?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)? (?:\s+$Inline|\s+$Modifier)* }x; - $Declare = qr{(?:$Storage\s+)?$Type}; + $Declare = qr{(?:$Storage\s+(?:$Inline\s+)?)?$Type}; + $DeclareMisordered = qr{(?:$Storage\s+(?:$Inline\s+)?)?$TypeMisordered}; } build_types(); @@ -487,15 +751,26 @@ our $Typecast = qr{\s*(\(\s*$NonptrType\s*\)){0,1}\s*}; # Any use must be runtime checked with $^V our $balanced_parens = qr/(\((?:[^\(\)]++|(?-1))*\))/; -our $LvalOrFunc = qr{($Lval)\s*($balanced_parens{0,1})\s*}; -our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)}; +our $LvalOrFunc = qr{((?:[\&\*]\s*)?$Lval)\s*($balanced_parens{0,1})\s*}; +our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant|$String)}; + +our $declaration_macros = qr{(?x: + (?:$Storage\s+)?(?:[A-Z_][A-Z0-9]*_){0,2}(?:DEFINE|DECLARE)(?:_[A-Z0-9]+){1,6}\s*\(| + (?:$Storage\s+)?[HLP]?LIST_HEAD\s*\(| + (?:$Storage\s+)?${Type}\s+uninitialized_var\s*\( +)}; sub deparenthesize { my ($string) = @_; return "" if (!defined($string)); - $string =~ s@^\s*\(\s*@@g; - $string =~ s@\s*\)\s*$@@g; + + while ($string =~ /^\s*\(.*\)\s*$/) { + $string =~ s@^\s*\(\s*@@; + $string =~ s@\s*\)\s*$@@; + } + $string =~ s@\s+@ @g; + return $string; } @@ -525,6 +800,16 @@ sub seed_camelcase_file { } } +sub is_maintained_obsolete { + my ($filename) = @_; + + return 0 if (!$tree || !(-e "$root/scripts/get_maintainer.pl")); + + my $status = `perl $root/scripts/get_maintainer.pl --status --nom --nol --nogit --nogit-fallback -f $filename 2>&1`; + + return $status =~ /obsolete/i; +} + my $camelcase_seeded = 0; sub seed_camelcase_includes { return if ($camelcase_seeded); @@ -583,17 +868,82 @@ sub seed_camelcase_includes { } } +sub git_commit_info { + my ($commit, $id, $desc) = @_; + + return ($id, $desc) if ((which("git") eq "") || !(-e ".git")); + + my $output = `git log --no-color --format='%H %s' -1 $commit 2>&1`; + $output =~ s/^\s*//gm; + my @lines = split("\n", $output); + + return ($id, $desc) if ($#lines < 0); + + if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous\./) { +# Maybe one day convert this block of bash into something that returns +# all matching commit ids, but it's very slow... +# +# echo "checking commits $1..." +# git rev-list --remotes | grep -i "^$1" | +# while read line ; do +# git log --format='%H %s' -1 $line | +# echo "commit $(cut -c 1-12,41-)" +# done + } elsif ($lines[0] =~ /^fatal: ambiguous argument '$commit': unknown revision or path not in the working tree\./) { + $id = undef; + } else { + $id = substr($lines[0], 0, 12); + $desc = substr($lines[0], 41); + } + + return ($id, $desc); +} + $chk_signoff = 0 if ($file); my @rawlines = (); my @lines = (); my @fixed = (); -my $vname; +my @fixed_inserted = (); +my @fixed_deleted = (); my $fixlinenr = -1; +# If input is git commits, extract all commits from the commit expressions. +# For example, HEAD-3 means we need check 'HEAD, HEAD~1, HEAD~2'. +die "$P: No git repository found\n" if ($git && !-e ".git"); + +if ($git) { + my @commits = (); + foreach my $commit_expr (@ARGV) { + my $git_range; + if ($commit_expr =~ m/^(.*)-(\d+)$/) { + $git_range = "-$2 $1"; + } elsif ($commit_expr =~ m/\.\./) { + $git_range = "$commit_expr"; + } else { + $git_range = "-1 $commit_expr"; + } + my $lines = `git log --no-color --no-merges --pretty=format:'%H %s' $git_range`; + foreach my $line (split(/\n/, $lines)) { + $line =~ /^([0-9a-fA-F]{40,40}) (.*)$/; + next if (!defined($1) || !defined($2)); + my $sha1 = $1; + my $subject = $2; + unshift(@commits, $sha1); + $git_commits{$sha1} = $subject; + } + } + die "$P: no git commits after extraction!\n" if (@commits == 0); + @ARGV = @commits; +} + +my $vname; for my $filename (@ARGV) { my $FILE; - if ($file) { + if ($git) { + open($FILE, '-|', "git format-patch -M --stdout -1 $filename") || + die "$P: $filename: git format-patch failed - $!\n"; + } elsif ($file) { open($FILE, '-|', "diff -u /dev/null $filename") || die "$P: $filename: diff failed - $!\n"; } elsif ($filename eq '-') { @@ -604,6 +954,8 @@ for my $filename (@ARGV) { } if ($filename eq '-') { $vname = 'Your patch'; + } elsif ($git) { + $vname = "Commit " . substr($filename, 0, 12) . ' ("' . $git_commits{$filename} . '")'; } else { $vname = $filename; } @@ -612,12 +964,45 @@ for my $filename (@ARGV) { push(@rawlines, $_); } close($FILE); + + if ($#ARGV > 0 && $quiet == 0) { + print '-' x length($vname) . "\n"; + print "$vname\n"; + print '-' x length($vname) . "\n"; + } + if (!process($filename)) { $exit = 1; } @rawlines = (); @lines = (); @fixed = (); + @fixed_inserted = (); + @fixed_deleted = (); + $fixlinenr = -1; + @modifierListFile = (); + @typeListFile = (); + build_types(); +} + +if (!$quiet) { + hash_show_words(\%use_type, "Used"); + hash_show_words(\%ignore_type, "Ignored"); + + if ($^V lt 5.10.0) { + print << "EOM" + +NOTE: perl $^V is not modern enough to detect all possible issues. + An upgrade to at least perl v5.10.0 is suggested. +EOM + } + if ($exit) { + print << "EOM" + +NOTE: If any of the errors are false positives, please report + them to the maintainer, see CHECKPATCH in MAINTAINERS. +EOM + } } exit($exit); @@ -709,6 +1094,18 @@ sub format_email { return $formatted_email; } +sub which { + my ($bin) = @_; + + foreach my $path (split(/:/, $ENV{PATH})) { + if (-e "$path/$bin") { + return "$path/$bin"; + } + } + + return ""; +} + sub which_conf { my ($conf) = @_; @@ -855,13 +1252,18 @@ sub sanitise_line { $res =~ s@(\#\s*(?:error|warning)\s+).*@$1$clean@; } + if ($allow_c99_comments && $res =~ m@(//.*$)@) { + my $match = $1; + $res =~ s/\Q$match\E/"$;" x length($match)/e; + } + return $res; } sub get_quoted_string { my ($line, $rawline) = @_; - return "" if ($line !~ m/(\"[X]+\")/g); + return "" if ($line !~ m/($String)/g); return substr($rawline, $-[0], $+[0] - $-[0]); } @@ -1470,13 +1872,13 @@ sub possible { for my $modifier (split(' ', $possible)) { if ($modifier !~ $notPermitted) { warn "MODIFIER: $modifier ($possible) ($line)\n" if ($dbg_possible); - push(@modifierList, $modifier); + push(@modifierListFile, $modifier); } } } else { warn "POSSIBLE: $possible ($line)\n" if ($dbg_possible); - push(@typeList, $possible); + push(@typeListFile, $possible); } build_types(); } else { @@ -1487,34 +1889,144 @@ sub possible { my $prefix = ''; sub show_type { - return defined $use_type{$_[0]} if (scalar keys %use_type > 0); + my ($type) = @_; + + $type =~ tr/[a-z]/[A-Z]/; + + return defined $use_type{$type} if (scalar keys %use_type > 0); - return !defined $ignore_type{$_[0]}; + return !defined $ignore_type{$type}; } sub report { - if (!show_type($_[1]) || - (defined $tst_only && $_[2] !~ /\Q$tst_only\E/)) { + my ($level, $type, $msg) = @_; + + if (!show_type($type) || + (defined $tst_only && $msg !~ /\Q$tst_only\E/)) { return 0; } - my $line; + my $output = ''; + if ($color) { + if ($level eq 'ERROR') { + $output .= RED; + } elsif ($level eq 'WARNING') { + $output .= YELLOW; + } else { + $output .= GREEN; + } + } + $output .= $prefix . $level . ':'; if ($show_types) { - $line = "$prefix$_[0]:$_[1]: $_[2]\n"; - } else { - $line = "$prefix$_[0]: $_[2]\n"; + $output .= BLUE if ($color); + $output .= "$type:"; } - $line = (split('\n', $line))[0] . "\n" if ($terse); + $output .= RESET if ($color); + $output .= ' ' . $msg . "\n"; - push(our @report, $line); + if ($showfile) { + my @lines = split("\n", $output, -1); + splice(@lines, 1, 1); + $output = join("\n", @lines); + } + $output = (split('\n', $output))[0] . "\n" if ($terse); + + push(our @report, $output); return 1; } + sub report_dump { our @report; } +sub fixup_current_range { + my ($lineRef, $offset, $length) = @_; + + if ($$lineRef =~ /^\@\@ -\d+,\d+ \+(\d+),(\d+) \@\@/) { + my $o = $1; + my $l = $2; + my $no = $o + $offset; + my $nl = $l + $length; + $$lineRef =~ s/\+$o,$l \@\@/\+$no,$nl \@\@/; + } +} + +sub fix_inserted_deleted_lines { + my ($linesRef, $insertedRef, $deletedRef) = @_; + + my $range_last_linenr = 0; + my $delta_offset = 0; + + my $old_linenr = 0; + my $new_linenr = 0; + + my $next_insert = 0; + my $next_delete = 0; + + my @lines = (); + + my $inserted = @{$insertedRef}[$next_insert++]; + my $deleted = @{$deletedRef}[$next_delete++]; + + foreach my $old_line (@{$linesRef}) { + my $save_line = 1; + my $line = $old_line; #don't modify the array + if ($line =~ /^(?:\+\+\+|\-\-\-)\s+\S+/) { #new filename + $delta_offset = 0; + } elsif ($line =~ /^\@\@ -\d+,\d+ \+\d+,\d+ \@\@/) { #new hunk + $range_last_linenr = $new_linenr; + fixup_current_range(\$line, $delta_offset, 0); + } + + while (defined($deleted) && ${$deleted}{'LINENR'} == $old_linenr) { + $deleted = @{$deletedRef}[$next_delete++]; + $save_line = 0; + fixup_current_range(\$lines[$range_last_linenr], $delta_offset--, -1); + } + + while (defined($inserted) && ${$inserted}{'LINENR'} == $old_linenr) { + push(@lines, ${$inserted}{'LINE'}); + $inserted = @{$insertedRef}[$next_insert++]; + $new_linenr++; + fixup_current_range(\$lines[$range_last_linenr], $delta_offset++, 1); + } + + if ($save_line) { + push(@lines, $line); + $new_linenr++; + } + + $old_linenr++; + } + + return @lines; +} + +sub fix_insert_line { + my ($linenr, $line) = @_; + + my $inserted = { + LINENR => $linenr, + LINE => $line, + }; + push(@fixed_inserted, $inserted); +} + +sub fix_delete_line { + my ($linenr, $line) = @_; + + my $deleted = { + LINENR => $linenr, + LINE => $line, + }; + + push(@fixed_deleted, $deleted); +} + sub ERROR { - if (report("ERROR", $_[0], $_[1])) { + my ($type, $msg) = @_; + + if (report("ERROR", $type, $msg)) { our $clean = 0; our $cnt_error++; return 1; @@ -1522,7 +2034,9 @@ sub ERROR { return 0; } sub WARN { - if (report("WARNING", $_[0], $_[1])) { + my ($type, $msg) = @_; + + if (report("WARNING", $type, $msg)) { our $clean = 0; our $cnt_warn++; return 1; @@ -1530,7 +2044,9 @@ sub WARN { return 0; } sub CHK { - if ($check && report("CHECK", $_[0], $_[1])) { + my ($type, $msg) = @_; + + if ($check && report("CHECK", $type, $msg)) { our $clean = 0; our $cnt_chk++; return 1; @@ -1640,7 +2156,7 @@ sub pos_last_openparen { } } - return $last_openparen + 1; + return length(expand_tabs(substr($line, 0, $last_openparen))) + 1; } sub process { @@ -1660,12 +2176,18 @@ sub process { our $clean = 1; my $signoff = 0; my $is_patch = 0; - - my $in_header_lines = 1; + my $in_header_lines = $file ? 0 : 1; my $in_commit_log = 0; #Scanning lines before patch - + my $has_commit_log = 0; #Encountered lines before patch + my $commit_log_possible_stack_dump = 0; + my $commit_log_long_line = 0; + my $commit_log_has_diff = 0; + my $reported_maintainer_file = 0; my $non_utf8_charset = 0; + my $last_blank_line = 0; + my $last_coalesced_string_linenr = -1; + our @report = (); our $cnt_lines = 0; our $cnt_error = 0; @@ -1677,6 +2199,7 @@ sub process { my $realline = 0; my $realcnt = 0; my $here = ''; + my $context_function; #undef'd unless there's a known function my $in_comment = 0; my $comment_edge = 0; my $first_line = 0; @@ -1710,12 +2233,12 @@ sub process { if ($rawline=~/^\+\+\+\s+(\S+)/) { $setup_docs = 0; - if ($1 =~ m@Documentation/kernel-parameters.txt$@) { + if ($1 =~ m@Documentation/admin-guide/kernel-parameters.rst$@) { $setup_docs = 1; } #next; } - if ($rawline=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) { + if ($rawline =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) { $realline=$1-1; if (defined $2) { $realcnt=$3+1; @@ -1783,15 +2306,19 @@ sub process { $realcnt = 0; $linenr = 0; + $fixlinenr = -1; foreach my $line (@lines) { $linenr++; + $fixlinenr++; my $sline = $line; #copy of $line $sline =~ s/$;/ /g; #with comments as spaces my $rawline = $rawlines[$linenr - 1]; #extract the line range in the file after the patch is applied - if ($line=~/^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@/) { + if (!$in_commit_log && + $line =~ /^\@\@ -\d+(?:,\d+)? \+(\d+)(,(\d+))? \@\@(.*)/) { + my $context = $4; $is_patch = 1; $first_line = $linenr + 1; $realline=$1-1; @@ -1807,6 +2334,11 @@ sub process { %suppress_whiletrailers = (); %suppress_export = (); $suppress_statement = 0; + if ($context =~ /\b(\w+)\s*\(/) { + $context_function = $1; + } else { + undef $context_function; + } next; # track the line number as we move through the hunk, note that @@ -1832,18 +2364,16 @@ sub process { my $hunk_line = ($realcnt != 0); -#make up the handle for any error we report on this line - $prefix = "$filename:$realline: " if ($emacs && $file); - $prefix = "$filename:$linenr: " if ($emacs && !$file); - $here = "#$linenr: " if (!$file); $here = "#$realline: " if ($file); + my $found_file = 0; # extract the filename as it passes if ($line =~ /^diff --git.*?(\S+)$/) { $realfile = $1; $realfile =~ s@^([^/]*)/@@ if (!$file); $in_commit_log = 0; + $found_file = 1; } elsif ($line =~ /^\+\+\+\s+(\S+)/) { $realfile = $1; $realfile =~ s@^([^/]*)/@@ if (!$file); @@ -1860,6 +2390,30 @@ sub process { ERROR("MODIFIED_INCLUDE_ASM", "do not modify files in include/asm, change architecture specific files in include/asm-<architecture>\n" . "$here$rawline\n"); } + $found_file = 1; + } + +#make up the handle for any error we report on this line + if ($showfile) { + $prefix = "$realfile:$realline: " + } elsif ($emacs) { + if ($file) { + $prefix = "$filename:$realline: "; + } else { + $prefix = "$filename:$linenr: "; + } + } + + if ($found_file) { + if (is_maintained_obsolete($realfile)) { + WARN("OBSOLETE", + "$realfile is marked as 'obsolete' in the MAINTAINERS hierarchy. No unnecessary modifications please.\n"); + } + if ($realfile =~ m@^(?:drivers/net/|net/|drivers/staging/)@) { + $check = 1; + } else { + $check = $check_orig; + } next; } @@ -1871,6 +2425,17 @@ sub process { $cnt_lines++ if ($realcnt != 0); +# Check if the commit log has what seems like a diff which can confuse patch + if ($in_commit_log && !$commit_log_has_diff && + (($line =~ m@^\s+diff\b.*a/[\w/]+@ && + $line =~ m@^\s+diff\b.*a/([\w/]+)\s+b/$1\b@) || + $line =~ m@^\s*(?:\-\-\-\s+a/|\+\+\+\s+b/)@ || + $line =~ m/^\s*\@\@ \-\d+,\d+ \+\d+,\d+ \@\@/)) { + ERROR("DIFF_IN_COMMIT_MSG", + "Avoid using diff content in the commit message - patch(1) might not work\n" . $herecurr); + $commit_log_has_diff = 1; + } + # Check for incorrect file permissions if ($line =~ /^new (file )?mode.*[7531]\d{0,2}$/) { my $permhere = $here . "FILE: $realfile\n"; @@ -1887,6 +2452,12 @@ sub process { $in_commit_log = 0; } +# Check if MAINTAINERS is being updated. If so, there's probably no need to +# emit the "does MAINTAINERS need updating?" message on file add/move/delete + if ($line =~ /^\s*MAINTAINERS\s*\|/) { + $reported_maintainer_file = 1; + } + # Check signature styles if (!$in_header_lines && $line =~ /^(\s*)([a-z0-9_-]+by:|$signature_tags)(\s*)(.*)/i) { @@ -1904,7 +2475,7 @@ sub process { if (WARN("BAD_SIGN_OFF", "Do not use whitespace before $ucfirst_sign_off\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] = + $fixed[$fixlinenr] = "$ucfirst_sign_off $email"; } } @@ -1912,7 +2483,7 @@ sub process { if (WARN("BAD_SIGN_OFF", "'$ucfirst_sign_off' is the preferred signature form\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] = + $fixed[$fixlinenr] = "$ucfirst_sign_off $email"; } @@ -1921,7 +2492,7 @@ sub process { if (WARN("BAD_SIGN_OFF", "Use a single space after $ucfirst_sign_off\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] = + $fixed[$fixlinenr] = "$ucfirst_sign_off $email"; } } @@ -1957,6 +2528,127 @@ sub process { } } +# Check email subject for common tools that don't need to be mentioned + if ($in_header_lines && + $line =~ /^Subject:.*\b(?:checkpatch|sparse|smatch)\b[^:]/i) { + WARN("EMAIL_SUBJECT", + "A patch subject line should describe the change not the tool that found it\n" . $herecurr); + } + +# Check for old stable address + if ($line =~ /^\s*cc:\s*.*<?\bstable\@kernel\.org\b>?.*$/i) { + ERROR("STABLE_ADDRESS", + "The 'stable' address should be 'stable\@vger.kernel.org'\n" . $herecurr); + } + +# Check for unwanted Gerrit info + if ($in_commit_log && $line =~ /^\s*change-id:/i) { + ERROR("GERRIT_CHANGE_ID", + "Remove Gerrit Change-Id's before submitting upstream.\n" . $herecurr); + } + +# Check if the commit log is in a possible stack dump + if ($in_commit_log && !$commit_log_possible_stack_dump && + ($line =~ /^\s*(?:WARNING:|BUG:)/ || + $line =~ /^\s*\[\s*\d+\.\d{6,6}\s*\]/ || + # timestamp + $line =~ /^\s*\[\<[0-9a-fA-F]{8,}\>\]/)) { + # stack dump address + $commit_log_possible_stack_dump = 1; + } + +# Check for line lengths > 75 in commit log, warn once + if ($in_commit_log && !$commit_log_long_line && + length($line) > 75 && + !($line =~ /^\s*[a-zA-Z0-9_\/\.]+\s+\|\s+\d+/ || + # file delta changes + $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ || + # filename then : + $line =~ /^\s*(?:Fixes:|Link:)/i || + # A Fixes: or Link: line + $commit_log_possible_stack_dump)) { + WARN("COMMIT_LOG_LONG_LINE", + "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr); + $commit_log_long_line = 1; + } + +# Reset possible stack dump if a blank line is found + if ($in_commit_log && $commit_log_possible_stack_dump && + $line =~ /^\s*$/) { + $commit_log_possible_stack_dump = 0; + } + +# Check for git id commit length and improperly formed commit descriptions + if ($in_commit_log && !$commit_log_possible_stack_dump && + $line !~ /^\s*(?:Link|Patchwork|http|https|BugLink):/i && + $line !~ /^This reverts commit [0-9a-f]{7,40}/ && + ($line =~ /\bcommit\s+[0-9a-f]{5,}\b/i || + ($line =~ /(?:\s|^)[0-9a-f]{12,40}(?:[\s"'\(\[]|$)/i && + $line !~ /[\<\[][0-9a-f]{12,40}[\>\]]/i && + $line !~ /\bfixes:\s*[0-9a-f]{12,40}/i))) { + my $init_char = "c"; + my $orig_commit = ""; + my $short = 1; + my $long = 0; + my $case = 1; + my $space = 1; + my $hasdesc = 0; + my $hasparens = 0; + my $id = '0123456789ab'; + my $orig_desc = "commit description"; + my $description = ""; + + if ($line =~ /\b(c)ommit\s+([0-9a-f]{5,})\b/i) { + $init_char = $1; + $orig_commit = lc($2); + } elsif ($line =~ /\b([0-9a-f]{12,40})\b/i) { + $orig_commit = lc($1); + } + + $short = 0 if ($line =~ /\bcommit\s+[0-9a-f]{12,40}/i); + $long = 1 if ($line =~ /\bcommit\s+[0-9a-f]{41,}/i); + $space = 0 if ($line =~ /\bcommit [0-9a-f]/i); + $case = 0 if ($line =~ /\b[Cc]ommit\s+[0-9a-f]{5,40}[^A-F]/); + if ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)"\)/i) { + $orig_desc = $1; + $hasparens = 1; + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s*$/i && + defined $rawlines[$linenr] && + $rawlines[$linenr] =~ /^\s*\("([^"]+)"\)/) { + $orig_desc = $1; + $hasparens = 1; + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("[^"]+$/i && + defined $rawlines[$linenr] && + $rawlines[$linenr] =~ /^\s*[^"]+"\)/) { + $line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("([^"]+)$/i; + $orig_desc = $1; + $rawlines[$linenr] =~ /^\s*([^"]+)"\)/; + $orig_desc .= " " . $1; + $hasparens = 1; + } + + ($id, $description) = git_commit_info($orig_commit, + $id, $orig_desc); + + if (defined($id) && + ($short || $long || $space || $case || ($orig_desc ne $description) || !$hasparens)) { + ERROR("GIT_COMMIT_ID", + "Please use git commit description style 'commit <12+ chars of sha1> (\"<title line>\")' - ie: '${init_char}ommit $id (\"$description\")'\n" . $herecurr); + } + } + +# Check for added, moved or deleted files + if (!$reported_maintainer_file && !$in_commit_log && + ($line =~ /^(?:new|deleted) file mode\s*\d+\s*$/ || + $line =~ /^rename (?:from|to) [\w\/\.\-]+\s*$/ || + ($line =~ /\{\s*([\w\/\.\-]*)\s*\=\>\s*([\w\/\.\-]*)\s*\}/ && + (defined($1) || defined($2))))) { + $is_patch = 1; + $reported_maintainer_file = 1; + WARN("FILE_PATH_CHANGES", + "added, moved or deleted file(s), does MAINTAINERS need updating?\n" . $herecurr); + } + # Check for wrappage within a valid hunk of the file if ($realcnt != 0 && $line !~ m{^(?:\+|-| |\\ No newline|$)}) { ERROR("CORRUPTED_PATCH", @@ -1964,20 +2656,6 @@ sub process { $herecurr) if (!$emitted_corrupt++); } -# Check for absolute kernel paths. - if ($tree) { - while ($line =~ m{(?:^|\s)(/\S*)}g) { - my $file = $1; - - if ($file =~ m{^(.*?)(?::\d+)+:?$} && - check_absolute_file($1, $herecurr)) { - # - } else { - check_absolute_file($file, $herecurr); - } - } - } - # UTF-8 regex found at http://www.w3.org/International/questions/qa-forms-utf-8.en.php if (($realfile =~ /^$/ || $line =~ /^\+/) && $rawline !~ m/^$UTF8*$/) { @@ -1994,9 +2672,11 @@ sub process { # Check if it's the start of a commit log # (not a header line and we haven't seen the patch filename) if ($in_header_lines && $realfile =~ /^$/ && - $rawline !~ /^(commit\b|from\b|[\w-]+:).+$/i) { + !($rawline =~ /^\s+(?:\S|$)/ || + $rawline =~ /^(?:commit\b|from\b|[\w-]+:)/i)) { $in_header_lines = 0; $in_commit_log = 1; + $has_commit_log = 1; } # Check if there is UTF-8 in a commit log when a mail header has explicitly @@ -2013,6 +2693,20 @@ sub process { "8-bit UTF-8 used in possible commit log\n" . $herecurr); } +# Check for absolute kernel paths in commit message + if ($tree && $in_commit_log) { + while ($line =~ m{(?:^|\s)(/\S*)}g) { + my $file = $1; + + if ($file =~ m{^(.*?)(?::\d+)+:?$} && + check_absolute_file($1, $herecurr)) { + # + } else { + check_absolute_file($file, $herecurr); + } + } + } + # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { @@ -2040,14 +2734,14 @@ sub process { if (ERROR("DOS_LINE_ENDINGS", "DOS line endings\n" . $herevet) && $fix) { - $fixed[$linenr - 1] =~ s/[\s\015]+$//; + $fixed[$fixlinenr] =~ s/[\s\015]+$//; } } elsif ($rawline =~ /^\+.*\S\s+$/ || $rawline =~ /^\+\s+$/) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; if (ERROR("TRAILING_WHITESPACE", "trailing whitespace\n" . $herevet) && $fix) { - $fixed[$linenr - 1] =~ s/\s+$//; + $fixed[$fixlinenr] =~ s/\s+$//; } $rpt_cleaners = 1; @@ -2055,6 +2749,7 @@ sub process { # Check for FSF mailing addresses. if ($rawline =~ /\bwrite to the Free/i || + $rawline =~ /\b675\s+Mass\s+Ave/i || $rawline =~ /\b59\s+Temple\s+Pl/i || $rawline =~ /\b51\s+Franklin\s+St/i) { my $herevet = "$here\n" . cat_vet($rawline) . "\n"; @@ -2068,7 +2763,7 @@ sub process { # Only applies when adding the entry originally, after that we do not have # sufficient context to determine whether it is indeed long enough. if ($realfile =~ /Kconfig/ && - $line =~ /.\s*config\s+/) { + $line =~ /^\+\s*config\s+/) { my $length = 0; my $cnt = $realcnt; my $ln = $linenr + 1; @@ -2081,10 +2776,11 @@ sub process { $is_end = $lines[$ln - 1] =~ /^\+/; next if ($f =~ /^-/); + last if (!$file && $f =~ /^\@\@/); - if ($lines[$ln - 1] =~ /.\s*(?:bool|tristate)\s*\"/) { + if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate)\s*\"/) { $is_start = 1; - } elsif ($lines[$ln - 1] =~ /.\s*(?:---)?help(?:---)?$/) { + } elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) { $length = -1; } @@ -2098,16 +2794,29 @@ sub process { } $length++; } - WARN("CONFIG_DESCRIPTION", - "please write a paragraph that describes the config symbol fully\n" . $herecurr) if ($is_start && $is_end && $length < 4); + if ($is_start && $is_end && $length < $min_conf_desc_length) { + WARN("CONFIG_DESCRIPTION", + "please write a paragraph that describes the config symbol fully\n" . $herecurr); + } #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } -# discourage the addition of CONFIG_EXPERIMENTAL in Kconfig. +# check for MAINTAINERS entries that don't have the right form + if ($realfile =~ /^MAINTAINERS$/ && + $rawline =~ /^\+[A-Z]:/ && + $rawline !~ /^\+[A-Z]:\t\S/) { + if (WARN("MAINTAINERS_STYLE", + "MAINTAINERS entries use one tab after TYPE:\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/^(\+[A-Z]):\s*/$1:\t/; + } + } + +# discourage the use of boolean for type definition attributes of Kconfig options if ($realfile =~ /Kconfig/ && - $line =~ /.\s*depends on\s+.*\bEXPERIMENTAL\b/) { - WARN("CONFIG_EXPERIMENTAL", - "Use of CONFIG_EXPERIMENTAL is deprecated. For alternatives, see https://lkml.org/lkml/2012/10/23/580\n"); + $line =~ /^\+\s*\bboolean\b/) { + WARN("CONFIG_TYPE_BOOLEAN", + "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); } if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && @@ -2125,65 +2834,93 @@ sub process { } # check for DT compatible documentation - if (defined $root && $realfile =~ /\.dts/ && - $rawline =~ /^\+\s*compatible\s*=/) { + if (defined $root && + (($realfile =~ /\.dtsi?$/ && $line =~ /^\+\s*compatible\s*=\s*\"/) || + ($realfile =~ /\.[ch]$/ && $line =~ /^\+.*\.compatible\s*=\s*\"/))) { + my @compats = $rawline =~ /\"([a-zA-Z0-9\-\,\.\+_]+)\"/g; + my $dt_path = $root . "/Documentation/devicetree/bindings/"; + my $vp_file = $dt_path . "vendor-prefixes.txt"; + foreach my $compat (@compats) { my $compat2 = $compat; - my $dt_path = $root . "/Documentation/devicetree/bindings/"; - $compat2 =~ s/\,[a-z]*\-/\,<\.\*>\-/; - `grep -Erq "$compat|$compat2" $dt_path`; + $compat2 =~ s/\,[a-zA-Z0-9]*\-/\,<\.\*>\-/; + my $compat3 = $compat; + $compat3 =~ s/\,([a-z]*)[0-9]*\-/\,$1<\.\*>\-/; + `grep -Erq "$compat|$compat2|$compat3" $dt_path`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", "DT compatible string \"$compat\" appears un-documented -- check $dt_path\n" . $herecurr); } - my $vendor = $compat; - my $vendor_path = $dt_path . "vendor-prefixes.txt"; - next if (! -f $vendor_path); - $vendor =~ s/^([a-zA-Z0-9]+)\,.*/$1/; - `grep -Eq "$vendor" $vendor_path`; + next if $compat !~ /^([a-zA-Z0-9\-]+)\,/; + my $vendor = $1; + `grep -Eq "^$vendor\\b" $vp_file`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", - "DT compatible string vendor \"$vendor\" appears un-documented -- check $vendor_path\n" . $herecurr); + "DT compatible string vendor \"$vendor\" appears un-documented -- check $vp_file\n" . $herecurr); } } } # check we are in a valid source file if not then ignore this hunk - next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/); - -#line length limit - if ($line =~ /^\+/ && $prevrawline !~ /\/\*\*/ && - $rawline !~ /^.\s*\*\s*\@$Ident\s/ && - !($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(KERN_\S+\s*|[^"]*))?"[X\t]*"\s*(?:|,|\)\s*;)\s*$/ || - $line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) && - $length > $max_line_length) - { - WARN("LONG_LINE", - "line over $max_line_length characters\n" . $herecurr); - } + next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); -# Check for user-visible strings broken across lines, which breaks the ability -# to grep for the string. Make exceptions when the previous string ends in a -# newline (multiple lines in one string constant) or '\t', '\r', ';', or '{' -# (common in inline assembly) or is a octal \123 or hexadecimal \xaf value - if ($line =~ /^\+\s*"/ && - $prevline =~ /"\s*$/ && - $prevrawline !~ /(?:\\(?:[ntr]|[0-7]{1,3}|x[0-9a-fA-F]{1,2})|;\s*|\{\s*)"\s*$/) { - WARN("SPLIT_STRING", - "quoted string split across lines\n" . $hereprev); - } +# line length limit (with some exclusions) +# +# There are a few types of lines that may extend beyond $max_line_length: +# logging functions like pr_info that end in a string +# lines with a single string +# #defines that are a single string +# +# There are 3 different line length message types: +# LONG_LINE_COMMENT a comment starts before but extends beyond $max_linelength +# LONG_LINE_STRING a string starts before but extends beyond $max_line_length +# LONG_LINE all other lines longer than $max_line_length +# +# if LONG_LINE is ignored, the other 2 types are also ignored +# -# check for spaces before a quoted newline - if ($rawline =~ /^.*\".*\s\\n/) { - if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", - "unnecessary whitespace before a quoted newline\n" . $herecurr) && - $fix) { - $fixed[$linenr - 1] =~ s/^(\+.*\".*)\s+\\n/$1\\n/; + if ($line =~ /^\+/ && $length > $max_line_length) { + my $msg_type = "LONG_LINE"; + + # Check the allowed long line types first + + # logging functions that end in a string that starts + # before $max_line_length + if ($line =~ /^\+\s*$logFunctions\s*\(\s*(?:(?:KERN_\S+\s*|[^"]*))?($String\s*(?:|,|\)\s*;)\s*)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = ""; + + # lines with only strings (w/ possible termination) + # #defines with only strings + } elsif ($line =~ /^\+\s*$String\s*(?:\s*|,|\)\s*;)\s*$/ || + $line =~ /^\+\s*#\s*define\s+\w+\s+$String$/) { + $msg_type = ""; + + # EFI_GUID is another special case + } elsif ($line =~ /^\+.*\bEFI_GUID\s*\(/) { + $msg_type = ""; + + # Otherwise set the alternate message types + + # a comment starts before $max_line_length + } elsif ($line =~ /($;[\s$;]*)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = "LONG_LINE_COMMENT" + + # a quoted string starts before $max_line_length + } elsif ($sline =~ /\s*($String(?:\s*(?:\\|,\s*|\)\s*;\s*))?)$/ && + length(expand_tabs(substr($line, 1, length($line) - length($1) - 1))) <= $max_line_length) { + $msg_type = "LONG_LINE_STRING" } + if ($msg_type ne "" && + (show_type("LONG_LINE") || show_type($msg_type))) { + WARN($msg_type, + "line over $max_line_length characters\n" . $herecurr); + } } # check for adding lines without a newline. @@ -2207,7 +2944,7 @@ sub process { } # check we are in a valid source file C or perl if not then ignore this hunk - next if ($realfile !~ /\.(h|c|pl)$/); + next if ($realfile !~ /\.(h|c|pl|dtsi|dts)$/); # at the beginning of a line any tabs must come first and anything # more than 8 must use tabs. @@ -2218,7 +2955,7 @@ sub process { if (ERROR("CODE_INDENT", "code indent should use tabs where possible\n" . $herevet) && $fix) { - $fixed[$linenr - 1] =~ s/^\+([ \t]+)/"\+" . tabify($1)/e; + $fixed[$fixlinenr] =~ s/^\+([ \t]+)/"\+" . tabify($1)/e; } } @@ -2228,9 +2965,9 @@ sub process { if (WARN("SPACE_BEFORE_TAB", "please, no space before tabs\n" . $herevet) && $fix) { - while ($fixed[$linenr - 1] =~ + while ($fixed[$fixlinenr] =~ s/(^\+.*) {8,8}\t/$1\t\t/) {} - while ($fixed[$linenr - 1] =~ + while ($fixed[$fixlinenr] =~ s/(^\+.*) +\t/$1\t/) {} } } @@ -2241,9 +2978,22 @@ sub process { "Logical continuations should be on the previous line\n" . $hereprev); } +# check indentation starts on a tab stop + if ($^V && $^V ge 5.10.0 && + $sline =~ /^\+\t+( +)(?:$c90_Keywords\b|\{\s*$|\}\s*(?:else\b|while\b|\s*$))/) { + my $indent = length($1); + if ($indent % 8) { + if (WARN("TABSTOP", + "Statements should start on a tabstop\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s@(^\+\t+) +@$1 . "\t" x ($indent/8)@e; + } + } + } + # check multi-line statement indentation matches previous line if ($^V && $^V ge 5.10.0 && - $prevline =~ /^\+(\t*)(if \(|$Ident\().*(\&\&|\|\||,)\s*$/) { + $prevline =~ /^\+([ \t]*)((?:$c90_Keywords(?:\s+if)\s*)|(?:$Declare\s*)?(?:$Ident|\(\s*\*\s*$Ident\s*\))\s*|(?:\*\s*)*$Lval\s*=\s*$Ident\s*)\(.*(\&\&|\|\||,)\s*$/) { $prevline =~ /^\+(\t*)(.*)$/; my $oldindent = $1; my $rest = $2; @@ -2264,45 +3014,154 @@ sub process { if (CHK("PARENTHESIS_ALIGNMENT", "Alignment should match open parenthesis\n" . $hereprev) && $fix && $line =~ /^\+/) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/^\+[ \t]*/\+$goodtabindent/; } } } } - if ($line =~ /^\+.*\*[ \t]*\)[ \t]+(?!$Assignment|$Arithmetic)/) { +# check for space after cast like "(int) foo" or "(struct foo) bar" +# avoid checking a few false positives: +# "sizeof(<type>)" or "__alignof__(<type>)" +# function pointer declarations like "(*foo)(int) = bar;" +# structure definitions like "(struct foo) { 0 };" +# multiline macros that define functions +# known attributes or the __attribute__ keyword + if ($line =~ /^\+(.*)\(\s*$Type\s*\)([ \t]++)((?![={]|\\$|$Attribute|__attribute__))/ && + (!defined($1) || $1 !~ /\b(?:sizeof|__alignof__)\s*$/)) { if (CHK("SPACING", - "No space is necessary after a cast\n" . $hereprev) && + "No space is necessary after a cast\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ - s/^(\+.*\*[ \t]*\))[ \t]+/$1/; + $fixed[$fixlinenr] =~ + s/(\(\s*$Type\s*\))[ \t]+/$1/; } } +# Block comment styles +# Networking with an initial /* if ($realfile =~ m@^(drivers/net/|net/)@ && $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ && - $rawline =~ /^\+[ \t]*\*/) { + $rawline =~ /^\+[ \t]*\*/ && + $realline > 2) { WARN("NETWORKING_BLOCK_COMMENT_STYLE", "networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev); } - if ($realfile =~ m@^(drivers/net/|net/)@ && - $prevrawline =~ /^\+[ \t]*\/\*/ && #starting /* +# Block comments use * on subsequent lines + if ($prevline =~ /$;[ \t]*$/ && #ends in comment + $prevrawline =~ /^\+.*?\/\*/ && #starting /* $prevrawline !~ /\*\/[ \t]*$/ && #no trailing */ $rawline =~ /^\+/ && #line is new $rawline !~ /^\+[ \t]*\*/) { #no leading * - WARN("NETWORKING_BLOCK_COMMENT_STYLE", - "networking block comments start with * on subsequent lines\n" . $hereprev); + WARN("BLOCK_COMMENT_STYLE", + "Block comments use * on subsequent lines\n" . $hereprev); } - if ($realfile =~ m@^(drivers/net/|net/)@ && - $rawline !~ m@^\+[ \t]*\*/[ \t]*$@ && #trailing */ +# Block comments use */ on trailing lines + if ($rawline !~ m@^\+[ \t]*\*/[ \t]*$@ && #trailing */ $rawline !~ m@^\+.*/\*.*\*/[ \t]*$@ && #inline /*...*/ $rawline !~ m@^\+.*\*{2,}/[ \t]*$@ && #trailing **/ $rawline =~ m@^\+[ \t]*.+\*\/[ \t]*$@) { #non blank */ - WARN("NETWORKING_BLOCK_COMMENT_STYLE", - "networking block comments put the trailing */ on a separate line\n" . $herecurr); + WARN("BLOCK_COMMENT_STYLE", + "Block comments use a trailing */ on a separate line\n" . $herecurr); + } + +# Block comment * alignment + if ($prevline =~ /$;[ \t]*$/ && #ends in comment + $line =~ /^\+[ \t]*$;/ && #leading comment + $rawline =~ /^\+[ \t]*\*/ && #leading * + (($prevrawline =~ /^\+.*?\/\*/ && #leading /* + $prevrawline !~ /\*\/[ \t]*$/) || #no trailing */ + $prevrawline =~ /^\+[ \t]*\*/)) { #leading * + my $oldindent; + $prevrawline =~ m@^\+([ \t]*/?)\*@; + if (defined($1)) { + $oldindent = expand_tabs($1); + } else { + $prevrawline =~ m@^\+(.*/?)\*@; + $oldindent = expand_tabs($1); + } + $rawline =~ m@^\+([ \t]*)\*@; + my $newindent = $1; + $newindent = expand_tabs($newindent); + if (length($oldindent) ne length($newindent)) { + WARN("BLOCK_COMMENT_STYLE", + "Block comments should align the * on each line\n" . $hereprev); + } + } + +# check for missing blank lines after struct/union declarations +# with exceptions for various attributes and macros + if ($prevline =~ /^[\+ ]};?\s*$/ && + $line =~ /^\+/ && + !($line =~ /^\+\s*$/ || + $line =~ /^\+\s*EXPORT_SYMBOL/ || + $line =~ /^\+\s*MODULE_/i || + $line =~ /^\+\s*\#\s*(?:end|elif|else)/ || + $line =~ /^\+[a-z_]*init/ || + $line =~ /^\+\s*(?:static\s+)?[A-Z_]*ATTR/ || + $line =~ /^\+\s*DECLARE/ || + $line =~ /^\+\s*__setup/)) { + if (CHK("LINE_SPACING", + "Please use a blank line after function/struct/union/enum declarations\n" . $hereprev) && + $fix) { + fix_insert_line($fixlinenr, "\+"); + } + } + +# check for multiple consecutive blank lines + if ($prevline =~ /^[\+ ]\s*$/ && + $line =~ /^\+\s*$/ && + $last_blank_line != ($linenr - 1)) { + if (CHK("LINE_SPACING", + "Please don't use multiple blank lines\n" . $hereprev) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } + + $last_blank_line = $linenr; + } + +# check for missing blank lines after declarations + if ($sline =~ /^\+\s+\S/ && #Not at char 1 + # actual declarations + ($prevline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || + # function pointer declarations + $prevline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || + # foo bar; where foo is some local typedef or #define + $prevline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || + # known declaration macros + $prevline =~ /^\+\s+$declaration_macros/) && + # for "else if" which can look like "$Ident $Ident" + !($prevline =~ /^\+\s+$c90_Keywords\b/ || + # other possible extensions of declaration lines + $prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || + # not starting a section or a macro "\" extended line + $prevline =~ /(?:\{\s*|\\)$/) && + # looks like a declaration + !($sline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || + # function pointer declarations + $sline =~ /^\+\s+$Declare\s*\(\s*\*\s*$Ident\s*\)\s*[=,;:\[\(]/ || + # foo bar; where foo is some local typedef or #define + $sline =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || + # known declaration macros + $sline =~ /^\+\s+$declaration_macros/ || + # start of struct or union or enum + $sline =~ /^\+\s+(?:union|struct|enum|typedef)\b/ || + # start or end of block or continuation of declaration + $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || + # bitfield continuation + $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ || + # other possible extensions of declaration lines + $sline =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/) && + # indentation of previous and current line are the same + (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/)) { + if (WARN("LINE_SPACING", + "Missing a blank line after declarations\n" . $hereprev) && + $fix) { + fix_insert_line($fixlinenr, "\+"); + } } # check for spaces at the beginning of a line. @@ -2315,17 +3174,46 @@ sub process { if (WARN("LEADING_SPACE", "please, no spaces at the start of a line\n" . $herevet) && $fix) { - $fixed[$linenr - 1] =~ s/^\+([ \t]+)/"\+" . tabify($1)/e; + $fixed[$fixlinenr] =~ s/^\+([ \t]+)/"\+" . tabify($1)/e; } } # check we are in a valid C source file if not then ignore this hunk next if ($realfile !~ /\.(h|c)$/); -# discourage the addition of CONFIG_EXPERIMENTAL in #if(def). - if ($line =~ /^\+\s*\#\s*if.*\bCONFIG_EXPERIMENTAL\b/) { - WARN("CONFIG_EXPERIMENTAL", - "Use of CONFIG_EXPERIMENTAL is deprecated. For alternatives, see https://lkml.org/lkml/2012/10/23/580\n"); +# check if this appears to be the start function declaration, save the name + if ($sline =~ /^\+\{\s*$/ && + $prevline =~ /^\+(?:(?:(?:$Storage|$Inline)\s*)*\s*$Type\s*)?($Ident)\(/) { + $context_function = $1; + } + +# check if this appears to be the end of function declaration + if ($sline =~ /^\+\}\s*$/) { + undef $context_function; + } + +# check indentation of any line with a bare else +# (but not if it is a multiple line "if (foo) return bar; else return baz;") +# if the previous line is a break or return and is indented 1 tab more... + if ($sline =~ /^\+([\t]+)(?:}[ \t]*)?else(?:[ \t]*{)?\s*$/) { + my $tabs = length($1) + 1; + if ($prevline =~ /^\+\t{$tabs,$tabs}break\b/ || + ($prevline =~ /^\+\t{$tabs,$tabs}return\b/ && + defined $lines[$linenr] && + $lines[$linenr] !~ /^[ \+]\t{$tabs,$tabs}return/)) { + WARN("UNNECESSARY_ELSE", + "else is not generally useful after a break or return\n" . $hereprev); + } + } + +# check indentation of a line with a break; +# if the previous line is a goto or return and is indented the same # of tabs + if ($sline =~ /^\+([\t]+)break\s*;\s*$/) { + my $tabs = $1; + if ($prevline =~ /^\+$tabs(?:goto|return)\b/) { + WARN("UNNECESSARY_BREAK", + "break is not useful after a goto or return\n" . $hereprev); + } } # check for RCS/CVS revision markers @@ -2356,7 +3244,7 @@ sub process { my ($stat, $cond, $line_nr_next, $remain_next, $off_next, $realline_next); #print "LINE<$line>\n"; - if ($linenr >= $suppress_statement && + if ($linenr > $suppress_statement && $realcnt && $sline =~ /.\s*\S/) { ($stat, $cond, $line_nr_next, $remain_next, $off_next) = ctx_statement_block($linenr, $realcnt, 0); @@ -2457,7 +3345,7 @@ sub process { # if/while/etc brace do not go on next line, unless defining a do while loop, # or if that brace on the next line is for something else - if ($line =~ /(.*)\b((?:if|while|for|switch)\s*\(|do\b|else\b)/ && $line !~ /^.\s*\#/) { + if ($line =~ /(.*)\b((?:if|while|for|switch|(?:[a-z_]+|)for_each[a-z_]+)\s*\(|do\b|else\b)/ && $line !~ /^.\s*\#/) { my $pre_ctx = "$1$2"; my ($level, @ctx) = ctx_statement_level($linenr, $realcnt, 0); @@ -2484,7 +3372,7 @@ sub process { #print "realcnt<$realcnt> ctx_cnt<$ctx_cnt>\n"; #print "pre<$pre_ctx>\nline<$line>\nctx<$ctx>\nnext<$lines[$ctx_ln - 1]>\n"; - if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln -1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) { + if ($ctx !~ /{\s*/ && defined($lines[$ctx_ln - 1]) && $lines[$ctx_ln - 1] =~ /^\+\s*{/) { ERROR("OPEN_BRACE", "that open brace { should be on the previous line\n" . "$here\n$ctx\n$rawlines[$ctx_ln - 1]\n"); @@ -2503,7 +3391,7 @@ sub process { } # Check relative indent for conditionals and blocks. - if ($line =~ /\b(?:(?:if|while|for)\s*\(|do\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) { + if ($line =~ /\b(?:(?:if|while|for|(?:[a-z_]+|)for_each[a-z_]+)\s*\(|(?:do|else)\b)/ && $line !~ /^.\s*#/ && $line !~ /\}\s*while\s*/) { ($stat, $cond, $line_nr_next, $remain_next, $off_next) = ctx_statement_block($linenr, $realcnt, 0) if (!defined $stat); @@ -2511,15 +3399,22 @@ sub process { substr($s, 0, length($c), ''); - # Make sure we remove the line prefixes as we have - # none on the first line, and are going to readd them - # where necessary. - $s =~ s/\n./\n/gs; + # remove inline comments + $s =~ s/$;/ /g; + $c =~ s/$;/ /g; # Find out how long the conditional actually is. my @newlines = ($c =~ /\n/gs); my $cond_lines = 1 + $#newlines; + # Make sure we remove the line prefixes as we have + # none on the first line, and are going to readd them + # where necessary. + $s =~ s/\n./\n/gs; + while ($s =~ /\n\s+\\\n/) { + $cond_lines += $s =~ s/\n\s+\\\n/\n/g; + } + # We want to check the first line inside the block # starting at the end of the conditional, so remove: # 1) any blank line termination @@ -2585,8 +3480,12 @@ sub process { #print "line<$line> prevline<$prevline> indent<$indent> sindent<$sindent> check<$check> continuation<$continuation> s<$s> cond_lines<$cond_lines> stat_real<$stat_real> stat<$stat>\n"; - if ($check && (($sindent % 8) != 0 || - ($sindent <= $indent && $s ne ''))) { + if ($check && $s ne '' && + (($sindent % 8) != 0 || + ($sindent < $indent) || + ($sindent == $indent && + ($s !~ /^\s*(?:\}|\{|else\b)/)) || + ($sindent > $indent + 8))) { WARN("SUSPECT_CODE_INDENT", "suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n"); } @@ -2608,6 +3507,42 @@ sub process { #ignore lines not being added next if ($line =~ /^[^\+]/); +# check for dereferences that span multiple lines + if ($prevline =~ /^\+.*$Lval\s*(?:\.|->)\s*$/ && + $line =~ /^\+\s*(?!\#\s*(?!define\s+|if))\s*$Lval/) { + $prevline =~ /($Lval\s*(?:\.|->))\s*$/; + my $ref = $1; + $line =~ /^.\s*($Lval)/; + $ref .= $1; + $ref =~ s/\s//g; + WARN("MULTILINE_DEREFERENCE", + "Avoid multiple line dereference - prefer '$ref'\n" . $hereprev); + } + +# check for declarations of signed or unsigned without int + while ($line =~ m{\b($Declare)\s*(?!char\b|short\b|int\b|long\b)\s*($Ident)?\s*[=,;\[\)\(]}g) { + my $type = $1; + my $var = $2; + $var = "" if (!defined $var); + if ($type =~ /^(?:(?:$Storage|$Inline|$Attribute)\s+)*((?:un)?signed)((?:\s*\*)*)\s*$/) { + my $sign = $1; + my $pointer = $2; + + $pointer = "" if (!defined $pointer); + + if (WARN("UNSPECIFIED_INT", + "Prefer '" . trim($sign) . " int" . rtrim($pointer) . "' to bare use of '$sign" . rtrim($pointer) . "'\n" . $herecurr) && + $fix) { + my $decl = trim($sign) . " int "; + my $comp_pointer = $pointer; + $comp_pointer =~ s/\s//g; + $decl .= $comp_pointer; + $decl = rtrim($decl) if ($var eq ""); + $fixed[$fixlinenr] =~ s@\b$sign\s*\Q$pointer\E\s*$var\b@$decl$var@; + } + } + } + # TEST: allow direct testing of the type matcher. if ($dbg_type) { if ($line =~ /^.\s*$Declare\s*$/) { @@ -2634,8 +3569,18 @@ sub process { # check for initialisation to aggregates open brace on the next line if ($line =~ /^.\s*{/ && $prevline =~ /(?:^|[^=])=\s*$/) { - ERROR("OPEN_BRACE", - "that open brace { should be on the previous line\n" . $hereprev); + if (ERROR("OPEN_BRACE", + "that open brace { should be on the previous line\n" . $hereprev) && + $fix && $prevline =~ /^\+/ && $line =~ /^\+/) { + fix_delete_line($fixlinenr - 1, $prevrawline); + fix_delete_line($fixlinenr, $rawline); + my $fixedline = $prevrawline; + $fixedline =~ s/\s*=\s*$/ = {/; + fix_insert_line($fixlinenr, $fixedline); + $fixedline = $line; + $fixedline =~ s/^(.\s*)\{\s*/$1/; + fix_insert_line($fixlinenr, $fixedline); + } } # @@ -2660,10 +3605,10 @@ sub process { if (ERROR("C99_COMMENTS", "do not use C99 // comments\n" . $herecurr) && $fix) { - my $line = $fixed[$linenr - 1]; + my $line = $fixed[$fixlinenr]; if ($line =~ /\/\/(.*)$/) { my $comment = trim($1); - $fixed[$linenr - 1] =~ s@\/\/(.*)$@/\* $comment \*/@; + $fixed[$fixlinenr] =~ s@\/\/(.*)$@/\* $comment \*/@; } } } @@ -2717,24 +3662,30 @@ sub process { } # check for global initialisers. - if ($line =~ /^\+(\s*$Type\s*$Ident\s*(?:\s+$Modifier))*\s*=\s*(0|NULL|false)\s*;/) { + if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) { if (ERROR("GLOBAL_INITIALISERS", - "do not initialise globals to 0 or NULL\n" . - $herecurr) && + "do not initialise globals to $1\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/($Type\s*$Ident\s*(?:\s+$Modifier))*\s*=\s*(0|NULL|false)\s*;/$1;/; + $fixed[$fixlinenr] =~ s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*$zero_initializer\s*;/$1;/; } } # check for static initialisers. - if ($line =~ /^\+.*\bstatic\s.*=\s*(0|NULL|false)\s*;/) { + if ($line =~ /^\+.*\bstatic\s.*=\s*($zero_initializer)\s*;/) { if (ERROR("INITIALISED_STATIC", - "do not initialise statics to 0 or NULL\n" . + "do not initialise statics to $1\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/(\bstatic\s.*?)\s*=\s*(0|NULL|false)\s*;/$1;/; + $fixed[$fixlinenr] =~ s/(\bstatic\s.*?)\s*=\s*$zero_initializer\s*;/$1;/; } } +# check for misordered declarations of char/short/int/long with signed/unsigned + while ($sline =~ m{(\b$TypeMisordered\b)}g) { + my $tmp = trim($1); + WARN("MISORDERED_TYPE", + "type '$tmp' should be specified in [[un]signed] [short|int|long|long long] order\n" . $herecurr); + } + # check for static const char * arrays. if ($line =~ /\bstatic\s+const\s+char\s*\*\s*(\w+)\s*\[\s*\]\s*=\s*/) { WARN("STATIC_CONST_CHAR_ARRAY", @@ -2749,21 +3700,44 @@ sub process { $herecurr); } +# check for const <foo> const where <foo> is not a pointer or array type + if ($sline =~ /\bconst\s+($BasicType)\s+const\b/) { + my $found = $1; + if ($sline =~ /\bconst\s+\Q$found\E\s+const\b\s*\*/) { + WARN("CONST_CONST", + "'const $found const *' should probably be 'const $found * const'\n" . $herecurr); + } elsif ($sline !~ /\bconst\s+\Q$found\E\s+const\s+\w+\s*\[/) { + WARN("CONST_CONST", + "'const $found const' should probably be 'const $found'\n" . $herecurr); + } + } + +# check for non-global char *foo[] = {"bar", ...} declarations. + if ($line =~ /^.\s+(?:static\s+|const\s+)?char\s+\*\s*\w+\s*\[\s*\]\s*=\s*\{/) { + WARN("STATIC_CONST_CHAR_ARRAY", + "char * array declaration might be better as static const\n" . + $herecurr); + } + +# check for sizeof(foo)/sizeof(foo[0]) that could be ARRAY_SIZE(foo) + if ($line =~ m@\bsizeof\s*\(\s*($Lval)\s*\)@) { + my $array = $1; + if ($line =~ m@\b(sizeof\s*\(\s*\Q$array\E\s*\)\s*/\s*sizeof\s*\(\s*\Q$array\E\s*\[\s*0\s*\]\s*\))@) { + my $array_div = $1; + if (WARN("ARRAY_SIZE", + "Prefer ARRAY_SIZE($array)\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$array_div\E/ARRAY_SIZE($array)/; + } + } + } + # check for function declarations without arguments like "int foo()" if ($line =~ /(\b$Type\s+$Ident)\s*\(\s*\)/) { if (ERROR("FUNCTION_WITHOUT_ARGS", "Bad function definition - $1() should probably be $1(void)\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/(\b($Type)\s+($Ident))\s*\(\s*\)/$2 $3(void)/; - } - } - -# check for uses of DEFINE_PCI_DEVICE_TABLE - if ($line =~ /\bDEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=/) { - if (WARN("DEFINE_PCI_DEVICE_TABLE", - "Prefer struct pci_device_id over deprecated DEFINE_PCI_DEVICE_TABLE\n" . $herecurr) && - $fix) { - $fixed[$linenr - 1] =~ s/\b(?:static\s+|)DEFINE_PCI_DEVICE_TABLE\s*\(\s*(\w+)\s*\)\s*=\s*/static const struct pci_device_id $1\[\] = /; + $fixed[$fixlinenr] =~ s/(\b($Type)\s+($Ident))\s*\(\s*\)/$2 $3(void)/; } } @@ -2773,7 +3747,7 @@ sub process { $line !~ /\btypedef\s+$Type\s*\(\s*\*?$Ident\s*\)\s*\(/ && $line !~ /\btypedef\s+$Type\s+$Ident\s*\(/ && $line !~ /\b$typeTypedefs\b/ && - $line !~ /\b__bitwise(?:__|)\b/) { + $line !~ /\b__bitwise\b/) { WARN("NEW_TYPEDEFS", "do not add new typedefs\n" . $herecurr); } @@ -2800,7 +3774,7 @@ sub process { my $sub_from = $ident; my $sub_to = $ident; $sub_to =~ s/\Q$from\E/$to/; - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s@\Q$sub_from\E@$sub_to@; } } @@ -2828,19 +3802,21 @@ sub process { my $sub_from = $match; my $sub_to = $match; $sub_to =~ s/\Q$from\E/$to/; - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s@\Q$sub_from\E@$sub_to@; } } } -# # no BUG() or BUG_ON() -# if ($line =~ /\b(BUG|BUG_ON)\b/) { -# print "Try to use WARN_ON & Recovery code rather than BUG() or BUG_ON()\n"; -# print "$herecurr"; -# $clean = 0; -# } +# avoid BUG() or BUG_ON() + if ($line =~ /\b(?:BUG|BUG_ON)\b/) { + my $msg_type = \&WARN; + $msg_type = \&CHK if ($file); + &{$msg_type}("AVOID_BUG", + "Avoid crashing the kernel - try using WARN_ON & recovery code rather than BUG() or BUG_ON()\n" . $herecurr); + } +# avoid LINUX_VERSION_CODE if ($line =~ /\bLINUX_VERSION_CODE\b/) { WARN("LINUX_VERSION_CODE", "LINUX_VERSION_CODE should be avoided, code should be for the version to which it is merged\n" . $herecurr); @@ -2849,7 +3825,7 @@ sub process { # check for uses of printk_ratelimit if ($line =~ /\bprintk_ratelimit\s*\(/) { WARN("PRINTK_RATELIMITED", -"Prefer printk_ratelimited or pr_<level>_ratelimited to printk_ratelimit\n" . $herecurr); + "Prefer printk_ratelimited or pr_<level>_ratelimited to printk_ratelimit\n" . $herecurr); } # printk should use KERN_* levels. Note that follow on printk's on the @@ -2883,14 +3859,14 @@ sub process { my $level2 = $level; $level2 = "dbg" if ($level eq "debug"); WARN("PREFER_PR_LEVEL", - "Prefer netdev_$level2(netdev, ... then dev_$level2(dev, ... then pr_$level(... to printk(KERN_$orig ...\n" . $herecurr); + "Prefer [subsystem eg: netdev]_$level2([subsystem]dev, ... then dev_$level2(dev, ... then pr_$level(... to printk(KERN_$orig ...\n" . $herecurr); } if ($line =~ /\bpr_warning\s*\(/) { if (WARN("PREFER_PR_LEVEL", "Prefer pr_warn(... to pr_warning(...\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/\bpr_warning\b/pr_warn/; } } @@ -2904,19 +3880,50 @@ sub process { "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr); } +# ENOSYS means "bad syscall nr" and nothing else. This will have a small +# number of false positives, but assembly files are not checked, so at +# least the arch entry code will not trigger this warning. + if ($line =~ /\bENOSYS\b/) { + WARN("ENOSYS", + "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr); + } + # function brace can't be on same line, except for #defines of do while, # or if closed on same line - if (($line=~/$Type\s*$Ident\(.*\).*\s\{/) and + if (($line=~/$Type\s*$Ident\(.*\).*\s*{/) and !($line=~/\#\s*define.*do\s\{/) and !($line=~/}/)) { - ERROR("OPEN_BRACE", - "open brace '{' following function declarations go on the next line\n" . $herecurr); + if (ERROR("OPEN_BRACE", + "open brace '{' following function declarations go on the next line\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + my $fixed_line = $rawline; + $fixed_line =~ /(^..*$Type\s*$Ident\(.*\)\s*){(.*)$/; + my $line1 = $1; + my $line2 = $2; + fix_insert_line($fixlinenr, ltrim($line1)); + fix_insert_line($fixlinenr, "\+{"); + if ($line2 !~ /^\s*$/) { + fix_insert_line($fixlinenr, "\+\t" . trim($line2)); + } + } } # open braces for enum, union and struct go on the same line. if ($line =~ /^.\s*{/ && $prevline =~ /^.\s*(?:typedef\s+)?(enum|union|struct)(?:\s+$Ident)?\s*$/) { - ERROR("OPEN_BRACE", - "open brace '{' following $1 go on the same line\n" . $hereprev); + if (ERROR("OPEN_BRACE", + "open brace '{' following $1 go on the same line\n" . $hereprev) && + $fix && $prevline =~ /^\+/ && $line =~ /^\+/) { + fix_delete_line($fixlinenr - 1, $prevrawline); + fix_delete_line($fixlinenr, $rawline); + my $fixedline = rtrim($prevrawline) . " {"; + fix_insert_line($fixlinenr, $fixedline); + $fixedline = $rawline; + $fixedline =~ s/^(.\s*)\{\s*/$1\t/; + if ($fixedline !~ /^\+\s*$/) { + fix_insert_line($fixlinenr, $fixedline); + } + } } # missing space after union, struct or enum definition @@ -2924,7 +3931,7 @@ sub process { if (WARN("SPACING", "missing space after $1 definition\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/^(.\s*(?:typedef\s+)?(?:enum|union|struct)(?:\s+$Ident){1,2})([=\{])/$1 $2/; } } @@ -2932,10 +3939,7 @@ sub process { # Function pointer declarations # check spacing between type, funcptr, and args # canonical declaration is "type (*funcptr)(args...)" -# -# the $Declare variable will capture all spaces after the type -# so check it for trailing missing spaces or multiple spaces - if ($line =~ /^.\s*($Declare)\((\s*)\*(\s*)$Ident(\s*)\)(\s*)\(/) { + if ($line =~ /^.\s*($Declare)\((\s*)\*(\s*)($Ident)(\s*)\)(\s*)\(/) { my $declare = $1; my $pre_pointer_space = $2; my $post_pointer_space = $3; @@ -2943,16 +3947,30 @@ sub process { my $post_funcname_space = $5; my $pre_args_space = $6; - if ($declare !~ /\s$/) { +# the $Declare variable will capture all spaces after the type +# so check it for a missing trailing missing space but pointer return types +# don't need a space so don't warn for those. + my $post_declare_space = ""; + if ($declare =~ /(\s+)$/) { + $post_declare_space = $1; + $declare = rtrim($declare); + } + if ($declare !~ /\*$/ && $post_declare_space =~ /^$/) { WARN("SPACING", "missing space after return type\n" . $herecurr); + $post_declare_space = " "; } # unnecessary space "type (*funcptr)(args...)" - elsif ($declare =~ /\s{2,}$/) { - WARN("SPACING", - "Multiple spaces after return type\n" . $herecurr); - } +# This test is not currently implemented because these declarations are +# equivalent to +# int foo(int bar, ...) +# and this is form shouldn't/doesn't generate a checkpatch warning. +# +# elsif ($declare =~ /\s{2,}$/) { +# WARN("SPACING", +# "Multiple spaces after return type\n" . $herecurr); +# } # unnecessary space "type ( *funcptr)(args...)" if (defined $pre_pointer_space && @@ -2983,8 +4001,8 @@ sub process { } if (show_type("SPACING") && $fix) { - $fixed[$linenr - 1] =~ - s/^(.\s*$Declare)\(\s*\*\s*($Ident)\s*\)\s*\(/rtrim($1) . " " . "\(\*$2\)\("/ex; + $fixed[$fixlinenr] =~ + s/^(.\s*)$Declare\s*\(\s*\*\s*$Ident\s*\)\s*\(/$1 . $declare . $post_declare_space . '(*' . $funcname . ')('/ex; } } @@ -3000,7 +4018,7 @@ sub process { if (ERROR("BRACKET_SPACE", "space prohibited before open square bracket '['\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/^(\+.*?)\s+\[/$1\[/; } } @@ -3035,7 +4053,7 @@ sub process { if (WARN("SPACING", "space prohibited between function name and open parenthesis '('\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/\b$name\s+\(/$name\(/; } } @@ -3126,7 +4144,7 @@ sub process { # Ignore operators passed as parameters. if ($op_type ne 'V' && - $ca =~ /\s$/ && $cc =~ /^\s*,/) { + $ca =~ /\s$/ && $cc =~ /^\s*[,\)]/) { # # Ignore comments # } elsif ($op =~ /^$;+$/) { @@ -3145,10 +4163,13 @@ sub process { # // is a comment } elsif ($op eq '//') { + # : when part of a bitfield + } elsif ($opv eq ':B') { + # skip the bitfield test for now + # No spaces for: # -> - # : when part of a bitfield - } elsif ($op eq '->' || $opv eq ':B') { + } elsif ($op eq '->') { if ($ctx =~ /Wx.|.xW/) { if (ERROR("SPACING", "spaces prohibited around that '$op' $at\n" . $hereptr)) { @@ -3160,14 +4181,33 @@ sub process { } } - # , must have a space on the right. + # , must not have a space before and must have a space on the right. } elsif ($op eq ',') { + my $rtrim_before = 0; + my $space_after = 0; + if ($ctx =~ /Wx./) { + if (ERROR("SPACING", + "space prohibited before that '$op' $at\n" . $hereptr)) { + $line_fixed = 1; + $rtrim_before = 1; + } + } if ($ctx !~ /.x[WEC]/ && $cc !~ /^}/) { if (ERROR("SPACING", "space required after that '$op' $at\n" . $hereptr)) { - $good = $fix_elements[$n] . trim($fix_elements[$n + 1]) . " "; $line_fixed = 1; $last_after = $n; + $space_after = 1; + } + } + if ($rtrim_before || $space_after) { + if ($rtrim_before) { + $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); + } else { + $good = $fix_elements[$n] . trim($fix_elements[$n + 1]); + } + if ($space_after) { + $good .= " "; } } @@ -3239,7 +4279,22 @@ sub process { $op eq '*' or $op eq '/' or $op eq '%') { - if ($ctx =~ /Wx[^WCE]|[^WCE]xW/) { + if ($check) { + if (defined $fix_elements[$n + 2] && $ctx !~ /[EW]x[EW]/) { + if (CHK("SPACING", + "spaces preferred around that '$op' $at\n" . $hereptr)) { + $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; + $fix_elements[$n + 2] =~ s/^\s+//; + $line_fixed = 1; + } + } elsif (!defined $fix_elements[$n + 2] && $ctx !~ /Wx[OE]/) { + if (CHK("SPACING", + "space preferred before that '$op' $at\n" . $hereptr)) { + $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]); + $line_fixed = 1; + } + } + } elsif ($ctx =~ /Wx[^WCE]|[^WCE]xW/) { if (ERROR("SPACING", "need consistent spacing around '$op' $at\n" . $hereptr)) { $good = rtrim($fix_elements[$n]) . " " . trim($fix_elements[$n + 1]) . " "; @@ -3274,6 +4329,14 @@ sub process { $ok = 1; } + # for asm volatile statements + # ignore a colon with another + # colon immediately before or after + if (($op eq ':') && + ($ca =~ /:$/ || $cc =~ /^:/)) { + $ok = 1; + } + # messages are ERROR, but ?: are CHK if ($ok == 0) { my $msg_type = \&ERROR; @@ -3300,8 +4363,8 @@ sub process { $fixed_line = $fixed_line . $fix_elements[$#elements]; } - if ($fix && $line_fixed && $fixed_line ne $fixed[$linenr - 1]) { - $fixed[$linenr - 1] = $fixed_line; + if ($fix && $line_fixed && $fixed_line ne $fixed[$fixlinenr]) { + $fixed[$fixlinenr] = $fixed_line; } @@ -3312,7 +4375,7 @@ sub process { if (WARN("SPACING", "space prohibited before semicolon\n" . $herecurr) && $fix) { - 1 while $fixed[$linenr - 1] =~ + 1 while $fixed[$fixlinenr] =~ s/^(\+.*\S)\s+;/$1;/; } } @@ -3340,12 +4403,12 @@ sub process { ## } #need space before brace following if, while, etc - if (($line =~ /\(.*\)\{/ && $line !~ /\($Type\){/) || + if (($line =~ /\(.*\)\{/ && $line !~ /\($Type\)\{/) || $line =~ /do\{/) { if (ERROR("SPACING", "space required before the open brace '{'\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/^(\+.*(?:do|\))){/$1 {/; + $fixed[$fixlinenr] =~ s/^(\+.*(?:do|\)))\{/$1 {/; } } @@ -3363,7 +4426,7 @@ sub process { if (ERROR("SPACING", "space required after that close brace '}'\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/}((?!(?:,|;|\)))\S)/} $1/; } } @@ -3373,7 +4436,7 @@ sub process { if (ERROR("SPACING", "space prohibited after that open square bracket '['\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/\[\s+/\[/; } } @@ -3381,7 +4444,7 @@ sub process { if (ERROR("SPACING", "space prohibited before that close square bracket ']'\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/\s+\]/\]/; } } @@ -3392,7 +4455,7 @@ sub process { if (ERROR("SPACING", "space prohibited after that open parenthesis '('\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/\(\s+/\(/; } } @@ -3402,36 +4465,77 @@ sub process { if (ERROR("SPACING", "space prohibited before that close parenthesis ')'\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/\s+\)/\)/; } } +# check unnecessary parentheses around addressof/dereference single $Lvals +# ie: &(foo->bar) should be &foo->bar and *(foo->bar) should be *foo->bar + + while ($line =~ /(?:[^&]&\s*|\*)\(\s*($Ident\s*(?:$Member\s*)+)\s*\)/g) { + my $var = $1; + if (CHK("UNNECESSARY_PARENTHESES", + "Unnecessary parentheses around $var\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\(\s*\Q$var\E\s*\)/$var/; + } + } + +# check for unnecessary parentheses around function pointer uses +# ie: (foo->bar)(); should be foo->bar(); +# but not "if (foo->bar) (" to avoid some false positives + if ($line =~ /(\bif\s*|)(\(\s*$Ident\s*(?:$Member\s*)+\))[ \t]*\(/ && $1 !~ /^if/) { + my $var = $2; + if (CHK("UNNECESSARY_PARENTHESES", + "Unnecessary parentheses around function pointer $var\n" . $herecurr) && + $fix) { + my $var2 = deparenthesize($var); + $var2 =~ s/\s//g; + $fixed[$fixlinenr] =~ s/\Q$var\E/$var2/; + } + } + #goto labels aren't indented, allow a single space however if ($line=~/^.\s+[A-Za-z\d_]+:(?![0-9]+)/ and !($line=~/^. [A-Za-z\d_]+:/) and !($line=~/^.\s+default:/)) { if (WARN("INDENTED_LABEL", "labels should not be indented\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/^(.)\s+/$1/; } } -# Return is not a function. +# return is not a function if (defined($stat) && $stat =~ /^.\s*return(\s*)\(/s) { my $spacing = $1; if ($^V && $^V ge 5.10.0 && - $stat =~ /^.\s*return\s*$balanced_parens\s*;\s*$/) { - ERROR("RETURN_PARENTHESES", - "return is not a function, parentheses are not required\n" . $herecurr); - + $stat =~ /^.\s*return\s*($balanced_parens)\s*;\s*$/) { + my $value = $1; + $value = deparenthesize($value); + if ($value =~ m/^\s*$FuncArg\s*(?:\?|$)/) { + ERROR("RETURN_PARENTHESES", + "return is not a function, parentheses are not required\n" . $herecurr); + } } elsif ($spacing !~ /\s+/) { ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr); } } +# unnecessary return in a void function +# at end-of-function, with the previous line a single leading tab, then return; +# and the line before that not a goto label target like "out:" + if ($sline =~ /^[ \+]}\s*$/ && + $prevline =~ /^\+\treturn\s*;\s*$/ && + $linenr >= 3 && + $lines[$linenr - 3] =~ /^[ +]/ && + $lines[$linenr - 3] !~ /^[ +]\s*$Ident\s*:/) { + WARN("RETURN_VOID", + "void function return statements are not generally useful\n" . $hereprev); + } + # if statements using unnecessary parentheses - ie: if ((foo == bar)) if ($^V && $^V ge 5.10.0 && $line =~ /\bif\s*((?:\(\s*){2,})/) { @@ -3446,12 +4550,41 @@ sub process { } } -# Return of what appears to be an errno should normally be -'ve - if ($line =~ /^.\s*return\s*(E[A-Z]*)\s*;/) { +# comparisons with a constant or upper case identifier on the left +# avoid cases like "foo + BAR < baz" +# only fix matches surrounded by parentheses to avoid incorrect +# conversions like "FOO < baz() + 5" being "misfixed" to "baz() > FOO + 5" + if ($^V && $^V ge 5.10.0 && + $line =~ /^\+(.*)\b($Constant|[A-Z_][A-Z0-9_]*)\s*($Compare)\s*($LvalOrFunc)/) { + my $lead = $1; + my $const = $2; + my $comp = $3; + my $to = $4; + my $newcomp = $comp; + if ($lead !~ /(?:$Operators|\.)\s*$/ && + $to !~ /^(?:Constant|[A-Z_][A-Z0-9_]*)$/ && + WARN("CONSTANT_COMPARISON", + "Comparisons should place the constant on the right side of the test\n" . $herecurr) && + $fix) { + if ($comp eq "<") { + $newcomp = ">"; + } elsif ($comp eq "<=") { + $newcomp = ">="; + } elsif ($comp eq ">") { + $newcomp = "<"; + } elsif ($comp eq ">=") { + $newcomp = "<="; + } + $fixed[$fixlinenr] =~ s/\(\s*\Q$const\E\s*$Compare\s*\Q$to\E\s*\)/($to $newcomp $const)/; + } + } + +# Return of what appears to be an errno should normally be negative + if ($sline =~ /\breturn(?:\s*\(+\s*|\s+)(E[A-Z]+)(?:\s*\)+\s*|\s*)[;:,]/) { my $name = $1; if ($name ne 'EOF' && $name ne 'ERROR') { WARN("USE_NEGATIVE_ERRNO", - "return of an errno should typically be -ve (return -$1)\n" . $herecurr); + "return of an errno should typically be negative (ie: return -$1)\n" . $herecurr); } } @@ -3460,7 +4593,7 @@ sub process { if (ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/\b(if|while|for|switch)\(/$1 \(/; } } @@ -3550,7 +4683,7 @@ sub process { # if should not continue a brace if ($line =~ /}\s*if\b/) { ERROR("TRAILING_STATEMENTS", - "trailing statements should be on next line\n" . + "trailing statements should be on next line (or did you mean 'else if'?)\n" . $herecurr); } # case and default should not have general statements after them @@ -3566,14 +4699,26 @@ sub process { # Check for }<nl>else {, these must be at the same # indent level to be relevant to each other. - if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ and - $previndent == $indent) { - ERROR("ELSE_AFTER_BRACE", - "else should follow close brace '}'\n" . $hereprev); + if ($prevline=~/}\s*$/ and $line=~/^.\s*else\s*/ && + $previndent == $indent) { + if (ERROR("ELSE_AFTER_BRACE", + "else should follow close brace '}'\n" . $hereprev) && + $fix && $prevline =~ /^\+/ && $line =~ /^\+/) { + fix_delete_line($fixlinenr - 1, $prevrawline); + fix_delete_line($fixlinenr, $rawline); + my $fixedline = $prevrawline; + $fixedline =~ s/}\s*$//; + if ($fixedline !~ /^\+\s*$/) { + fix_insert_line($fixlinenr, $fixedline); + } + $fixedline = $rawline; + $fixedline =~ s/^(.\s*)else/$1} else/; + fix_insert_line($fixlinenr, $fixedline); + } } - if ($prevline=~/}\s*$/ and $line=~/^.\s*while\s*/ and - $previndent == $indent) { + if ($prevline=~/}\s*$/ and $line=~/^.\s*while\s*/ && + $previndent == $indent) { my ($s, $c) = ctx_statement_block($linenr, $realcnt, 0); # Find out what is on the end of the line after the @@ -3582,8 +4727,18 @@ sub process { $s =~ s/\n.*//g; if ($s =~ /^\s*;/) { - ERROR("WHILE_AFTER_BRACE", - "while should follow close brace '}'\n" . $hereprev); + if (ERROR("WHILE_AFTER_BRACE", + "while should follow close brace '}'\n" . $hereprev) && + $fix && $prevline =~ /^\+/ && $line =~ /^\+/) { + fix_delete_line($fixlinenr - 1, $prevrawline); + fix_delete_line($fixlinenr, $rawline); + my $fixedline = $prevrawline; + my $trailing = $rawline; + $trailing =~ s/^\+//; + $trailing = trim($trailing); + $fixedline =~ s/}\s*$/} $trailing/; + fix_insert_line($fixlinenr, $fixedline); + } } } @@ -3597,7 +4752,7 @@ sub process { "Avoid gcc v4.3+ binary constant extension: <$var>\n" . $herecurr) && $fix) { my $hexval = sprintf("0x%x", oct($var)); - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/\b$var\b/$hexval/; } } @@ -3608,7 +4763,9 @@ sub process { #Ignore Page<foo> variants $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && #Ignore SI style variants like nS, mV and dB (ie: max_uV, regulator_min_uA_show) - $var !~ /^(?:[a-z_]*?)_?[a-z][A-Z](?:_[a-z_]+)?$/) { + $var !~ /^(?:[a-z_]*?)_?[a-z][A-Z](?:_[a-z_]+)?$/ && +#Ignore some three character SI units explicitly, like MiB and KHz + $var !~ /^(?:[a-z_]*?)_?(?:[KMGT]iB|[KMGT]?Hz)(?:_[a-z_]+)?$/) { while ($var =~ m{($Ident)}g) { my $word = $1; next if ($word !~ /[A-Z][a-z]|[a-z][A-Z]/); @@ -3633,11 +4790,12 @@ sub process { if (WARN("WHITESPACE_AFTER_LINE_CONTINUATION", "Whitespace after \\ makes next lines useless\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/\s+$//; + $fixed[$fixlinenr] =~ s/\s+$//; } } -#warn if <asm/foo.h> is #included and <linux/foo.h> is available (uses RAW line) +# warn if <asm/foo.h> is #included and <linux/foo.h> is available and includes +# itself <asm/foo.h> (uses RAW line) if ($tree && $rawline =~ m{^.\s*\#\s*include\s*\<asm\/(.*)\.h\>}) { my $file = "$1.h"; my $checkfile = "include/linux/$file"; @@ -3645,12 +4803,15 @@ sub process { $realfile ne $checkfile && $1 !~ /$allowed_asm_includes/) { - if ($realfile =~ m{^arch/}) { - CHK("ARCH_INCLUDE_LINUX", - "Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr); - } else { - WARN("INCLUDE_LINUX", - "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr); + my $asminclude = `grep -Ec "#include\\s+<asm/$file>" $root/$checkfile`; + if ($asminclude > 0) { + if ($realfile =~ m{^arch/}) { + CHK("ARCH_INCLUDE_LINUX", + "Consider using #include <linux/$file> instead of <asm/$file>\n" . $herecurr); + } else { + WARN("INCLUDE_LINUX", + "Use #include <linux/$file> instead of <asm/$file>\n" . $herecurr); + } } } } @@ -3664,13 +4825,28 @@ sub process { my $cnt = $realcnt; my ($off, $dstat, $dcond, $rest); my $ctx = ''; + my $has_flow_statement = 0; + my $has_arg_concat = 0; ($dstat, $dcond, $ln, $cnt, $off) = ctx_statement_block($linenr, $realcnt, 0); $ctx = $dstat; #print "dstat<$dstat> dcond<$dcond> cnt<$cnt> off<$off>\n"; #print "LINE<$lines[$ln-1]> len<" . length($lines[$ln-1]) . "\n"; - $dstat =~ s/^.\s*\#\s*define\s+$Ident(?:\([^\)]*\))?\s*//; + $has_flow_statement = 1 if ($ctx =~ /\b(goto|return)\b/); + $has_arg_concat = 1 if ($ctx =~ /\#\#/ && $ctx !~ /\#\#\s*(?:__VA_ARGS__|args)\b/); + + $dstat =~ s/^.\s*\#\s*define\s+$Ident(\([^\)]*\))?\s*//; + my $define_args = $1; + my $define_stmt = $dstat; + my @def_args = (); + + if (defined $define_args && $define_args ne "") { + $define_args = substr($define_args, 1, length($define_args) - 2); + $define_args =~ s/\s*//g; + @def_args = split(",", $define_args); + } + $dstat =~ s/$;//g; $dstat =~ s/\\\n.//g; $dstat =~ s/^\s*//s; @@ -3679,16 +4855,19 @@ sub process { # Flatten any parentheses and braces while ($dstat =~ s/\([^\(\)]*\)/1/ || $dstat =~ s/\{[^\{\}]*\}/1/ || - $dstat =~ s/\[[^\[\]]*\]/1/) + $dstat =~ s/.\[[^\[\]]*\]/1/) { } # Flatten any obvious string concatentation. - while ($dstat =~ s/("X*")\s*$Ident/$1/ || - $dstat =~ s/$Ident\s*("X*")/$1/) + while ($dstat =~ s/($String)\s*$Ident/$1/ || + $dstat =~ s/$Ident\s*($String)/$1/) { } + # Make asm volatile uses seem like a generic function + $dstat =~ s/\b_*asm_*\s+_*volatile_*\b/asm_volatile/g; + my $exceptions = qr{ $Declare| module_param_named| @@ -3699,14 +4878,24 @@ sub process { union| struct| \.$Ident\s*=\s*| - ^\"|\"$ + ^\"|\"$| + ^\[ }x; #print "REST<$rest> dstat<$dstat> ctx<$ctx>\n"; + + $ctx =~ s/\n*$//; + my $herectx = $here . "\n"; + my $stmt_cnt = statement_rawlines($ctx); + + for (my $n = 0; $n < $stmt_cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + } + if ($dstat ne '' && $dstat !~ /^(?:$Ident|-?$Constant),$/ && # 10, // foo(), $dstat !~ /^(?:$Ident|-?$Constant);$/ && # foo(); $dstat !~ /^[!~-]?(?:$Lval|$Constant)$/ && # 10 // foo() // !foo // ~foo // -foo // foo->bar // foo.bar->baz - $dstat !~ /^'X'$/ && # character constants + $dstat !~ /^'X'$/ && $dstat !~ /^'XX'$/ && # character constants $dstat !~ /$exceptions/ && $dstat !~ /^\.$Ident\s*=/ && # .foo = $dstat !~ /^(?:\#\s*$Ident|\#\s*$Constant)\s*$/ && # stringification #foo @@ -3717,21 +4906,69 @@ sub process { $dstat !~ /^\(\{/ && # ({... $ctx !~ /^.\s*#\s*define\s+TRACE_(?:SYSTEM|INCLUDE_FILE|INCLUDE_PATH)\b/) { - $ctx =~ s/\n*$//; + if ($dstat =~ /^\s*if\b/) { + ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", + "Macros starting with if should be enclosed by a do - while loop to avoid possible if/else logic defects\n" . "$herectx"); + } elsif ($dstat =~ /;/) { + ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", + "Macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx"); + } else { + ERROR("COMPLEX_MACRO", + "Macros with complex values should be enclosed in parentheses\n" . "$herectx"); + } + + } + + # Make $define_stmt single line, comment-free, etc + my @stmt_array = split('\n', $define_stmt); + my $first = 1; + $define_stmt = ""; + foreach my $l (@stmt_array) { + $l =~ s/\\$//; + if ($first) { + $define_stmt = $l; + $first = 0; + } elsif ($l =~ /^[\+ ]/) { + $define_stmt .= substr($l, 1); + } + } + $define_stmt =~ s/$;//g; + $define_stmt =~ s/\s+/ /g; + $define_stmt = trim($define_stmt); + +# check if any macro arguments are reused (ignore '...' and 'type') + foreach my $arg (@def_args) { + next if ($arg =~ /\.\.\./); + next if ($arg =~ /^type$/i); + my $tmp_stmt = $define_stmt; + $tmp_stmt =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*\(\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*\)*\b//g; + $tmp_stmt =~ s/\#+\s*$arg\b//g; + $tmp_stmt =~ s/\b$arg\s*\#\#//g; + my $use_cnt = $tmp_stmt =~ s/\b$arg\b//g; + if ($use_cnt > 1) { + CHK("MACRO_ARG_REUSE", + "Macro argument reuse '$arg' - possible side-effects?\n" . "$herectx"); + } +# check if any macro arguments may have other precedence issues + if ($tmp_stmt =~ m/($Operators)?\s*\b$arg\b\s*($Operators)?/m && + ((defined($1) && $1 ne ',') || + (defined($2) && $2 ne ','))) { + CHK("MACRO_ARG_PRECEDENCE", + "Macro argument '$arg' may be better as '($arg)' to avoid precedence issues\n" . "$herectx"); + } + } + +# check for macros with flow control, but without ## concatenation +# ## concatenation is commonly a macro that defines a function so ignore those + if ($has_flow_statement && !$has_arg_concat) { my $herectx = $here . "\n"; my $cnt = statement_rawlines($ctx); for (my $n = 0; $n < $cnt; $n++) { $herectx .= raw_line($linenr, $n) . "\n"; } - - if ($dstat =~ /;/) { - ERROR("MULTISTATEMENT_MACRO_USE_DO_WHILE", - "Macros with multiple statements should be enclosed in a do - while loop\n" . "$herectx"); - } else { - ERROR("COMPLEX_MACRO", - "Macros with complex values should be enclosed in parenthesis\n" . "$herectx"); - } + WARN("MACRO_WITH_FLOW_CONTROL", + "Macros with flow control statements should be avoided\n" . "$herectx"); } # check for line continuations outside of #defines, preprocessor #, and asm @@ -3761,6 +4998,7 @@ sub process { $ctx = $dstat; $dstat =~ s/\\\n.//g; + $dstat =~ s/$;/ /g; if ($dstat =~ /^\+\s*#\s*define\s+$Ident\s*${balanced_parens}\s*do\s*{(.*)\s*}\s*while\s*\(\s*0\s*\)\s*([;\s]*)\s*$/) { my $stmts = $2; @@ -3783,6 +5021,17 @@ sub process { WARN("DO_WHILE_MACRO_WITH_TRAILING_SEMICOLON", "do {} while (0) macros should not be semicolon terminated\n" . "$herectx"); } + } elsif ($dstat =~ /^\+\s*#\s*define\s+$Ident.*;\s*$/) { + $ctx =~ s/\n*$//; + my $cnt = statement_rawlines($ctx); + my $herectx = $here . "\n"; + + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + } + + WARN("TRAILING_SEMICOLON", + "macros should not use a trailing semicolon\n" . "$herectx"); } } @@ -3914,21 +5163,137 @@ sub process { } } +# check for single line unbalanced braces + if ($sline =~ /^.\s*\}\s*else\s*$/ || + $sline =~ /^.\s*else\s*\{\s*$/) { + CHK("BRACES", "Unbalanced braces around else statement\n" . $herecurr); + } + # check for unnecessary blank lines around braces if (($line =~ /^.\s*}\s*$/ && $prevrawline =~ /^.\s*$/)) { - CHK("BRACES", - "Blank lines aren't necessary before a close brace '}'\n" . $hereprev); + if (CHK("BRACES", + "Blank lines aren't necessary before a close brace '}'\n" . $hereprev) && + $fix && $prevrawline =~ /^\+/) { + fix_delete_line($fixlinenr - 1, $prevrawline); + } } if (($rawline =~ /^.\s*$/ && $prevline =~ /^..*{\s*$/)) { - CHK("BRACES", - "Blank lines aren't necessary after an open brace '{'\n" . $hereprev); + if (CHK("BRACES", + "Blank lines aren't necessary after an open brace '{'\n" . $hereprev) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } } # no volatiles please my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b}; if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) { WARN("VOLATILE", - "Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr); + "Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst\n" . $herecurr); + } + +# Check for user-visible strings broken across lines, which breaks the ability +# to grep for the string. Make exceptions when the previous string ends in a +# newline (multiple lines in one string constant) or '\t', '\r', ';', or '{' +# (common in inline assembly) or is a octal \123 or hexadecimal \xaf value + if ($line =~ /^\+\s*$String/ && + $prevline =~ /"\s*$/ && + $prevrawline !~ /(?:\\(?:[ntr]|[0-7]{1,3}|x[0-9a-fA-F]{1,2})|;\s*|\{\s*)"\s*$/) { + if (WARN("SPLIT_STRING", + "quoted string split across lines\n" . $hereprev) && + $fix && + $prevrawline =~ /^\+.*"\s*$/ && + $last_coalesced_string_linenr != $linenr - 1) { + my $extracted_string = get_quoted_string($line, $rawline); + my $comma_close = ""; + if ($rawline =~ /\Q$extracted_string\E(\s*\)\s*;\s*$|\s*,\s*)/) { + $comma_close = $1; + } + + fix_delete_line($fixlinenr - 1, $prevrawline); + fix_delete_line($fixlinenr, $rawline); + my $fixedline = $prevrawline; + $fixedline =~ s/"\s*$//; + $fixedline .= substr($extracted_string, 1) . trim($comma_close); + fix_insert_line($fixlinenr - 1, $fixedline); + $fixedline = $rawline; + $fixedline =~ s/\Q$extracted_string\E\Q$comma_close\E//; + if ($fixedline !~ /\+\s*$/) { + fix_insert_line($fixlinenr, $fixedline); + } + $last_coalesced_string_linenr = $linenr; + } + } + +# check for missing a space in a string concatenation + if ($prevrawline =~ /[^\\]\w"$/ && $rawline =~ /^\+[\t ]+"\w/) { + WARN('MISSING_SPACE', + "break quoted strings at a space character\n" . $hereprev); + } + +# check for an embedded function name in a string when the function is known +# This does not work very well for -f --file checking as it depends on patch +# context providing the function name or a single line form for in-file +# function declarations + if ($line =~ /^\+.*$String/ && + defined($context_function) && + get_quoted_string($line, $rawline) =~ /\b$context_function\b/ && + length(get_quoted_string($line, $rawline)) != (length($context_function) + 2)) { + WARN("EMBEDDED_FUNCTION_NAME", + "Prefer using '\"%s...\", __func__' to using '$context_function', this function's name, in a string\n" . $herecurr); + } + +# check for spaces before a quoted newline + if ($rawline =~ /^.*\".*\s\\n/) { + if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", + "unnecessary whitespace before a quoted newline\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/^(\+.*\".*)\s+\\n/$1\\n/; + } + + } + +# concatenated string without spaces between elements + if ($line =~ /$String[A-Z_]/ || $line =~ /[A-Za-z0-9_]$String/) { + CHK("CONCATENATED_STRING", + "Concatenated strings should use spaces between elements\n" . $herecurr); + } + +# uncoalesced string fragments + if ($line =~ /$String\s*"/) { + WARN("STRING_FRAGMENTS", + "Consecutive strings are generally better as a single string\n" . $herecurr); + } + +# check for non-standard and hex prefixed decimal printf formats + my $show_L = 1; #don't show the same defect twice + my $show_Z = 1; + while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { + my $string = substr($rawline, $-[1], $+[1] - $-[1]); + $string =~ s/%%/__/g; + # check for %L + if ($show_L && $string =~ /%[\*\d\.\$]*L([diouxX])/) { + WARN("PRINTF_L", + "\%L$1 is non-standard C, use %ll$1\n" . $herecurr); + $show_L = 0; + } + # check for %Z + if ($show_Z && $string =~ /%[\*\d\.\$]*Z([diouxX])/) { + WARN("PRINTF_Z", + "%Z$1 is non-standard C, use %z$1\n" . $herecurr); + $show_Z = 0; + } + # check for 0x<decimal> + if ($string =~ /0x%[\*\d\.\$\Llzth]*[diou]/) { + ERROR("PRINTF_0XDECIMAL", + "Prefixing 0x with decimal output is defective\n" . $herecurr); + } + } + +# check for line continuations in quoted strings with odd counts of " + if ($rawline =~ /\\$/ && $rawline =~ tr/"/"/ % 2) { + WARN("LINE_CONTINUATIONS", + "Avoid line continuations in quoted strings\n" . $herecurr); } # warn about #if 0 @@ -3940,10 +5305,90 @@ sub process { # check for needless "if (<foo>) fn(<foo>)" uses if ($prevline =~ /\bif\s*\(\s*($Lval)\s*\)/) { - my $expr = '\s*\(\s*' . quotemeta($1) . '\s*\)\s*;'; - if ($line =~ /\b(kfree|usb_free_urb|debugfs_remove(?:_recursive)?)$expr/) { - WARN('NEEDLESS_IF', - "$1(NULL) is safe this check is probably not required\n" . $hereprev); + my $tested = quotemeta($1); + my $expr = '\s*\(\s*' . $tested . '\s*\)\s*;'; + if ($line =~ /\b(kfree|usb_free_urb|debugfs_remove(?:_recursive)?|(?:kmem_cache|mempool|dma_pool)_destroy)$expr/) { + my $func = $1; + if (WARN('NEEDLESS_IF', + "$func(NULL) is safe and this check is probably not required\n" . $hereprev) && + $fix) { + my $do_fix = 1; + my $leading_tabs = ""; + my $new_leading_tabs = ""; + if ($lines[$linenr - 2] =~ /^\+(\t*)if\s*\(\s*$tested\s*\)\s*$/) { + $leading_tabs = $1; + } else { + $do_fix = 0; + } + if ($lines[$linenr - 1] =~ /^\+(\t+)$func\s*\(\s*$tested\s*\)\s*;\s*$/) { + $new_leading_tabs = $1; + if (length($leading_tabs) + 1 ne length($new_leading_tabs)) { + $do_fix = 0; + } + } else { + $do_fix = 0; + } + if ($do_fix) { + fix_delete_line($fixlinenr - 1, $prevrawline); + $fixed[$fixlinenr] =~ s/^\+$new_leading_tabs/\+$leading_tabs/; + } + } + } + } + +# check for unnecessary "Out of Memory" messages + if ($line =~ /^\+.*\b$logFunctions\s*\(/ && + $prevline =~ /^[ \+]\s*if\s*\(\s*(\!\s*|NULL\s*==\s*)?($Lval)(\s*==\s*NULL\s*)?\s*\)/ && + (defined $1 || defined $3) && + $linenr > 3) { + my $testval = $2; + my $testline = $lines[$linenr - 3]; + + my ($s, $c) = ctx_statement_block($linenr - 3, $realcnt, 0); +# print("line: <$line>\nprevline: <$prevline>\ns: <$s>\nc: <$c>\n\n\n"); + + if ($s =~ /(?:^|\n)[ \+]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:\([^\)]*\)\s*)?\s*(?:devm_)?(?:[kv][czm]alloc(?:_node|_array)?\b|kstrdup|kmemdup|(?:dev_)?alloc_skb)/) { + WARN("OOM_MESSAGE", + "Possible unnecessary 'out of memory' message\n" . $hereprev); + } + } + +# check for logging functions with KERN_<LEVEL> + if ($line !~ /printk(?:_ratelimited|_once)?\s*\(/ && + $line =~ /\b$logFunctions\s*\(.*\b(KERN_[A-Z]+)\b/) { + my $level = $1; + if (WARN("UNNECESSARY_KERN_LEVEL", + "Possible unnecessary $level\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\s*$level\s*//; + } + } + +# check for logging continuations + if ($line =~ /\bprintk\s*\(\s*KERN_CONT\b|\bpr_cont\s*\(/) { + WARN("LOGGING_CONTINUATION", + "Avoid logging continuation uses where feasible\n" . $herecurr); + } + +# check for mask then right shift without a parentheses + if ($^V && $^V ge 5.10.0 && + $line =~ /$LvalOrFunc\s*\&\s*($LvalOrFunc)\s*>>/ && + $4 !~ /^\&/) { # $LvalOrFunc may be &foo, ignore if so + WARN("MASK_THEN_SHIFT", + "Possible precedence defect with mask then right shift - may need parentheses\n" . $herecurr); + } + +# check for pointer comparisons to NULL + if ($^V && $^V ge 5.10.0) { + while ($line =~ /\b$LvalOrFunc\s*(==|\!=)\s*NULL\b/g) { + my $val = $1; + my $equal = "!"; + $equal = "" if ($4 eq "!="); + if (CHK("COMPARISON_TO_NULL", + "Comparison to NULL could be written \"${equal}${val}\"\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b\Q$val\E\s*(?:==|\!=)\s*NULL\b/$equal$val/; + } } } @@ -3960,7 +5405,7 @@ sub process { WARN("MISPLACED_INIT", "$attr should be placed after $var\n" . $herecurr))) && $fix) { - $fixed[$linenr - 1] =~ s/(\bstatic\s+(?:const\s+)?)(?:$attr\s+)?($NonptrTypeWithAttr)\s+(?:$attr\s+)?($Ident(?:\[[^]]*\])?)\s*([=;])\s*/"$1" . trim(string_find_replace($2, "\\s*$attr\\s*", " ")) . " " . trim(string_find_replace($3, "\\s*$attr\\s*", "")) . " $attr" . ("$4" eq ";" ? ";" : " = ")/e; + $fixed[$fixlinenr] =~ s/(\bstatic\s+(?:const\s+)?)(?:$attr\s+)?($NonptrTypeWithAttr)\s+(?:$attr\s+)?($Ident(?:\[[^]]*\])?)\s*([=;])\s*/"$1" . trim(string_find_replace($2, "\\s*$attr\\s*", " ")) . " " . trim(string_find_replace($3, "\\s*$attr\\s*", "")) . " $attr" . ("$4" eq ";" ? ";" : " = ")/e; } } } @@ -3974,7 +5419,7 @@ sub process { if (ERROR("INIT_ATTRIBUTE", "Use of const init definition must use ${attr_prefix}initconst\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/$InitAttributeData/${attr_prefix}initconst/; } } @@ -3985,21 +5430,49 @@ sub process { if (ERROR("INIT_ATTRIBUTE", "Use of $attr requires a separate use of const\n" . $herecurr) && $fix) { - my $lead = $fixed[$linenr - 1] =~ + my $lead = $fixed[$fixlinenr] =~ /(^\+\s*(?:static\s+))/; $lead = rtrim($1); $lead = "$lead " if ($lead !~ /^\+$/); $lead = "${lead}const "; - $fixed[$linenr - 1] =~ s/(^\+\s*(?:static\s+))/$lead/; + $fixed[$fixlinenr] =~ s/(^\+\s*(?:static\s+))/$lead/; + } + } + +# check for __read_mostly with const non-pointer (should just be const) + if ($line =~ /\b__read_mostly\b/ && + $line =~ /($Type)\s*$Ident/ && $1 !~ /\*\s*$/ && $1 =~ /\bconst\b/) { + if (ERROR("CONST_READ_MOSTLY", + "Invalid use of __read_mostly with const type\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\s+__read_mostly\b//; + } + } + +# don't use __constant_<foo> functions outside of include/uapi/ + if ($realfile !~ m@^include/uapi/@ && + $line =~ /(__constant_(?:htons|ntohs|[bl]e(?:16|32|64)_to_cpu|cpu_to_[bl]e(?:16|32|64)))\s*\(/) { + my $constant_func = $1; + my $func = $constant_func; + $func =~ s/^__constant_//; + if (WARN("CONSTANT_CONVERSION", + "$constant_func should be $func\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b$constant_func\b/$func/g; } } # prefer usleep_range over udelay if ($line =~ /\budelay\s*\(\s*(\d+)\s*\)/) { + my $delay = $1; # ignore udelay's < 10, however - if (! ($1 < 10) ) { + if (! ($delay < 10) ) { CHK("USLEEP_RANGE", - "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $line); + "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $herecurr); + } + if ($delay > 2000) { + WARN("LONG_UDELAY", + "long udelay - prefer mdelay; see arch/arm/include/asm/delay.h\n" . $herecurr); } } @@ -4007,7 +5480,7 @@ sub process { if ($line =~ /\bmsleep\s*\((\d+)\);/) { if ($1 < 20) { WARN("MSLEEP", - "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $line); + "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $herecurr); } } @@ -4035,7 +5508,7 @@ sub process { if (ERROR("SPACING", "exactly one space required after that #$1\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ + $fixed[$fixlinenr] =~ s/^(.\s*\#\s*(ifdef|ifndef|elif))\s{2,}/$1 /; } @@ -4051,22 +5524,70 @@ sub process { } } # check for memory barriers without a comment. - if ($line =~ /\b(mb|rmb|wmb|read_barrier_depends|smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) { + + my $barriers = qr{ + mb| + rmb| + wmb| + read_barrier_depends + }x; + my $barrier_stems = qr{ + mb__before_atomic| + mb__after_atomic| + store_release| + load_acquire| + store_mb| + (?:$barriers) + }x; + my $all_barriers = qr{ + (?:$barriers)| + smp_(?:$barrier_stems)| + virt_(?:$barrier_stems) + }x; + + if ($line =~ /\b(?:$all_barriers)\s*\(/) { if (!ctx_has_comment($first_line, $linenr)) { WARN("MEMORY_BARRIER", "memory barrier without comment\n" . $herecurr); } } + + my $underscore_smp_barriers = qr{__smp_(?:$barrier_stems)}x; + + if ($realfile !~ m@^include/asm-generic/@ && + $realfile !~ m@/barrier\.h$@ && + $line =~ m/\b(?:$underscore_smp_barriers)\s*\(/ && + $line !~ m/^.\s*\#\s*define\s+(?:$underscore_smp_barriers)\s*\(/) { + WARN("MEMORY_BARRIER", + "__smp memory barriers shouldn't be used outside barrier.h and asm-generic\n" . $herecurr); + } + +# check for waitqueue_active without a comment. + if ($line =~ /\bwaitqueue_active\s*\(/) { + if (!ctx_has_comment($first_line, $linenr)) { + WARN("WAITQUEUE_ACTIVE", + "waitqueue_active without comment\n" . $herecurr); + } + } + # check of hardware specific defines if ($line =~ m@^.\s*\#\s*if.*\b(__i386__|__powerpc64__|__sun__|__s390x__)\b@ && $realfile !~ m@include/asm-@) { CHK("ARCH_DEFINES", "architecture specific defines should be avoided\n" . $herecurr); } +# check that the storage class is not after a type + if ($line =~ /\b($Type)\s+($Storage)\b/) { + WARN("STORAGE_CLASS", + "storage class '$2' should be located before type '$1'\n" . $herecurr); + } # Check that the storage class is at the beginning of a declaration - if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) { + if ($line =~ /\b$Storage\b/ && + $line !~ /^.\s*$Storage/ && + $line =~ /^.\s*(.+?)\$Storage\s/ && + $1 !~ /[\,\)]\s*$/) { WARN("STORAGE_CLASS", - "storage class should be at the beginning of the declaration\n" . $herecurr) + "storage class should be at the beginning of the declaration\n" . $herecurr); } # check the location of the inline attribute, that it is between @@ -4083,7 +5604,7 @@ sub process { if (WARN("INLINE", "plain inline is preferred over $1\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/\b(__inline__|__inline)\b/inline/; + $fixed[$fixlinenr] =~ s/\b(__inline__|__inline)\b/inline/; } } @@ -4094,8 +5615,10 @@ sub process { WARN("PREFER_PACKED", "__packed is preferred over __attribute__((packed))\n" . $herecurr); } + # Check for new packed members, warn to use care - if ($line =~ /\b(__attribute__\s*\(\s*\(.*\bpacked|__packed)\b/) { + if ($realfile !~ m@\binclude/uapi/@ && + $line =~ /\b(__attribute__\s*\(\s*\(.*\bpacked|__packed)\b/) { WARN("NEW_PACKED", "Adding new packed members is to be done with care\n" . $herecurr); } @@ -4113,7 +5636,7 @@ sub process { if (WARN("PREFER_PRINTF", "__printf(string-index, first-to-check) is preferred over __attribute__((format(printf, string-index, first-to-check)))\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.*)\)\s*\)\s*\)/"__printf(" . trim($1) . ")"/ex; + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.*)\)\s*\)\s*\)/"__printf(" . trim($1) . ")"/ex; } } @@ -4124,7 +5647,55 @@ sub process { if (WARN("PREFER_SCANF", "__scanf(string-index, first-to-check) is preferred over __attribute__((format(scanf, string-index, first-to-check)))\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)\)\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex; + $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)\)\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex; + } + } + +# Check for __attribute__ weak, or __weak declarations (may have link issues) + if ($^V && $^V ge 5.10.0 && + $line =~ /(?:$Declare|$DeclareMisordered)\s*$Ident\s*$balanced_parens\s*(?:$Attribute)?\s*;/ && + ($line =~ /\b__attribute__\s*\(\s*\(.*\bweak\b/ || + $line =~ /\b__weak\b/)) { + ERROR("WEAK_DECLARATION", + "Using weak declarations can have unintended link defects\n" . $herecurr); + } + +# check for c99 types like uint8_t used outside of uapi/ and tools/ + if ($realfile !~ m@\binclude/uapi/@ && + $realfile !~ m@\btools/@ && + $line =~ /\b($Declare)\s*$Ident\s*[=;,\[]/) { + my $type = $1; + if ($type =~ /\b($typeC99Typedefs)\b/) { + $type = $1; + my $kernel_type = 'u'; + $kernel_type = 's' if ($type =~ /^_*[si]/); + $type =~ /(\d+)/; + $kernel_type .= $1; + if (CHK("PREFER_KERNEL_TYPES", + "Prefer kernel type '$kernel_type' over '$type'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b$type\b/$kernel_type/; + } + } + } + +# check for cast of C90 native int or longer types constants + if ($line =~ /(\(\s*$C90_int_types\s*\)\s*)($Constant)\b/) { + my $cast = $1; + my $const = $2; + if (WARN("TYPECAST_INT_CONSTANT", + "Unnecessary typecast of c90 int constant\n" . $herecurr) && + $fix) { + my $suffix = ""; + my $newconst = $const; + $newconst =~ s/${Int_type}$//; + $suffix .= 'U' if ($cast =~ /\bunsigned\b/); + if ($cast =~ /\blong\s+long\b/) { + $suffix .= 'LL'; + } elsif ($cast =~ /\blong\b/) { + $suffix .= 'L'; + } + $fixed[$fixlinenr] =~ s/\Q$cast\E$const\b/$newconst$suffix/; } } @@ -4139,16 +5710,10 @@ sub process { if (WARN("SIZEOF_PARENTHESIS", "sizeof $1 should be sizeof($1)\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/\bsizeof\s+((?:\*\s*|)$Lval|$Type(?:\s+$Lval|))/"sizeof(" . trim($1) . ")"/ex; + $fixed[$fixlinenr] =~ s/\bsizeof\s+((?:\*\s*|)$Lval|$Type(?:\s+$Lval|))/"sizeof(" . trim($1) . ")"/ex; } } -# check for line continuations in quoted strings with odd counts of " - if ($rawline =~ /\\$/ && $rawline =~ tr/"/"/ % 2) { - WARN("LINE_CONTINUATIONS", - "Avoid line continuations in quoted strings\n" . $herecurr); - } - # check for struct spinlock declarations if ($line =~ /^.\s*\bstruct\s+spinlock\s+\w+\s*;/) { WARN("USE_SPINLOCK_T", @@ -4158,19 +5723,46 @@ sub process { # check for seq_printf uses that could be seq_puts if ($sline =~ /\bseq_printf\s*\(.*"\s*\)\s*;\s*$/) { my $fmt = get_quoted_string($line, $rawline); - if ($fmt ne "" && $fmt !~ /[^\\]\%/) { + $fmt =~ s/%%//g; + if ($fmt !~ /%/) { if (WARN("PREFER_SEQ_PUTS", "Prefer seq_puts to seq_printf\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/\bseq_printf\b/seq_puts/; + $fixed[$fixlinenr] =~ s/\bseq_printf\b/seq_puts/; } } } + # check for vsprintf extension %p<foo> misuses + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^\+(?![^\{]*\{\s*).*\b(\w+)\s*\(.*$String\s*,/s && + $1 !~ /^_*volatile_*$/) { + my $bad_extension = ""; + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + for (my $count = $linenr; $count <= $lc; $count++) { + my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); + $fmt =~ s/%%//g; + if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { + $bad_extension = $1; + last; + } + } + if ($bad_extension ne "") { + my $stat_real = raw_line($linenr, 0); + for (my $count = $linenr + 1; $count <= $lc; $count++) { + $stat_real = $stat_real . "\n" . raw_line($count, 0); + } + WARN("VSPRINTF_POINTER_EXTENSION", + "Invalid vsprintf pointer extension '$bad_extension'\n" . "$here\n$stat_real\n"); + } + } + # Check for misused memsets if ($^V && $^V ge 5.10.0 && defined $stat && - $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/s) { + $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*$FuncArg\s*\)/) { my $ms_addr = $2; my $ms_val = $7; @@ -4186,14 +5778,46 @@ sub process { } # Check for memcpy(foo, bar, ETH_ALEN) that could be ether_addr_copy(foo, bar) - if ($^V && $^V ge 5.10.0 && - $line =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/s) { - if (WARN("PREFER_ETHER_ADDR_COPY", - "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . $herecurr) && - $fix) { - $fixed[$linenr - 1] =~ s/\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/ether_addr_copy($2, $7)/; - } - } +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# if (WARN("PREFER_ETHER_ADDR_COPY", +# "Prefer ether_addr_copy() over memcpy() if the Ethernet addresses are __aligned(2)\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemcpy\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/ether_addr_copy($2, $7)/; +# } +# } + +# Check for memcmp(foo, bar, ETH_ALEN) that could be ether_addr_equal*(foo, bar) +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemcmp\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# WARN("PREFER_ETHER_ADDR_EQUAL", +# "Prefer ether_addr_equal() or ether_addr_equal_unaligned() over memcmp()\n" . "$here\n$stat\n") +# } + +# check for memset(foo, 0x0, ETH_ALEN) that could be eth_zero_addr +# check for memset(foo, 0xFF, ETH_ALEN) that could be eth_broadcast_addr +# if ($^V && $^V ge 5.10.0 && +# defined $stat && +# $stat =~ /^\+(?:.*?)\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*\,\s*ETH_ALEN\s*\)/) { +# +# my $ms_val = $7; +# +# if ($ms_val =~ /^(?:0x|)0+$/i) { +# if (WARN("PREFER_ETH_ZERO_ADDR", +# "Prefer eth_zero_addr over memset()\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_zero_addr($2)/; +# } +# } elsif ($ms_val =~ /^(?:0xff|255)$/i) { +# if (WARN("PREFER_ETH_BROADCAST_ADDR", +# "Prefer eth_broadcast_addr() over memset()\n" . "$here\n$stat\n") && +# $fix) { +# $fixed[$fixlinenr] =~ s/\bmemset\s*\(\s*$FuncArg\s*,\s*$FuncArg\s*,\s*ETH_ALEN\s*\)/eth_broadcast_addr($2)/; +# } +# } +# } # typecasts on min/max could be min_t/max_t if ($^V && $^V ge 5.10.0 && @@ -4238,7 +5862,7 @@ sub process { # check for naked sscanf if ($^V && $^V ge 5.10.0 && defined $stat && - $stat =~ /\bsscanf\b/ && + $line =~ /\bsscanf\b/ && ($stat !~ /$Ident\s*=\s*sscanf\s*$balanced_parens/ && $stat !~ /\bsscanf\s*$balanced_parens\s*(?:$Compare)/ && $stat !~ /(?:$Compare)\s*\bsscanf\s*$balanced_parens/)) { @@ -4252,13 +5876,34 @@ sub process { "unchecked sscanf return value\n" . "$here\n$stat_real\n"); } +# check for simple sscanf that should be kstrto<foo> + if ($^V && $^V ge 5.10.0 && + defined $stat && + $line =~ /\bsscanf\b/) { + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + my $stat_real = raw_line($linenr, 0); + for (my $count = $linenr + 1; $count <= $lc; $count++) { + $stat_real = $stat_real . "\n" . raw_line($count, 0); + } + if ($stat_real =~ /\bsscanf\b\s*\(\s*$FuncArg\s*,\s*("[^"]+")/) { + my $format = $6; + my $count = $format =~ tr@%@%@; + if ($count == 1 && + $format =~ /^"\%(?i:ll[udxi]|[udxi]ll|ll|[hl]h?[udxi]|[udxi][hl]h?|[hl]h?|[udxi])"$/) { + WARN("SSCANF_TO_KSTRTO", + "Prefer kstrto<type> to single variable sscanf\n" . "$here\n$stat_real\n"); + } + } + } + # check for new externs in .h files. if ($realfile =~ /\.h$/ && $line =~ /^\+\s*(extern\s+)$Type\s*$Ident\s*\(/s) { if (CHK("AVOID_EXTERNS", "extern prototypes should be avoided in .h files\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/(.*)\bextern\b\s*(.*)/$1$2/; + $fixed[$fixlinenr] =~ s/(.*)\bextern\b\s*(.*)/$1$2/; } } @@ -4292,13 +5937,50 @@ sub process { "externs should be avoided in .c files\n" . $herecurr); } +# check for function declarations that have arguments without identifier names + if (defined $stat && + $stat =~ /^.\s*(?:extern\s+)?$Type\s*$Ident\s*\(\s*([^{]+)\s*\)\s*;/s && + $1 ne "void") { + my $args = trim($1); + while ($args =~ m/\s*($Type\s*(?:$Ident|\(\s*\*\s*$Ident?\s*\)\s*$balanced_parens)?)/g) { + my $arg = trim($1); + if ($arg =~ /^$Type$/ && $arg !~ /enum\s+$Ident$/) { + WARN("FUNCTION_ARGUMENTS", + "function definition argument '$arg' should also have an identifier name\n" . $herecurr); + } + } + } + +# check for function definitions + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^.\s*(?:$Storage\s+)?$Type\s*($Ident)\s*$balanced_parens\s*{/s) { + $context_function = $1; + +# check for multiline function definition with misplaced open brace + my $ok = 0; + my $cnt = statement_rawlines($stat); + my $herectx = $here . "\n"; + for (my $n = 0; $n < $cnt; $n++) { + my $rl = raw_line($linenr, $n); + $herectx .= $rl . "\n"; + $ok = 1 if ($rl =~ /^[ \+]\{/); + $ok = 1 if ($rl =~ /\{/ && $n == 0); + last if $rl =~ /^[ \+].*\{/; + } + if (!$ok) { + ERROR("OPEN_BRACE", + "open brace '{' following function definitions go on the next line\n" . $herectx); + } + } + # checks for new __setup's if ($rawline =~ /\b__setup\("([^"]*)"/) { my $name = $1; if (!grep(/$name/, @setup_docs)) { CHK("UNDOCUMENTED_SETUP", - "__setup appears un-documented -- check Documentation/kernel-parameters.txt\n" . $herecurr); + "__setup appears un-documented -- check Documentation/admin-guide/kernel-parameters.rst\n" . $herecurr); } } @@ -4316,6 +5998,38 @@ sub process { "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); } +# check for k[mz]alloc with multiplies that could be kmalloc_array/kcalloc + if ($^V && $^V ge 5.10.0 && + defined $stat && + $stat =~ /^\+\s*($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)\s*,/) { + my $oldfunc = $3; + my $a1 = $4; + my $a2 = $10; + my $newfunc = "kmalloc_array"; + $newfunc = "kcalloc" if ($oldfunc eq "kzalloc"); + my $r1 = $a1; + my $r2 = $a2; + if ($a1 =~ /^sizeof\s*\S/) { + $r1 = $a2; + $r2 = $a1; + } + if ($r1 !~ /^sizeof\b/ && $r2 =~ /^sizeof\s*\S/ && + !($r1 =~ /^$Constant$/ || $r1 =~ /^[A-Z_][A-Z0-9_]*$/)) { + my $ctx = ''; + my $herectx = $here . "\n"; + my $cnt = statement_rawlines($stat); + for (my $n = 0; $n < $cnt; $n++) { + $herectx .= raw_line($linenr, $n) . "\n"; + } + if (WARN("ALLOC_WITH_MULTIPLY", + "Prefer $newfunc over $oldfunc with multiply\n" . $herectx) && + $cnt == 1 && + $fix) { + $fixed[$fixlinenr] =~ s/\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*(k[mz]alloc)\s*\(\s*($FuncArg)\s*\*\s*($FuncArg)/$1 . ' = ' . "$newfunc(" . trim($r1) . ', ' . trim($r2)/e; + } + } + } + # check for krealloc arg reuse if ($^V && $^V ge 5.10.0 && $line =~ /\b($Lval)\s*\=\s*(?:$balanced_parens)?\s*krealloc\s*\(\s*\1\s*,/) { @@ -4329,18 +6043,34 @@ sub process { "$1 uses number as first arg, sizeof is generally wrong\n" . $herecurr); } -# check for GFP_NOWAIT use - if ($line =~ /\b__GFP_NOFAIL\b/) { - WARN("__GFP_NOFAIL", - "Use of __GFP_NOFAIL is deprecated, no new users should be added\n" . $herecurr); - } - # check for multiple semicolons if ($line =~ /;\s*;\s*$/) { if (WARN("ONE_SEMICOLON", "Statements terminations use 1 semicolon\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/(\s*;\s*){2,}$/;/g; + $fixed[$fixlinenr] =~ s/(\s*;\s*){2,}$/;/g; + } + } + +# check for #defines like: 1 << <digit> that could be BIT(digit), it is not exported to uapi + if ($realfile !~ m@^include/uapi/@ && + $line =~ /#\s*define\s+\w+\s+\(?\s*1\s*([ulUL]*)\s*\<\<\s*(?:\d+|$Ident)\s*\)?/) { + my $ull = ""; + $ull = "_ULL" if (defined($1) && $1 =~ /ll/i); + if (CHK("BIT_MACRO", + "Prefer using the BIT$ull macro\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\(?\s*1\s*[ulUL]*\s*<<\s*(\d+|$Ident)\s*\)?/BIT${ull}($1)/; + } + } + +# check for #if defined CONFIG_<FOO> || defined CONFIG_<FOO>_MODULE + if ($line =~ /^\+\s*#\s*if\s+defined(?:\s*\(?\s*|\s+)(CONFIG_[A-Z_]+)\s*\)?\s*\|\|\s*defined(?:\s*\(?\s*|\s+)\1_MODULE\s*\)?\s*$/) { + my $config = $1; + if (WARN("PREFER_IS_ENABLED", + "Prefer IS_ENABLED(<FOO>) to CONFIG_<FOO> || CONFIG_<FOO>_MODULE\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)"; } } @@ -4350,7 +6080,7 @@ sub process { my $has_statement = 0; my $count = 0; my $prevline = $linenr; - while ($prevline > 1 && $count < 3 && !$has_break) { + while ($prevline > 1 && ($file || $count < 3) && !$has_break) { $prevline--; my $rline = $rawlines[$prevline - 1]; my $fline = $lines[$prevline - 1]; @@ -4388,10 +6118,16 @@ sub process { if (WARN("USE_FUNC", "__func__ should be used instead of gcc specific __FUNCTION__\n" . $herecurr) && $fix) { - $fixed[$linenr - 1] =~ s/\b__FUNCTION__\b/__func__/g; + $fixed[$fixlinenr] =~ s/\b__FUNCTION__\b/__func__/g; } } +# check for uses of __DATE__, __TIME__, __TIMESTAMP__ + while ($line =~ /\b(__(?:DATE|TIME|TIMESTAMP)__)\b/g) { + ERROR("DATE_TIME", + "Use of the '$1' macro makes the build non-deterministic\n" . $herecurr); + } + # check for use of yield() if ($line =~ /\byield\s*\(\s*\)/) { WARN("YIELD", @@ -4437,55 +6173,18 @@ sub process { "$1 is obsolete, use k$3 instead\n" . $herecurr); } -# check for __initcall(), use device_initcall() explicitly please +# check for __initcall(), use device_initcall() explicitly or more appropriate function please if ($line =~ /^.\s*__initcall\s*\(/) { WARN("USE_DEVICE_INITCALL", - "please use device_initcall() instead of __initcall()\n" . $herecurr); - } - -# check for various ops structs, ensure they are const. - my $struct_ops = qr{acpi_dock_ops| - address_space_operations| - backlight_ops| - block_device_operations| - dentry_operations| - dev_pm_ops| - dma_map_ops| - extent_io_ops| - file_lock_operations| - file_operations| - hv_ops| - ide_dma_ops| - intel_dvo_dev_ops| - item_operations| - iwl_ops| - kgdb_arch| - kgdb_io| - kset_uevent_ops| - lock_manager_operations| - microcode_ops| - mtrr_ops| - neigh_ops| - nlmsvc_binding| - pci_raw_ops| - pipe_buf_operations| - platform_hibernation_ops| - platform_suspend_ops| - proto_ops| - rpc_pipe_ops| - seq_operations| - snd_ac97_build_ops| - soc_pcmcia_socket_ops| - stacktrace_ops| - sysfs_ops| - tty_operations| - usb_mon_operations| - wd_ops}x; + "please use device_initcall() or more appropriate function instead of __initcall() (see include/linux/init.h)\n" . $herecurr); + } + +# check for various structs that are normally const (ops, kgdb, device_tree) +# and avoid what seem like struct definitions 'struct foo {' if ($line !~ /\bconst\b/ && - $line =~ /\bstruct\s+($struct_ops)\b/) { + $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) { WARN("CONST_STRUCT", - "struct $1 should normally be const\n" . - $herecurr); + "struct $1 should normally be const\n" . $herecurr); } # use of NR_CPUS is usually wrong @@ -4507,16 +6206,11 @@ sub process { "#define of '$1' is wrong - use Kconfig variables or standard guards instead\n" . $herecurr); } -# check for %L{u,d,i} in strings - my $string; - while ($line =~ /(?:^|")([X\t]*)(?:"|$)/g) { - $string = substr($rawline, $-[1], $+[1] - $-[1]); - $string =~ s/%%/__/g; - if ($string =~ /(?<!%)%L[udi]/) { - WARN("PRINTF_L", - "\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr); - last; - } +# likely/unlikely comparisons similar to "(likely(foo) > 0)" + if ($^V && $^V ge 5.10.0 && + $line =~ /\b((?:un)?likely)\s*\(\s*$FuncArg\s*\)\s*$Compare/) { + WARN("LIKELY_MISUSE", + "Using $1 should generally have parentheses around the comparison\n" . $herecurr); } # whine mightly about in_atomic @@ -4530,6 +6224,34 @@ sub process { } } +# whine about ACCESS_ONCE + if ($^V && $^V ge 5.10.0 && + $line =~ /\bACCESS_ONCE\s*$balanced_parens\s*(=(?!=))?\s*($FuncArg)?/) { + my $par = $1; + my $eq = $2; + my $fun = $3; + $par =~ s/^\(\s*(.*)\s*\)$/$1/; + if (defined($eq)) { + if (WARN("PREFER_WRITE_ONCE", + "Prefer WRITE_ONCE(<FOO>, <BAR>) over ACCESS_ONCE(<FOO>) = <BAR>\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bACCESS_ONCE\s*\(\s*\Q$par\E\s*\)\s*$eq\s*\Q$fun\E/WRITE_ONCE($par, $fun)/; + } + } else { + if (WARN("PREFER_READ_ONCE", + "Prefer READ_ONCE(<FOO>) over ACCESS_ONCE(<FOO>)\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bACCESS_ONCE\s*\(\s*\Q$par\E\s*\)/READ_ONCE($par)/; + } + } + } + +# check for mutex_trylock_recursive usage + if ($line =~ /mutex_trylock_recursive/) { + ERROR("LOCKING", + "recursive locking is bad, do not use this ever.\n" . $herecurr); + } + # check for lockdep_set_novalidate_class if ($line =~ /^.\s*lockdep_set_novalidate_class\s*\(/ || $line =~ /__lockdep_no_validate__\s*\)/ ) { @@ -4541,11 +6263,95 @@ sub process { } } - if ($line =~ /debugfs_create_file.*S_IWUGO/ || - $line =~ /DEVICE_ATTR.*S_IWUGO/ ) { + if ($line =~ /debugfs_create_\w+.*\b$mode_perms_world_writable\b/ || + $line =~ /DEVICE_ATTR.*\b$mode_perms_world_writable\b/) { WARN("EXPORTED_WORLD_WRITABLE", "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr); } + +# Mode permission misuses where it seems decimal should be octal +# This uses a shortcut match to avoid unnecessary uses of a slow foreach loop + if ($^V && $^V ge 5.10.0 && + defined $stat && + $line =~ /$mode_perms_search/) { + foreach my $entry (@mode_permission_funcs) { + my $func = $entry->[0]; + my $arg_pos = $entry->[1]; + + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + my $stat_real = raw_line($linenr, 0); + for (my $count = $linenr + 1; $count <= $lc; $count++) { + $stat_real = $stat_real . "\n" . raw_line($count, 0); + } + + my $skip_args = ""; + if ($arg_pos > 1) { + $arg_pos--; + $skip_args = "(?:\\s*$FuncArg\\s*,\\s*){$arg_pos,$arg_pos}"; + } + my $test = "\\b$func\\s*\\(${skip_args}($FuncArg(?:\\|\\s*$FuncArg)*)\\s*[,\\)]"; + if ($stat =~ /$test/) { + my $val = $1; + $val = $6 if ($skip_args ne ""); + if (($val =~ /^$Int$/ && $val !~ /^$Octal$/) || + ($val =~ /^$Octal$/ && length($val) ne 4)) { + ERROR("NON_OCTAL_PERMISSIONS", + "Use 4 digit octal (0777) not decimal permissions\n" . "$here\n" . $stat_real); + } + if ($val =~ /^$Octal$/ && (oct($val) & 02)) { + ERROR("EXPORTED_WORLD_WRITABLE", + "Exporting writable files is usually an error. Consider more restrictive permissions.\n" . "$here\n" . $stat_real); + } + } + } + } + +# check for uses of S_<PERMS> that could be octal for readability + if ($line =~ /\b$mode_perms_string_search\b/) { + my $val = ""; + my $oval = ""; + my $to = 0; + my $curpos = 0; + my $lastpos = 0; + while ($line =~ /\b(($mode_perms_string_search)\b(?:\s*\|\s*)?\s*)/g) { + $curpos = pos($line); + my $match = $2; + my $omatch = $1; + last if ($lastpos > 0 && ($curpos - length($omatch) != $lastpos)); + $lastpos = $curpos; + $to |= $mode_permission_string_types{$match}; + $val .= '\s*\|\s*' if ($val ne ""); + $val .= $match; + $oval .= $omatch; + } + $oval =~ s/^\s*\|\s*//; + $oval =~ s/\s*\|\s*$//; + my $octal = sprintf("%04o", $to); + if (WARN("SYMBOLIC_PERMS", + "Symbolic permissions '$oval' are not preferred. Consider using octal permissions '$octal'.\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/$val/$octal/; + } + } + +# validate content of MODULE_LICENSE against list from include/linux/module.h + if ($line =~ /\bMODULE_LICENSE\s*\(\s*($String)\s*\)/) { + my $extracted_string = get_quoted_string($line, $rawline); + my $valid_licenses = qr{ + GPL| + GPL\ v2| + GPL\ and\ additional\ rights| + Dual\ BSD/GPL| + Dual\ MIT/GPL| + Dual\ MPL/GPL| + Proprietary + }x; + if ($extracted_string !~ /^"(?:$valid_licenses)"$/x) { + WARN("MODULE_LICENSE", + "unknown module license " . $extracted_string . "\n" . $herecurr); + } + } } # If we have no input at all, then there is nothing to report on @@ -4566,11 +6372,11 @@ sub process { exit(0); } - if (!$is_patch) { + if (!$is_patch && $file !~ /cover-letter\.patch$/) { ERROR("NOT_UNIFIED_DIFF", "Does not appear to be a unified-diff format patch\n"); } - if ($is_patch && $chk_signoff && $signoff == 0) { + if ($is_patch && $has_commit_log && $chk_signoff && $signoff == 0) { ERROR("MISSING_SIGN_OFF", "Missing Signed-off-by: line(s)\n"); } @@ -4581,34 +6387,39 @@ sub process { print "total: $cnt_error errors, $cnt_warn warnings, " . (($check)? "$cnt_chk checks, " : "") . "$cnt_lines lines checked\n"; - print "\n" if ($quiet == 0); } if ($quiet == 0) { + # If there were any defects found and not already fixing them + if (!$clean and !$fix) { + print << "EOM" - if ($^V lt 5.10.0) { - print("NOTE: perl $^V is not modern enough to detect all possible issues.\n"); - print("An upgrade to at least perl v5.10.0 is suggested.\n\n"); +NOTE: For some of the reported defects, checkpatch may be able to + mechanically convert to the typical style using --fix or --fix-inplace. +EOM } - # If there were whitespace errors which cleanpatch can fix # then suggest that. if ($rpt_cleaners) { - print "NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or\n"; - print " scripts/cleanfile\n\n"; $rpt_cleaners = 0; + print << "EOM" + +NOTE: Whitespace errors detected. + You may wish to use scripts/cleanpatch or scripts/cleanfile +EOM } } - hash_show_words(\%use_type, "Used"); - hash_show_words(\%ignore_type, "Ignored"); - - if ($clean == 0 && $fix && "@rawlines" ne "@fixed") { + if ($clean == 0 && $fix && + ("@rawlines" ne "@fixed" || + $#fixed_inserted >= 0 || $#fixed_deleted >= 0)) { my $newfile = $filename; $newfile .= ".EXPERIMENTAL-checkpatch-fixes" if (!$fix_inplace); my $linecount = 0; my $f; + @fixed = fix_inserted_deleted_lines(\@fixed, \@fixed_inserted, \@fixed_deleted); + open($f, '>', $newfile) or die "$P: Can't open $newfile for write\n"; foreach my $fixed_line (@fixed) { @@ -4616,7 +6427,7 @@ sub process { if ($file) { if ($linecount > 3) { $fixed_line =~ s/^\+//; - print $f $fixed_line. "\n"; + print $f $fixed_line . "\n"; } } else { print $f $fixed_line . "\n"; @@ -4626,6 +6437,7 @@ sub process { if (!$quiet) { print << "EOM"; + Wrote EXPERIMENTAL --fix correction(s) to '$newfile' Do _NOT_ trust the results written to this file. @@ -4633,22 +6445,17 @@ Do _NOT_ submit these changes without inspecting them for correctness. This EXPERIMENTAL file is simply a convenience to help rewrite patches. No warranties, expressed or implied... - EOM } } - if ($clean == 1 && $quiet == 0) { - print "$vname has no obvious style problems and is ready for submission.\n" - } - if ($clean == 0 && $quiet == 0) { - print << "EOM"; -$vname has style problems, please review. - -If any of these errors are false positives, please report -them to the maintainer, see CHECKPATCH in MAINTAINERS. -EOM + if ($quiet == 0) { + print "\n"; + if ($clean == 1) { + print "$vname has no obvious style problems and is ready for submission.\n"; + } else { + print "$vname has style problems, please review.\n"; + } } - return $clean; } diff --git a/scripts/const_structs.checkpatch b/scripts/const_structs.checkpatch new file mode 100644 index 0000000000..da775bca0c --- /dev/null +++ b/scripts/const_structs.checkpatch @@ -0,0 +1,2 @@ +# Put structs here that should be constant +__dummy__ diff --git a/test/py/tests/test_gpt.py b/test/py/tests/test_gpt.py new file mode 100644 index 0000000000..e2bbd08e6d --- /dev/null +++ b/test/py/tests/test_gpt.py @@ -0,0 +1,114 @@ +# Copyright (c) 2017 Alison Chaiken +# Copyright (c) 2017, NVIDIA CORPORATION. All rights reserved. +# +# SPDX-License-Identifier: GPL-2.0 + +# Test GPT manipulation commands. + +import os +import pytest +import u_boot_utils + +""" +These tests rely on a 4 MB disk image, which is automatically created by +the test. +""" + +class GptTestDiskImage(object): + """Disk Image used by the GPT tests.""" + + def __init__(self, u_boot_console): + """Initialize a new GptTestDiskImage object. + + Args: + u_boot_console: A U-Boot console. + + Returns: + Nothing. + """ + + filename = 'test_gpt_disk_image.bin' + self.path = u_boot_console.config.persistent_data_dir + '/' + filename + + if os.path.exists(self.path): + u_boot_console.log.action('Disk image file ' + self.path + + ' already exists') + else: + u_boot_console.log.action('Generating ' + self.path) + fd = os.open(self.path, os.O_RDWR | os.O_CREAT) + os.ftruncate(fd, 4194304) + os.close(fd) + sgdisk = '/sbin/sgdisk' + cmd = (sgdisk, '-U', '375a56f7-d6c9-4e81-b5f0-09d41ca89efe', + self.path) + u_boot_utils.run_and_log(u_boot_console, cmd) + cmd = (sgdisk, '--new=1:2048:2560', self.path) + u_boot_utils.run_and_log(u_boot_console, cmd) + cmd = (sgdisk, '--new=2:4096:4608', self.path) + u_boot_utils.run_and_log(u_boot_console, cmd) + cmd = (sgdisk, '-l', self.path) + u_boot_utils.run_and_log(u_boot_console, cmd) + +gtdi = None +@pytest.fixture(scope='function') +def state_disk_image(u_boot_console): + """pytest fixture to provide a GptTestDiskImage object to tests. + This is function-scoped because it uses u_boot_console, which is also + function-scoped. However, we don't need to actually do any function-scope + work, so this simply returns the same object over and over each time.""" + + global gtdi + if not gtdi: + gtdi = GptTestDiskImage(u_boot_console) + return gtdi + +@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('cmd_gpt') +def test_gpt_guid(state_disk_image, u_boot_console): + """Test the gpt guid command.""" + + u_boot_console.run_command('host bind 0 ' + state_disk_image.path) + output = u_boot_console.run_command('gpt guid host 0') + assert '375a56f7-d6c9-4e81-b5f0-09d41ca89efe' in output + +@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('cmd_gpt') +def test_gpt_save_guid(state_disk_image, u_boot_console): + """Test the gpt guid command to save GUID into a string.""" + + if u_boot_console.config.buildconfig.get('config_cmd_gpt', 'n') != 'y': + pytest.skip('gpt command not supported') + u_boot_console.run_command('host bind 0 ' + state_disk_image.path) + output = u_boot_console.run_command('gpt guid host 0 newguid') + output = u_boot_console.run_command('printenv newguid') + assert '375a56f7-d6c9-4e81-b5f0-09d41ca89efe' in output + +@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('cmd_gpt') +@pytest.mark.buildconfigspec('cmd_gpt_rename') +def test_gpt_rename_partition(state_disk_image, u_boot_console): + """Test the gpt rename command to write partition names.""" + + u_boot_console.run_command('host bind 0 ' + state_disk_image.path) + u_boot_console.run_command('gpt rename host 0 1 first') + output = u_boot_console.run_command('gpt read host 0') + assert 'name first' in output + u_boot_console.run_command('gpt rename host 0 2 second') + output = u_boot_console.run_command('gpt read host 0') + assert 'name second' in output + +@pytest.mark.boardspec('sandbox') +@pytest.mark.buildconfigspec('cmd_gpt') +@pytest.mark.buildconfigspec('cmd_gpt_rename') +@pytest.mark.buildconfigspec('cmd_part') +def test_gpt_swap_partitions(state_disk_image, u_boot_console): + """Test the gpt swap command to exchange two partition names.""" + + u_boot_console.run_command('host bind 0 ' + state_disk_image.path) + output = u_boot_console.run_command('part list host 0') + assert '0x000007ff "first"' in output + assert '0x000017ff "second"' in output + u_boot_console.run_command('gpt swap host 0 first second') + output = u_boot_console.run_command('part list host 0') + assert '0x000007ff "second"' in output + assert '0x000017ff "first"' in output diff --git a/test/py/u_boot_console_base.py b/test/py/u_boot_console_base.py index b1f474236e..eedf73f858 100644 --- a/test/py/u_boot_console_base.py +++ b/test/py/u_boot_console_base.py @@ -160,7 +160,7 @@ class ConsoleBase(object): Args: cmd: The command to send. - wait_for_each: Boolean indicating whether to wait for U-Boot to + wait_for_echo: Boolean indicating whether to wait for U-Boot to echo the command text back to its output. send_nl: Boolean indicating whether to send a newline character after the command string. |