August 2021 - U-Boot

sandbox TPM
by Heinrich Schuchardt 02 Sep '21

02 Sep '21

The current TPM emulation in drivers/tpm/tpm(2)_tis_sandbox.c is not spec compliant. A TPM emulation as UNIX socket exists with https://github.com/stefanberger/swtpm.git. QEMU already uses this emulator. Couldn't the sandbox do the same? I think this is the fastest way to get a compliant sandbox TPM. Best regards Heinrich

4 5

Setting serverip from DHCP server
by Gregory Anders 02 Sep '21

02 Sep '21

(Re-sending now that I'm subscribed to the list) Hi all, I am running U-Boot on an embedded device that is connected via Ethernet to my workstation. The workstation is running dhcpd and U-Boot is able to successfully obtain an IP address via DHCP from the server. However, the `serverip` environment variable is not being set which prevents U-Boot from continuing to boot over the network. I have to manually enter `setenv serverip 10.0.10.1` each time. How do I get the DHCP server to set the serverip variable? My dhcpd.conf file is quite simple: subnet 10.0.10.0 netmask 255.255.255.0 { option routers 10.0.10.1; range 10.0.10.2; } I would have thought the 'option routers' line would do the trick, but apparently not. I've done a bit of searching online but haven't yet found anything helpful. Thanks, Greg

2 2

[PATCH v5 0/3] arm64: Add optimized memset/memcpy/memove functions
by Stefan Roese 02 Sep '21

02 Sep '21

On an NXP LX2160 based platform it has been noticed, that the currently implemented memset/memcpy functions for aarch64 are suboptimal. Especially the memset() for clearing the NXP MC firmware memory is very expensive (time-wise). This patchset now adds the optimized functions ported from this repository: https://github.com/ARM-software/optimized-routines As the optimized memset function make use of the dc opcode, which needs the caches to be enabled, an additional check is added and a simple memset version is used in this case. Please note that checkpatch.pl complains about some issue with this imported file: arch/arm/lib/asmdefs.h Since it's imported I did explicitly not make any changes here, to make potential future sync'ing easer. Here some numbers to see the speed improments: Current original version: ------------------------- memset() 32 Bytes, 16M times: time: 0.446 seconds memset() 16MiB, 256 times: time: 1.076 seconds memcpy() 512MiB: time: 0.224 seconds New optimized version: ---------------------- memset() 32 Bytes, 16M times: time: 0.287 seconds memset() 16MiB, 256 times: time: 0.292 seconds memcpy() 512MiB: time: 0.222 seconds Summary: The optimized memcpy is nearly identical to the original one. But the optimized memset is much faster, for small and big sizes. Small sizes factor ~1.6 and big sizes factor ~3.7. Note: These measurements were done on the NXP LX2160ARDB board. Thanks, Stefan Changes in v5: - memmove is now auto-selected (or deselected) with the memcpy Kconfig selection as it's entry is the same as memcpy for ARM64 Changes in v4: - Use macros instead of register names, following the optimized code - Add zero size check Changes in v3: - Add memmove alias, as this function also handles it optimized - Add memmove as well Changes in v2: - Add file names and locations and git commit ID from imported files to the commit message - New patch Stefan Roese (3): arm64: arch/arm/lib: Add optimized memset/memcpy/memmove functions arm64: memset-arm64: Use simple memset when cache is disabled arm64: Kconfig: Enable usage of optimized memset/memcpy/memmove arch/arm/Kconfig | 37 +++++- arch/arm/include/asm/string.h | 4 + arch/arm/lib/Makefile | 5 + arch/arm/lib/asmdefs.h | 98 ++++++++++++++ arch/arm/lib/memcpy-arm64.S | 242 ++++++++++++++++++++++++++++++++++ arch/arm/lib/memset-arm64.S | 148 +++++++++++++++++++++ 6 files changed, 528 insertions(+), 6 deletions(-) create mode 100644 arch/arm/lib/asmdefs.h create mode 100644 arch/arm/lib/memcpy-arm64.S create mode 100644 arch/arm/lib/memset-arm64.S -- 2.33.0

2 6

[PATCH] list: include linux/kernel.h
by Mark-PK Tsai 02 Sep '21

02 Sep '21

linux/list.h uses container_of, therefore it depends on linux/kernel.h. Sync from below linux commit: commit 8b21d9ca17ff8 ("list: include linux/kernel.h") Signed-off-by: Mark-PK Tsai <mark-pk.tsai(a)mediatek.com> Cc: YJ Chiang <yj.chiang(a)mediatek.com> --- include/linux/list.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/linux/list.h b/include/linux/list.h index f62afa092c..5320c04bf3 100644 --- a/include/linux/list.h +++ b/include/linux/list.h @@ -3,6 +3,7 @@ #include <linux/stddef.h> #include <linux/poison.h> +#include <linux/kernel.h> #ifndef ARCH_HAS_PREFETCH #define ARCH_HAS_PREFETCH -- 2.18.0

2 3

[PATCH 1/2] ata: ahci-pci: Fix dependency on DM_PCI
by Pali Rohár 02 Sep '21

02 Sep '21

File drivers/ata/ahci-pci.c calls function ahci_probe_scsi_pci() which is compiled only when DM_PCI is enabled. So add missing dependency into Kconfig. Signed-off-by: Pali Rohár <pali(a)kernel.org> --- drivers/ata/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig index 6f0b77238364..0c1490a9f900 100644 --- a/drivers/ata/Kconfig +++ b/drivers/ata/Kconfig @@ -36,6 +36,7 @@ menu "SATA/SCSI device support" config AHCI_PCI bool "Support for PCI-based AHCI controller" + depends on DM_PCI depends on DM_SCSI help Enables support for the PCI-based AHCI controller. -- 2.20.1

3 5

[PATCH 0/1] rtc: ds1307: Fix incorrect clock reset for DS13xx
by Callum Sinclair 02 Sep '21

02 Sep '21

The DS1307 driver also supports the DS1337, DS1339 and DS1340 rtc. However the reset registers between these rtc devices are not the same. This means calling the rtc reset routine for a DS1307 clock on a DS1340 device will cause the clock to be calibrated incorrectly rather than correctly reset. Define different reset routines for the different clock variants to prevent a rtc reset causing the clock to become incorrectly calibrated rather than reset. Callum Sinclair (1): rtc: ds1307: Fix incorrect clock reset for DS13xx drivers/rtc/ds1307.c | 69 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 56 insertions(+), 13 deletions(-) -- 2.32.0

2 2

[PATCH 0/7] Enable new drivers added for U-Boot 2021.10 in "stemmy" board
by Stephan Gerhold 02 Sep '21

02 Sep '21

This series finishes up the work done for ARM U8500 in the U-Boot 2021.10 merge window. The new drivers are added to MAINTAINERS, then enabled in Kconfig and finally the "stemmy" board is updated to make use of them. Stephan Gerhold (7): MAINTAINERS: Add new drivers for ARM U8500 arm: u8500: Imply options for new drivers arm: dts: u8500: Update from Linux ux500-dts-for-v5.15 arm: dts: u8500: u-boot: Add fixed clock for eMMC board: stemmy: Add basic Fastboot support board: stemmy: Update documentation arm: u8500: Prefer building in thumb mode by default MAINTAINERS | 5 + arch/arm/Kconfig | 8 ++ arch/arm/dts/ste-ab8500.dtsi | 116 +++++++++++++++++----- arch/arm/dts/ste-ab8505.dtsi | 95 ++++++++++++++---- arch/arm/dts/ste-dbx5x0-u-boot.dtsi | 11 +- arch/arm/dts/ste-dbx5x0.dtsi | 14 +-- arch/arm/dts/ste-ux500-samsung-stemmy.dts | 16 +++ arch/arm/mach-u8500/Kconfig | 11 +- board/ste/stemmy/MAINTAINERS | 3 +- board/ste/stemmy/README | 50 ---------- configs/stemmy_defconfig | 14 +++ doc/board/index.rst | 1 + doc/board/ste/index.rst | 9 ++ doc/board/ste/stemmy.rst | 81 +++++++++++++++ include/configs/stemmy.h | 27 +++++ 15 files changed, 354 insertions(+), 107 deletions(-) delete mode 100644 board/ste/stemmy/README create mode 100644 doc/board/ste/index.rst create mode 100644 doc/board/ste/stemmy.rst -- 2.32.0

3 9

[PATCH] loadb: Properly indicate aborted kermit transfer
by Pali Rohár 02 Sep '21

02 Sep '21

When k_recv() returns zero it indicates that kermit transfer was aborted. Function do_load_serial_bin() (caller of load_serial_bin()) interprets value ~0 as aborted transfer, so properly propagates information about aborted transfer from k_recv() to do_load_serial_bin(). Signed-off-by: Pali Rohár <pali(a)kernel.org> --- cmd/load.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cmd/load.c b/cmd/load.c index 462340ad2cde..249ebd4ae078 100644 --- a/cmd/load.c +++ b/cmd/load.c @@ -551,6 +551,9 @@ static ulong load_serial_bin(ulong offset) udelay(1000); } + if (size == 0) + return ~0; /* Download aborted */ + flush_cache(offset, size); printf("## Total Size = 0x%08x = %d Bytes\n", size, size); -- 2.20.1

3 4

[PATCH] scripts/checkpatch.pl: Resync with v5.13
by Tom Rini 02 Sep '21

02 Sep '21

This resyncs us with the version found in v5.13 of the Linux kernel with the following exceptions: - Keep our u-boot specific tests / code area. - Change the location of checkpatch.rst (which we now import) - Drop the "use strscpy" test as we don't have that, but do have strlcpy and want that used now. - Keep debug/printf in the list for $logFunctions And note that we now also include the spdxcheck.py tool that checkpatch.pl supports calling out to, and include upstream's checkpatch.rst in our develop section of the documentation. Signed-off-by: Tom Rini <trini(a)konsulko.com> --- doc/develop/checkpatch.rst | 755 ++++++++++++++++++++++++ doc/develop/index.rst | 1 + scripts/checkpatch.pl | 1114 ++++++++++++++++++++++++++++-------- scripts/spdxcheck.py | 296 ++++++++++ 4 files changed, 1919 insertions(+), 247 deletions(-) create mode 100644 doc/develop/checkpatch.rst create mode 100755 scripts/spdxcheck.py diff --git a/doc/develop/checkpatch.rst b/doc/develop/checkpatch.rst new file mode 100644 index 000000000000..51fed1bd72ec --- /dev/null +++ b/doc/develop/checkpatch.rst @@ -0,0 +1,755 @@ +.. SPDX-License-Identifier: GPL-2.0-only + +========== +Checkpatch +========== + +Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial +style violations in patches and optionally corrects them. Checkpatch can +also be run on file contexts and without the kernel tree. + +Checkpatch is not always right. Your judgement takes precedence over checkpatch +messages. If your code looks better with the violations, then its probably +best left alone. + + +Options +======= + +This section will describe the options checkpatch can be run with. + +Usage:: + + ./scripts/checkpatch.pl [OPTION]... [FILE]... + +Available options: + + - -q, --quiet + + Enable quiet mode. + + - -v, --verbose + Enable verbose mode. Additional verbose test descriptions are output + so as to provide information on why that particular message is shown. + + - --no-tree + + Run checkpatch without the kernel tree. + + - --no-signoff + + Disable the 'Signed-off-by' line check. The sign-off is a simple line at + the end of the explanation for the patch, which certifies that you wrote it + or otherwise have the right to pass it on as an open-source patch. + + Example:: + + Signed-off-by: Random J Developer <random(a)developer.example.org> + + Setting this flag effectively stops a message for a missing signed-off-by + line in a patch context. + + - --patch + + Treat FILE as a patch. This is the default option and need not be + explicitly specified. + + - --emacs + + Set output to emacs compile window format. This allows emacs users to jump + from the error in the compile window directly to the offending line in the + patch. + + - --terse + + Output only one line per report. + + - --showfile + + Show the diffed file position instead of the input file position. + + - -g, --git + + Treat FILE as a single commit or a git revision range. + + Single commit with: + + - <rev> + - <rev>^ + - <rev>~n + + Multiple commits with: + + - <rev1>..<rev2> + - <rev1>...<rev2> + - <rev>-<count> + + - -f, --file + + Treat FILE as a regular source file. This option must be used when running + checkpatch on source files in the kernel. + + - --subjective, --strict + + Enable stricter tests in checkpatch. By default the tests emitted as CHECK + do not activate by default. Use this flag to activate the CHECK tests. + + - --list-types + + Every message emitted by checkpatch has an associated TYPE. Add this flag + to display all the types in checkpatch. + + Note that when this flag is active, checkpatch does not read the input FILE, + and no message is emitted. Only a list of types in checkpatch is output. + + - --types TYPE(,TYPE2...) + + Only display messages with the given types. + + Example:: + + ./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,BRACES + + - --ignore TYPE(,TYPE2...) + + Checkpatch will not emit messages for the specified types. + + Example:: + + ./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,BRACES + + - --show-types + + By default checkpatch doesn't display the type associated with the messages. + Set this flag to show the message type in the output. + + - --max-line-length=n + + Set the max line length (default 100). If a line exceeds the specified + length, a LONG_LINE message is emitted. + + + The message level is different for patch and file contexts. For patches, + a WARNING is emitted. While a milder CHECK is emitted for files. So for + file contexts, the --strict flag must also be enabled. + + - --min-conf-desc-length=n + + Set the Kconfig entry minimum description length, if shorter, warn. + + - --tab-size=n + + Set the number of spaces for tab (default 8). + + - --root=PATH + + PATH to the kernel tree root. + + This option must be specified when invoking checkpatch from outside + the kernel root. + + - --no-summary + + Suppress the per file summary. + + - --mailback + + Only produce a report in case of Warnings or Errors. Milder Checks are + excluded from this. + + - --summary-file + + Include the filename in summary. + + - --debug KEY=[0|1] + + Turn on/off debugging of KEY, where KEY is one of 'values', 'possible', + 'type', and 'attr' (default is all off). + + - --fix + + This is an EXPERIMENTAL feature. If correctable errors exists, a file + <inputfile>.EXPERIMENTAL-checkpatch-fixes is created which has the + automatically fixable errors corrected. + + - --fix-inplace + + EXPERIMENTAL - Similar to --fix but input file is overwritten with fixes. + + DO NOT USE this flag unless you are absolutely sure and you have a backup + in place. + + - --ignore-perl-version + + Override checking of perl version. Runtime errors maybe encountered after + enabling this flag if the perl version does not meet the minimum specified. + + - --codespell + + Use the codespell dictionary for checking spelling errors. + + - --codespellfile + + Use the specified codespell file. + Default is '/usr/share/codespell/dictionary.txt'. + + - --typedefsfile + + Read additional types from this file. + + - --color[=WHEN] + + Use colors 'always', 'never', or only when output is a terminal ('auto'). + Default is 'auto'. + + - --kconfig-prefix=WORD + + Use WORD as a prefix for Kconfig symbols (default is `CONFIG_`). + + - -h, --help, --version + + Display the help text. + +Message Levels +============== + +Messages in checkpatch are divided into three levels. The levels of messages +in checkpatch denote the severity of the error. They are: + + - ERROR + + This is the most strict level. Messages of type ERROR must be taken + seriously as they denote things that are very likely to be wrong. + + - WARNING + + This is the next stricter level. Messages of type WARNING requires a + more careful review. But it is milder than an ERROR. + + - CHECK + + This is the mildest level. These are things which may require some thought. + +Type Descriptions +================= + +This section contains a description of all the message types in checkpatch. + +.. Types in this section are also parsed by checkpatch. +.. The types are grouped into subsections based on use. + + +Allocation style +---------------- + + **ALLOC_ARRAY_ARGS** + The first argument for kcalloc or kmalloc_array should be the + number of elements. sizeof() as the first argument is generally + wrong. + See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html + + **ALLOC_SIZEOF_STRUCT** + The allocation style is bad. In general for family of + allocation functions using sizeof() to get memory size, + constructs like:: + + p = alloc(sizeof(struct foo), ...) + + should be:: + + p = alloc(sizeof(*p), ...) + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#allocating… + + **ALLOC_WITH_MULTIPLY** + Prefer kmalloc_array/kcalloc over kmalloc/kzalloc with a + sizeof multiply. + See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html + + +API usage +--------- + + **ARCH_DEFINES** + Architecture specific defines should be avoided wherever + possible. + + **ARCH_INCLUDE_LINUX** + Whenever asm/file.h is included and linux/file.h exists, a + conversion can be made when linux/file.h includes asm/file.h. + However this is not always the case (See signal.h). + This message type is emitted only for includes from arch/. + + **AVOID_BUG** + BUG() or BUG_ON() should be avoided totally. + Use WARN() and WARN_ON() instead, and handle the "impossible" + error condition as gracefully as possible. + See: https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-… + + **CONSIDER_KSTRTO** + The simple_strtol(), simple_strtoll(), simple_strtoul(), and + simple_strtoull() functions explicitly ignore overflows, which + may lead to unexpected results in callers. The respective kstrtol(), + kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the + correct replacements. + See: https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strto… + + **LOCKDEP** + The lockdep_no_validate class was added as a temporary measure to + prevent warnings on conversion of device->sem to device->mutex. + It should not be used for any other purpose. + See: https://lore.kernel.org/lkml/1268959062.9440.467.camel@laptop/ + + **MALFORMED_INCLUDE** + The #include statement has a malformed path. This has happened + because the author has included a double slash "//" in the pathname + accidentally. + + **USE_LOCKDEP** + lockdep_assert_held() annotations should be preferred over + assertions based on spin_is_locked() + See: https://www.kernel.org/doc/html/latest/locking/lockdep-design.html#annotati… + + **UAPI_INCLUDE** + No #include statements in include/uapi should use a uapi/ path. + + +Comment style +------------- + + **BLOCK_COMMENT_STYLE** + The comment style is incorrect. The preferred style for multi- + line comments is:: + + /* + * This is the preferred style + * for multi line comments. + */ + + The networking comment style is a bit different, with the first line + not empty like the former:: + + /* This is the preferred comment style + * for files in net/ and drivers/net/ + */ + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting + + **C99_COMMENTS** + C99 style single line comments (//) should not be used. + Prefer the block comment style instead. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting + + +Commit message +-------------- + + **BAD_SIGN_OFF** + The signed-off-by line does not fall in line with the standards + specified by the community. + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#deve… + + **BAD_STABLE_ADDRESS_STYLE** + The email format for stable is incorrect. + Some valid options for stable address are:: + + 1. stable(a)vger.kernel.org + 2. stable(a)kernel.org + + For adding version info, the following comment style should be used:: + + stable(a)vger.kernel.org # version info + + **COMMIT_COMMENT_SYMBOL** + Commit log lines starting with a '#' are ignored by git as + comments. To solve this problem addition of a single space + infront of the log line is enough. + + **COMMIT_MESSAGE** + The patch is missing a commit description. A brief + description of the changes made by the patch should be added. + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#desc… + + **MISSING_SIGN_OFF** + The patch is missing a Signed-off-by line. A signed-off-by + line should be added according to Developer's certificate of + Origin. + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign… + + **NO_AUTHOR_SIGN_OFF** + The author of the patch has not signed off the patch. It is + required that a simple sign off line should be present at the + end of explanation of the patch to denote that the author has + written it or otherwise has the rights to pass it on as an open + source patch. + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign… + + **DIFF_IN_COMMIT_MSG** + Avoid having diff content in commit message. + This causes problems when one tries to apply a file containing both + the changelog and the diff because patch(1) tries to apply the diff + which it found in the changelog. + See: https://lore.kernel.org/lkml/20150611134006.9df79a893e3636019ad2759e@linux-… + + **GERRIT_CHANGE_ID** + To be picked up by gerrit, the footer of the commit message might + have a Change-Id like:: + + Change-Id: Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b + Signed-off-by: A. U. Thor <author(a)example.com> + + The Change-Id line must be removed before submitting. + + **GIT_COMMIT_ID** + The proper way to reference a commit id is: + commit <12+ chars of sha1> ("<title line>") + + An example may be:: + + Commit e21d2170f36602ae2708 ("video: remove unnecessary + platform_set_drvdata()") removed the unnecessary + platform_set_drvdata(), but left the variable "dev" unused, + delete it. + + See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#desc… + + +Comparison style +---------------- + + **ASSIGN_IN_IF** + Do not use assignments in if condition. + Example:: + + if ((foo = bar(...)) < BAZ) { + + should be written as:: + + foo = bar(...); + if (foo < BAZ) { + + **BOOL_COMPARISON** + Comparisons of A to true and false are better written + as A and !A. + See: https://lore.kernel.org/lkml/1365563834.27174.12.camel@joe-AO722/ + + **COMPARISON_TO_NULL** + Comparisons to NULL in the form (foo == NULL) or (foo != NULL) + are better written as (!foo) and (foo). + + **CONSTANT_COMPARISON** + Comparisons with a constant or upper case identifier on the left + side of the test should be avoided. + + +Macros, Attributes and Symbols +------------------------------ + + **ARRAY_SIZE** + The ARRAY_SIZE(foo) macro should be preferred over + sizeof(foo)/sizeof(foo[0]) for finding number of elements in an + array. + + The macro is defined in include/linux/kernel.h:: + + #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + + **AVOID_EXTERNS** + Function prototypes don't need to be declared extern in .h + files. It's assumed by the compiler and is unnecessary. + + **AVOID_L_PREFIX** + Local symbol names that are prefixed with `.L` should be avoided, + as this has special meaning for the assembler; a symbol entry will + not be emitted into the symbol table. This can prevent `objtool` + from generating correct unwind info. + + Symbols with STB_LOCAL binding may still be used, and `.L` prefixed + local symbol names are still generally usable within a function, + but `.L` prefixed local symbol names should not be used to denote + the beginning or end of code regions via + `SYM_CODE_START_LOCAL`/`SYM_CODE_END` + + **BIT_MACRO** + Defines like: 1 << <digit> could be BIT(digit). + The BIT() macro is defined in include/linux/bitops.h:: + + #define BIT(nr) (1UL << (nr)) + + **CONST_READ_MOSTLY** + When a variable is tagged with the __read_mostly annotation, it is a + signal to the compiler that accesses to the variable will be mostly + reads and rarely(but NOT never) a write. + + const __read_mostly does not make any sense as const data is already + read-only. The __read_mostly annotation thus should be removed. + + **DATE_TIME** + It is generally desirable that building the same source code with + the same set of tools is reproducible, i.e. the output is always + exactly the same. + + The kernel does *not* use the ``__DATE__`` and ``__TIME__`` macros, + and enables warnings if they are used as they can lead to + non-deterministic builds. + See: https://www.kernel.org/doc/html/latest/kbuild/reproducible-builds.html#time… + + **DEFINE_ARCH_HAS** + The ARCH_HAS_xyz and ARCH_HAVE_xyz patterns are wrong. + + For big conceptual features use Kconfig symbols instead. And for + smaller things where we have compatibility fallback functions but + want architectures able to override them with optimized ones, we + should either use weak functions (appropriate for some cases), or + the symbol that protects them should be the same symbol we use. + See: https://lore.kernel.org/lkml/CA+55aFycQ9XJvEOsiM3txHL5bjUc8CeKWJNR_H+Miicad… + + **INIT_ATTRIBUTE** + Const init definitions should use __initconst instead of + __initdata. + + Similarly init definitions without const require a separate + use of const. + + **INLINE_LOCATION** + The inline keyword should sit between storage class and type. + + For example, the following segment:: + + inline static int example_function(void) + { + ... + } + + should be:: + + static inline int example_function(void) + { + ... + } + + **MULTISTATEMENT_MACRO_USE_DO_WHILE** + Macros with multiple statements should be enclosed in a + do - while block. Same should also be the case for macros + starting with `if` to avoid logic defects:: + + #define macrofun(a, b, c) \ + do { \ + if (a == 5) \ + do_this(b, c); \ + } while (0) + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enu… + + **WEAK_DECLARATION** + Using weak declarations like __attribute__((weak)) or __weak + can have unintended link defects. Avoid using them. + + +Functions and Variables +----------------------- + + **CAMELCASE** + Avoid CamelCase Identifiers. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#naming + + **FUNCTION_WITHOUT_ARGS** + Function declarations without arguments like:: + + int foo() + + should be:: + + int foo(void) + + **GLOBAL_INITIALISERS** + Global variables should not be initialized explicitly to + 0 (or NULL, false, etc.). Your compiler (or rather your + loader, which is responsible for zeroing out the relevant + sections) automatically does it for you. + + **INITIALISED_STATIC** + Static variables should not be initialized explicitly to zero. + Your compiler (or rather your loader) automatically does + it for you. + + **RETURN_PARENTHESES** + return is not a function and as such doesn't need parentheses:: + + return (bar); + + can simply be:: + + return bar; + + +Spacing and Brackets +-------------------- + + **ASSIGNMENT_CONTINUATIONS** + Assignment operators should not be written at the start of a + line but should follow the operand at the previous line. + + **BRACES** + The placement of braces is stylistically incorrect. + The preferred way is to put the opening brace last on the line, + and put the closing brace first:: + + if (x is true) { + we do y + } + + This applies for all non-functional blocks. + However, there is one special case, namely functions: they have the + opening brace at the beginning of the next line, thus:: + + int function(int x) + { + body of function + } + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-br… + + **BRACKET_SPACE** + Whitespace before opening bracket '[' is prohibited. + There are some exceptions: + + 1. With a type on the left:: + + ;int [] a; + + 2. At the beginning of a line for slice initialisers:: + + [0...10] = 5, + + 3. Inside a curly brace:: + + = { [0...10] = 5 } + + **CODE_INDENT** + Code indent should use tabs instead of spaces. + Outside of comments, documentation and Kconfig, + spaces are never used for indentation. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation + + **CONCATENATED_STRING** + Concatenated elements should have a space in between. + Example:: + + printk(KERN_INFO"bar"); + + should be:: + + printk(KERN_INFO "bar"); + + **ELSE_AFTER_BRACE** + `else {` should follow the closing block `}` on the same line. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-br… + + **LINE_SPACING** + Vertical space is wasted given the limited number of lines an + editor window can display when multiple blank lines are used. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces + + **OPEN_BRACE** + The opening brace should be following the function definitions on the + next line. For any non-functional block it should be on the same line + as the last construct. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-br… + + **POINTER_LOCATION** + When using pointer data or a function that returns a pointer type, + the preferred use of * is adjacent to the data name or function name + and not adjacent to the type name. + Examples:: + + char *linux_banner; + unsigned long long memparse(char *ptr, char **retptr); + char *match_strdup(substring_t *s); + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces + + **SPACING** + Whitespace style used in the kernel sources is described in kernel docs. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces + + **SWITCH_CASE_INDENT_LEVEL** + switch should be at the same indent as case. + Example:: + + switch (suffix) { + case 'G': + case 'g': + mem <<= 30; + break; + case 'M': + case 'm': + mem <<= 20; + break; + case 'K': + case 'k': + mem <<= 10; + /* fall through */ + default: + break; + } + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation + + **TRAILING_WHITESPACE** + Trailing whitespace should always be removed. + Some editors highlight the trailing whitespace and cause visual + distractions when editing files. + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces + + **WHILE_AFTER_BRACE** + while should follow the closing bracket on the same line:: + + do { + ... + } while(something); + + See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-br… + + +Others +------ + + **CONFIG_DESCRIPTION** + Kconfig symbols should have a help text which fully describes + it. + + **CORRUPTED_PATCH** + The patch seems to be corrupted or lines are wrapped. + Please regenerate the patch file before sending it to the maintainer. + + **DOS_LINE_ENDINGS** + For DOS-formatted patches, there are extra ^M symbols at the end of + the line. These should be removed. + + **EXECUTE_PERMISSIONS** + There is no reason for source files to be executable. The executable + bit can be removed safely. + + **NON_OCTAL_PERMISSIONS** + Permission bits should use 4 digit octal permissions (like 0700 or 0444). + Avoid using any other base like decimal. + + **NOT_UNIFIED_DIFF** + The patch file does not appear to be in unified-diff format. Please + regenerate the patch file before sending it to the maintainer. + + **PRINTF_0XDECIMAL** + Prefixing 0x with decimal output is defective and should be corrected. + + **TRAILING_STATEMENTS** + Trailing statements (for example after any conditional) should be + on the next line. + Like:: + + if (x == y) break; + + should be:: + + if (x == y) + break; diff --git a/doc/develop/index.rst b/doc/develop/index.rst index 3ead7bda8fd7..63cdc6cd8997 100644 --- a/doc/develop/index.rst +++ b/doc/develop/index.rst @@ -52,5 +52,6 @@ Refactoring .. toctree:: :maxdepth: 1 + checkpatch coccinelle moveconfig diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 08a827535aaf..5696d3a5f3b4 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -23,6 +23,9 @@ my $V = '0.32'; use Getopt::Long qw(:config no_auto_abbrev); my $quiet = 0; +my $verbose = 0; +my %verbose_messages = (); +my %verbose_emitted = (); my $tree = 1; my $chk_signoff = 1; my $chk_patch = 1; @@ -43,6 +46,8 @@ my $list_types = 0; my $fix = 0; my $fix_inplace = 0; my $root; +my $gitroot = $ENV{'GIT_DIR'}; +$gitroot = ".git" if !defined($gitroot); my %debug; my %camelcase = (); my %use_type = (); @@ -59,13 +64,15 @@ 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 $u_boot = 0; +my $docsfile = "$D/../doc/develop/checkpatch.rst"; +my $typedefsfile; my $color = "auto"; my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE # git output parsing needs US English output, so first set backtick child process LANGUAGE my $git_command ='export LANGUAGE=en_US.UTF-8; git'; my $tabsize = 8; +my ${CONFIG_} = "CONFIG_"; sub help { my ($exitcode) = @_; @@ -76,6 +83,7 @@ Version: $V Options: -q, --quiet quiet + -v, --verbose verbose mode --no-tree run without a kernel tree --no-signoff do not check for 'Signed-off-by' line --patch treat FILE as patchfile (default) @@ -129,6 +137,8 @@ Options: --color[=WHEN] Use colors 'always', 'never', or only when output is a terminal ('auto'). Default is 'auto'. --u-boot Run additional checks for U-Boot + --kconfig-prefix=WORD use WORD as a prefix for Kconfig symbols (default + ${CONFIG_}) -h, --help, --version display this help and exit When FILE is - read standard input. @@ -155,15 +165,51 @@ sub list_types { my $text = <$script>; close($script); - my @types = (); + my %types = (); # Also catch when type or level is passed through a variable - for ($text =~ /(?:(?:\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) { - push (@types, $_); + while ($text =~ /(?:(\bCHK|\bWARN|\bERROR|&\{\$msg_level})\s*\(|\$msg_type\s*=)\s*"([^"]+)"/g) { + if (defined($1)) { + if (exists($types{$2})) { + $types{$2} .= ",$1" if ($types{$2} ne $1); + } else { + $types{$2} = $1; + } + } else { + $types{$2} = "UNDETERMINED"; + } } - @types = sort(uniq(@types)); + print("#\tMessage type\n\n"); - foreach my $type (@types) { + if ($color) { + print(" ( Color coding: "); + print(RED . "ERROR" . RESET); + print(" | "); + print(YELLOW . "WARNING" . RESET); + print(" | "); + print(GREEN . "CHECK" . RESET); + print(" | "); + print("Multiple levels / Undetermined"); + print(" )\n\n"); + } + + foreach my $type (sort keys %types) { + my $orig_type = $type; + if ($color) { + my $level = $types{$type}; + if ($level eq "ERROR") { + $type = RED . $type . RESET; + } elsif ($level eq "WARN") { + $type = YELLOW . $type . RESET; + } elsif ($level eq "CHK") { + $type = GREEN . $type . RESET; + } + } print(++$count . "\t" . $type . "\n"); + if ($verbose && exists($verbose_messages{$orig_type})) { + my $message = $verbose_messages{$orig_type}; + $message =~ s/\n/\n\t/g; + print("\t" . $message . "\n\n"); + } } exit($exitcode); @@ -195,6 +241,46 @@ if (-f $conf) { unshift(@ARGV, @conf_args) if @conf_args; } +sub load_docs { + open(my $docs, '<', "$docsfile") + or warn "$P: Can't read the documentation file $docsfile $!\n"; + + my $type = ''; + my $desc = ''; + my $in_desc = 0; + + while (<$docs>) { + chomp; + my $line = $_; + $line =~ s/\s+$//; + + if ($line =~ /^\s*\*\*(.+)\*\*$/) { + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + $type = $1; + $desc = ''; + $in_desc = 1; + } elsif ($in_desc) { + if ($line =~ /^(?:\s{4,}|$)/) { + $line =~ s/^\s{4}//; + $desc .= $line; + $desc .= "\n"; + } else { + $verbose_messages{$type} = trim($desc); + $type = ''; + $desc = ''; + $in_desc = 0; + } + } + } + + if ($desc ne '') { + $verbose_messages{$type} = trim($desc); + } + close($docs); +} + # Perl's Getopt::Long allows options to take optional arguments after a space. # Prevent --color by itself from consuming other arguments foreach (@ARGV) { @@ -205,6 +291,7 @@ foreach (@ARGV) { GetOptions( 'q|quiet+' => \$quiet, + 'v|verbose!' => \$verbose, 'tree!' => \$tree, 'signoff!' => \$chk_signoff, 'patch!' => \$chk_patch, @@ -238,12 +325,29 @@ GetOptions( 'color=s' => \$color, 'no-color' => \$color, #keep old behaviors of -nocolor 'nocolor' => \$color, #keep old behaviors of -nocolor + 'kconfig-prefix=s' => \${CONFIG_}, 'h|help' => \$help, 'version' => \$help ) or help(1); help(0) if ($help); +die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix)); +die "$P: --verbose cannot be used with --terse\n" if ($verbose && $terse); + +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 "$P: Invalid color mode: $color\n"; +} + +load_docs() if ($verbose); list_types(0) if ($list_types); $fix = 1 if ($fix_inplace); @@ -263,20 +367,8 @@ if ($#ARGV < 0) { 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"; -} - # skip TAB size 1 to avoid additional checks on $tabsize - 1 -die "Invalid TAB size: $tabsize\n" if ($tabsize < 2); +die "$P: Invalid TAB size: $tabsize\n" if ($tabsize < 2); sub hash_save_array_words { my ($hashRef, $arrayRef) = @_; @@ -377,6 +469,7 @@ our $InitAttribute = qr{$InitAttributeData|$InitAttributeConst|$InitAttributeIni # We need \b after 'init' otherwise 'initconst' will cause a false positive in a check our $Attribute = qr{ const| + volatile| __percpu| __nocast| __safe| @@ -483,7 +576,7 @@ our $logFunctions = qr{(?x: our $allocFunctions = qr{(?x: (?:(?:devm_)? - (?:kv|k|v)[czm]alloc(?:_node|_array)? | + (?:kv|k|v)[czm]alloc(?:_array)?(?:_node)? | kstrdup(?:_const)? | kmemdup(?:_nul)?) | (?:\w+)?alloc_skb(?:_ip_align)? | @@ -503,6 +596,88 @@ our $signature_tags = qr{(?xi: Cc: )}; +our $tracing_logging_tags = qr{(?xi: + [=-]*> | + <[=-]* | + \[ | + \] | + start | + called | + entered | + entry | + enter | + in | + inside | + here | + begin | + exit | + end | + done | + leave | + completed | + out | + return | + [\.\!:\s]* +)}; + +sub edit_distance_min { + my (@arr) = @_; + my $len = scalar @arr; + if ((scalar @arr) < 1) { + # if underflow, return + return; + } + my $min = $arr[0]; + for my $i (0 .. ($len-1)) { + if ($arr[$i] < $min) { + $min = $arr[$i]; + } + } + return $min; +} + +sub get_edit_distance { + my ($str1, $str2) = @_; + $str1 = lc($str1); + $str2 = lc($str2); + $str1 =~ s/-//g; + $str2 =~ s/-//g; + my $len1 = length($str1); + my $len2 = length($str2); + # two dimensional array storing minimum edit distance + my @distance; + for my $i (0 .. $len1) { + for my $j (0 .. $len2) { + if ($i == 0) { + $distance[$i][$j] = $j; + } elsif ($j == 0) { + $distance[$i][$j] = $i; + } elsif (substr($str1, $i-1, 1) eq substr($str2, $j-1, 1)) { + $distance[$i][$j] = $distance[$i - 1][$j - 1]; + } else { + my $dist1 = $distance[$i][$j - 1]; #insert distance + my $dist2 = $distance[$i - 1][$j]; # remove + my $dist3 = $distance[$i - 1][$j - 1]; #replace + $distance[$i][$j] = 1 + edit_distance_min($dist1, $dist2, $dist3); + } + } + } + return $distance[$len1][$len2]; +} + +sub find_standard_signature { + my ($sign_off) = @_; + my @standard_signature_tags = ( + 'Signed-off-by:', 'Co-developed-by:', 'Acked-by:', 'Tested-by:', + 'Reviewed-by:', 'Reported-by:', 'Suggested-by:' + ); + foreach my $signature (@standard_signature_tags) { + return $signature if (get_edit_distance($sign_off, $signature) <= 2); + } + + return ""; +} + our @typeListMisordered = ( qr{char\s+(?:un)?signed}, qr{int\s+(?:(?:un)?signed\s+)?short\s}, @@ -591,6 +766,8 @@ our @mode_permission_funcs = ( ["__ATTR", 2], ); +my $word_pattern = '\b[A-Z]?[a-z]{2,}\b'; + #Create a search pattern for all these functions to speed up a loop below our $mode_perms_search = ""; foreach my $entry (@mode_permission_funcs) { @@ -759,7 +936,7 @@ sub read_words { next; } - $$wordsRef .= '|' if ($$wordsRef ne ""); + $$wordsRef .= '|' if (defined $$wordsRef); $$wordsRef .= $line; } close($file); @@ -769,16 +946,18 @@ sub read_words { 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 $const_structs; +if (show_type("CONST_STRUCT")) { + read_words(\$const_structs, $conststructsfile) + or warn "No structs that should be const will be found - file '$conststructsfile': $!\n"; +} -my $typeOtherTypedefs = ""; -if (length($typedefsfile)) { +if (defined($typedefsfile)) { + my $typeOtherTypedefs; read_words(\$typeOtherTypedefs, $typedefsfile) or warn "No additional types will be considered - file '$typedefsfile': $!\n"; + $typeTypedefs .= '|' . $typeOtherTypedefs if (defined $typeOtherTypedefs); } -$typeTypedefs .= '|' . $typeOtherTypedefs if ($typeOtherTypedefs ne ""); sub build_types { my $mods = "(?x: \n" . join("|\n ", (@modifierList, @modifierListFile)) . "\n)"; @@ -843,10 +1022,16 @@ 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*\(| (?:SKCIPHER_REQUEST|SHASH_DESC|AHASH_REQUEST)_ON_STACK\s*\( )}; +our %allow_repeated_words = ( + add => '', + added => '', + bad => '', + be => '', +); + sub deparenthesize { my ($string) = @_; return "" if (!defined($string)); @@ -904,7 +1089,7 @@ sub is_maintained_obsolete { sub is_SPDX_License_valid { my ($license) = @_; - return 1 if (!$tree || which("python") eq "" || !(-e "$root/scripts/spdxcheck.py") || !(-e "$root/.git")); + return 1 if (!$tree || which("python") eq "" || !(-e "$root/scripts/spdxcheck.py") || !(-e "$gitroot")); my $root_path = abs_path($root); my $status = `cd "$root_path"; echo "$license" | python scripts/spdxcheck.py -`; @@ -922,7 +1107,7 @@ sub seed_camelcase_includes { $camelcase_seeded = 1; - if (-e ".git") { + if (-e "$gitroot") { my $git_last_include_commit = `${git_command} log --no-merges --pretty=format:"%h%n" -1 -- include`; chomp $git_last_include_commit; $camelcase_cache = ".checkpatch-camelcase.git.$git_last_include_commit"; @@ -950,7 +1135,7 @@ sub seed_camelcase_includes { return; } - if (-e ".git") { + if (-e "$gitroot") { $files = `${git_command} ls-files "include/*.h"`; @include_files = split('\n', $files); } @@ -970,10 +1155,20 @@ sub seed_camelcase_includes { } } +sub git_is_single_file { + my ($filename) = @_; + + return 0 if ((which("git") eq "") || !(-e "$gitroot")); + + my $output = `${git_command} ls-files -- $filename 2>/dev/null`; + my $count = $output =~ tr/\n//; + return $count eq 1 && $output =~ m{^${filename}$}; +} + sub git_commit_info { my ($commit, $id, $desc) = @_; - return ($id, $desc) if ((which("git") eq "") || !(-e ".git")); + return ($id, $desc) if ((which("git") eq "") || !(-e "$gitroot")); my $output = `${git_command} log --no-color --format='%H %s' -1 $commit 2>&1`; $output =~ s/^\s*//gm; @@ -1012,7 +1207,7 @@ 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"); +die "$P: No git repository found\n" if ($git && !-e "$gitroot"); if ($git) { my @commits = (); @@ -1043,6 +1238,9 @@ my $vname; $allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"}; for my $filename (@ARGV) { my $FILE; + my $is_git_file = git_is_single_file($filename); + my $oldfile = $file; + $file = 1 if ($is_git_file); if ($git) { open($FILE, '-|', "git format-patch -M --stdout -1 $filename") || die "$P: $filename: git format-patch failed - $!\n"; @@ -1065,6 +1263,7 @@ for my $filename (@ARGV) { while (<$FILE>) { chomp; push(@rawlines, $_); + $vname = qq("$1") if ($filename eq '-' && $_ =~ m/^Subject:\s+(.+)/i); } close($FILE); @@ -1086,6 +1285,7 @@ for my $filename (@ARGV) { @modifierListFile = (); @typeListFile = (); build_types(); + $file = $oldfile if ($is_git_file); } if (!$quiet) { @@ -1131,6 +1331,7 @@ sub parse_email { my ($formatted_email) = @_; my $name = ""; + my $quoted = ""; my $name_comment = ""; my $address = ""; my $comment = ""; @@ -1162,14 +1363,20 @@ sub parse_email { } } - $name = trim($name); - $name =~ s/^\"|\"$//g; - $name =~ s/(\s*$[^$]+\))\s*//; - if (defined($1)) { - $name_comment = trim($1); + # Extract comments from names excluding quoted parts + # "John D. (Doe)" - Do not extract + if ($name =~ s/\"(.+)\"//) { + $quoted = $1; + } + while ($name =~ s/\s*($balanced_parens)\s*/ /) { + $name_comment .= trim($1); } + $name =~ s/^[ \"]+|[ \"]+$//g; + $name = trim("$quoted $name"); + $address = trim($address); $address =~ s/^\<|\>$//g; + $comment = trim($comment); if ($name =~ /[^\w \-]/i) { ##has "must quote" chars $name =~ s/(?<!\\)"/\\"/g; ##escape quotes @@ -1180,25 +1387,30 @@ sub parse_email { } sub format_email { - my ($name, $address) = @_; + my ($name, $name_comment, $address, $comment) = @_; my $formatted_email; - $name = trim($name); - $name =~ s/^\"|\"$//g; + $name =~ s/^[ \"]+|[ \"]+$//g; $address = trim($address); + $address =~ s/(?:\.|\,|\")+$//; ##trailing commas, dots or quotes if ($name =~ /[^\w \-]/i) { ##has "must quote" chars $name =~ s/(?<!\\)"/\\"/g; ##escape quotes $name = "\"$name\""; } + $name_comment = trim($name_comment); + $name_comment = " $name_comment" if ($name_comment ne ""); + $comment = trim($comment); + $comment = " $comment" if ($comment ne ""); + if ("$name" eq "") { $formatted_email = "$address"; } else { - $formatted_email = "$name <$address>"; + $formatted_email = "$name$name_comment <$address>"; } - + $formatted_email .= "$comment"; return $formatted_email; } @@ -1206,7 +1418,7 @@ sub reformat_email { my ($email) = @_; my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); - return format_email($email_name, $email_address); + return format_email($email_name, $name_comment, $email_address, $comment); } sub same_email_addresses { @@ -1216,7 +1428,9 @@ sub same_email_addresses { my ($email2_name, $name2_comment, $email2_address, $comment2) = parse_email($email2); return $email1_name eq $email2_name && - $email1_address eq $email2_address; + $email1_address eq $email2_address && + $name1_comment eq $name2_comment && + $comment1 eq $comment2; } sub which { @@ -1681,8 +1895,16 @@ sub ctx_statement_level { sub ctx_locate_comment { my ($first_line, $end_line) = @_; + # If c99 comment on the current line, or the line before or after + my ($current_comment) = ($rawlines[$end_line - 1] =~ m@^\+.*(//.*$)@); + return $current_comment if (defined $current_comment); + ($current_comment) = ($rawlines[$end_line - 2] =~ m@^[\+ ].*(//.*$)@); + return $current_comment if (defined $current_comment); + ($current_comment) = ($rawlines[$end_line] =~ m@^[\+ ].*(//.*$)@); + return $current_comment if (defined $current_comment); + # Catch a comment on the end of the line itself. - my ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*(?:\\\s*)?$@); + ($current_comment) = ($rawlines[$end_line - 1] =~ m@.*(/\*.*\*/)\s*(?:\\\s*)?$@); return $current_comment if (defined $current_comment); # Look through the context and try and figure out if there is a @@ -2076,7 +2298,16 @@ sub report { splice(@lines, 1, 1); $output = join("\n", @lines); } - $output = (split('\n', $output))[0] . "\n" if ($terse); + + if ($terse) { + $output = (split('\n', $output))[0] . "\n"; + } + + if ($verbose && exists($verbose_messages{$type}) && + !exists($verbose_emitted{$type})) { + $output .= $verbose_messages{$type} . "\n\n"; + $verbose_emitted{$type} = 1; + } push(our @report, $output); @@ -2425,6 +2656,15 @@ sub u_boot_line { "DEVICE_PLAT_AUTO", $herecurr); } +sub exclude_global_initialisers { + my ($realfile) = @_; + + # Do not check for BPF programs (tools/testing/selftests/bpf/progs/*.c, samples/bpf/*_kern.c, *.bpf.c). + return $realfile =~ m@^tools/testing/selftests/bpf/progs/.*\.c$@ || + $realfile =~ m@^samples/bpf/.*_kern\.c$@ || + $realfile =~ m@/bpf/.*\.bpf\.c$@; +} + sub process { my $filename = shift; @@ -2443,6 +2683,7 @@ sub process { my $signoff = 0; my $author = ''; my $authorsignoff = 0; + my $author_sob = ''; my $is_patch = 0; my $is_binding_patch = -1; my $in_header_lines = $file ? 0 : 1; @@ -2506,7 +2747,7 @@ sub process { if ($rawline=~/^\+\+\+\s+(\S+)/) { $setup_docs = 0; - if ($1 =~ m@Documentation/admin-guide/kernel-parameters.rst$@) { + if ($1 =~ m@Documentation/admin-guide/kernel-parameters.txt$@) { $setup_docs = 1; } #next; @@ -2706,7 +2947,7 @@ sub process { if (($last_binding_patch != -1) && ($last_binding_patch ^ $is_binding_patch)) { WARN("DT_SPLIT_BINDING_PATCH", - "DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.txt\n"); + "DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst\n"); } } @@ -2735,8 +2976,8 @@ sub process { # 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+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", @@ -2757,6 +2998,10 @@ sub process { # Check the patch for a From: if (decode("MIME-Header", $line) =~ /^From:\s*(.*)/) { $author = $1; + my $curline = $linenr; + while(defined($rawlines[$curline]) && ($rawlines[$curline++] =~ /^[ \t]\s*(.*)/)) { + $author .= $1; + } $author = encode("utf8", $author) if ($line =~ /=\?utf-8\?/i); $author =~ s/"//g; $author = reformat_email($author); @@ -2766,9 +3011,37 @@ sub process { if ($line =~ /^\s*signed-off-by:\s*(.*)/i) { $signoff++; $in_commit_log = 0; - if ($author ne '') { + if ($author ne '' && $authorsignoff != 1) { if (same_email_addresses($1, $author)) { $authorsignoff = 1; + } else { + my $ctx = $1; + my ($email_name, $email_comment, $email_address, $comment1) = parse_email($ctx); + my ($author_name, $author_comment, $author_address, $comment2) = parse_email($author); + + if ($email_address eq $author_address && $email_name eq $author_name) { + $author_sob = $ctx; + $authorsignoff = 2; + } elsif ($email_address eq $author_address) { + $author_sob = $ctx; + $authorsignoff = 3; + } elsif ($email_name eq $author_name) { + $author_sob = $ctx; + $authorsignoff = 4; + + my $address1 = $email_address; + my $address2 = $author_address; + + if ($address1 =~ /(\S+)\+\S+(\@.*)/) { + $address1 = "$1$2"; + } + if ($address2 =~ /(\S+)\+\S+(\@.*)/) { + $address2 = "$1$2"; + } + if ($address1 eq $address2) { + $authorsignoff = 5; + } + } } } } @@ -2795,8 +3068,17 @@ sub process { my $ucfirst_sign_off = ucfirst(lc($sign_off)); if ($sign_off !~ /$signature_tags/) { - WARN("BAD_SIGN_OFF", - "Non-standard signature: $sign_off\n" . $herecurr); + my $suggested_signature = find_standard_signature($sign_off); + if ($suggested_signature eq "") { + WARN("BAD_SIGN_OFF", + "Non-standard signature: $sign_off\n" . $herecurr); + } else { + if (WARN("BAD_SIGN_OFF", + "Non-standard signature: '$sign_off' - perhaps '$suggested_signature'?\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/$sign_off/$suggested_signature/; + } + } } if (defined $space_before && $space_before ne "") { if (WARN("BAD_SIGN_OFF", @@ -2825,7 +3107,7 @@ sub process { } my ($email_name, $name_comment, $email_address, $comment) = parse_email($email); - my $suggested_email = format_email(($email_name, $email_address)); + my $suggested_email = format_email(($email_name, $name_comment, $email_address, $comment)); if ($suggested_email eq "") { ERROR("BAD_SIGN_OFF", "Unrecognized email address: '$email'\n" . $herecurr); @@ -2836,8 +3118,76 @@ sub process { # Don't force email to have quotes # Allow just an angle bracketed address if (!same_email_addresses($email, $suggested_email)) { + if (WARN("BAD_SIGN_OFF", + "email address '$email' might be better as '$suggested_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$suggested_email/; + } + } + + # Address part shouldn't have comments + my $stripped_address = $email_address; + $stripped_address =~ s/$[^\($]*\)//g; + if ($email_address ne $stripped_address) { + if (WARN("BAD_SIGN_OFF", + "address part of email should not have comments: '$email_address'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email_address\E/$stripped_address/; + } + } + + # Only one name comment should be allowed + my $comment_count = () = $name_comment =~ /$[^$]+\)/g; + if ($comment_count > 1) { WARN("BAD_SIGN_OFF", - "email address '$email' might be better as '$suggested_email$comment'\n" . $herecurr); + "Use a single name comment in email: '$email'\n" . $herecurr); + } + + + # stable(a)vger.kernel.org or stable(a)kernel.org shouldn't + # have an email name. In addition comments should strictly + # begin with a # + if ($email =~ /^.*stable\@(?:vger\.)?kernel\.org/i) { + if (($comment ne "" && $comment !~ /^#.+/) || + ($email_name ne "")) { + my $cur_name = $email_name; + my $new_comment = $comment; + $cur_name =~ s/[a-zA-Z\s\-\"]+//g; + + # Remove brackets enclosing comment text + # and # from start of comments to get comment text + $new_comment =~ s/^$(.*)$$/$1/; + $new_comment =~ s/^\[(.*)\]$/$1/; + $new_comment =~ s/^[\s\#]+|\s+$//g; + + $new_comment = trim("$new_comment $cur_name") if ($cur_name ne $new_comment); + $new_comment = " # $new_comment" if ($new_comment ne ""); + my $new_email = "$email_address$new_comment"; + + if (WARN("BAD_STABLE_ADDRESS_STYLE", + "Invalid email format for stable: '$email', prefer '$new_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/; + } + } + } elsif ($comment ne "" && $comment !~ /^(?:#.+|$.+$)$/) { + my $new_comment = $comment; + + # Extract comment text from within brackets or + # c89 style /*...*/ comments + $new_comment =~ s/^\[(.*)\]$/$1/; + $new_comment =~ s/^\/\*(.*)\*\/$/$1/; + + $new_comment = trim($new_comment); + $new_comment =~ s/^[^\w]$//; # Single lettered comment with non word character is usually a typo + $new_comment = "($new_comment)" if ($new_comment ne ""); + my $new_email = format_email($email_name, $name_comment, $email_address, $new_comment); + + if (WARN("BAD_SIGN_OFF", + "Unexpected content after email: '$email', should be: '$new_email'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\Q$email\E/$new_email/; + } } } @@ -2860,7 +3210,7 @@ sub process { } if (!defined $lines[$linenr]) { WARN("BAD_SIGN_OFF", - "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); + "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); } elsif ($rawlines[$linenr] !~ /^\s*signed-off-by:\s*(.*)/i) { WARN("BAD_SIGN_OFF", "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); @@ -2880,8 +3230,11 @@ sub process { # Check for Gerrit Change-Ids not in any patch context if ($realfile eq '' && !$has_patch_separator && $line =~ /^\s*change-id:/i) { - ERROR("GERRIT_CHANGE_ID", - "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr); + if (ERROR("GERRIT_CHANGE_ID", + "Remove Gerrit Change-Id's before submitting upstream\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } } # Check if the commit log is in a possible stack dump @@ -2903,8 +3256,8 @@ sub process { # file delta changes $line =~ /^\s*(?:[\w\.\-]+\/)++[\w\.\-]+:/ || # filename then : - $line =~ /^\s*(?:Fixes:|Link:)/i || - # A Fixes: or Link: line + $line =~ /^\s*(?:Fixes:|Link:|$signature_tags)/i || + # A Fixes: or Link: line or signature tag line $commit_log_possible_stack_dump)) { WARN("COMMIT_LOG_LONG_LINE", "Possible unwrapped commit description (prefer a maximum 75 chars per line)\n" . $herecurr); @@ -2917,6 +3270,15 @@ sub process { $commit_log_possible_stack_dump = 0; } +# Check for lines starting with a # + if ($in_commit_log && $line =~ /^#/) { + if (WARN("COMMIT_COMMENT_SYMBOL", + "Commit log lines starting with '#' are dropped by git as comments\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/^/ /; + } + } + # 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|base-commit):/i && @@ -2993,7 +3355,7 @@ sub process { ($line =~ /^new file mode\s*\d+\s*$/) && ($realfile =~ m@^Documentation/devicetree/bindings/.*\.txt$@)) { WARN("DT_SCHEMA_BINDING_PATCH", - "DT bindings should be in DT schema format. See: Documentation/devicetree/writing-schema.rst\n"); + "DT bindings should be in DT schema format. See: Documentation/devicetree/bindings/writing-schema.rst\n"); } # Check for wrappage within a valid hunk of the file @@ -3057,15 +3419,18 @@ sub process { # Check for various typo / spelling mistakes if (defined($misspellings) && ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) { - while ($rawline =~ /(?:^|[^a-z@])($misspellings)(?:\b|$|[^a-z@])/gi) { + while ($rawline =~ /(?:^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) { my $typo = $1; + my $blank = copy_spacing($rawline); + my $ptr = substr($blank, 0, $-[1]) . "^" x length($typo); + my $hereptr = "$hereline$ptr\n"; my $typo_fix = $spelling_fix{lc($typo)}; $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/); $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/); my $msg_level = \&WARN; $msg_level = \&CHK if ($file); if (&{$msg_level}("TYPO_SPELLING", - "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $herecurr) && + "'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $hereptr) && $fix) { $fixed[$fixlinenr] =~ s/(^|[^A-Za-z@])($typo)($|[^A-Za-z@])/$1$typo_fix$3/; } @@ -3083,6 +3448,60 @@ sub process { } } +# check for repeated words separated by a single space +# avoid false positive from list command eg, '-rw-r--r-- 1 root root' + if (($rawline =~ /^\+/ || $in_commit_log) && + $rawline !~ /[bcCdDlMnpPs\?-][rwxsStT-]{9}/) { + pos($rawline) = 1 if (!$in_commit_log); + while ($rawline =~ /\b($word_pattern) (?=($word_pattern))/g) { + + my $first = $1; + my $second = $2; + my $start_pos = $-[1]; + my $end_pos = $+[2]; + if ($first =~ /(?:struct|union|enum)/) { + pos($rawline) += length($first) + length($second) + 1; + next; + } + + next if (lc($first) ne lc($second)); + next if ($first eq 'long'); + + # check for character before and after the word matches + my $start_char = ''; + my $end_char = ''; + $start_char = substr($rawline, $start_pos - 1, 1) if ($start_pos > ($in_commit_log ? 0 : 1)); + $end_char = substr($rawline, $end_pos, 1) if ($end_pos < length($rawline)); + + next if ($start_char =~ /^\S$/); + next if (index(" \t.,;?!", $end_char) == -1); + + # avoid repeating hex occurrences like 'ff ff fe 09 ...' + if ($first =~ /\b[0-9a-f]{2,}\b/i) { + next if (!exists($allow_repeated_words{lc($first)})); + } + + if (WARN("REPEATED_WORD", + "Possible repeated word: '$first'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\b$first $second\b/$first/; + } + } + + # if it's a repeated word on consecutive lines in a comment block + if ($prevline =~ /$;+\s*$/ && + $prevrawline =~ /($word_pattern)\s*$/) { + my $last_word = $1; + if ($rawline =~ /^\+\s*\*\s*$last_word /) { + if (WARN("REPEATED_WORD", + "Possible repeated word: '$last_word'\n" . $hereprev) && + $fix) { + $fixed[$fixlinenr] =~ s/(\+\s*\*\s*)$last_word /$1/; + } + } + } + } + # ignore non-hunk lines and lines being removed next if (!$hunk_line || $line =~ /^-/); @@ -3141,11 +3560,7 @@ sub process { if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { $is_start = 1; - } elsif ($lines[$ln - 1] =~ /^\+\s*(?:help|---help---)\s*$/) { - if ($lines[$ln - 1] =~ "---help---") { - WARN("CONFIG_DESCRIPTION", - "prefer 'help' over '---help---' for new help texts\n" . $herecurr); - } + } elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) { $length = -1; } @@ -3172,22 +3587,44 @@ sub process { #print "is_start<$is_start> is_end<$is_end> length<$length>\n"; } -# 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/; +# check MAINTAINERS entries + if ($realfile =~ /^MAINTAINERS$/) { +# check MAINTAINERS entries for the right form + if ($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/; + } + } +# check MAINTAINERS entries for the right ordering too + my $preferred_order = 'MRLSWQBCPTFXNK'; + if ($rawline =~ /^\+[A-Z]:/ && + $prevrawline =~ /^[\+ ][A-Z]:/) { + $rawline =~ /^\+([A-Z]):\s*(.*)/; + my $cur = $1; + my $curval = $2; + $prevrawline =~ /^[\+ ]([A-Z]):\s*(.*)/; + my $prev = $1; + my $prevval = $2; + my $curindex = index($preferred_order, $cur); + my $previndex = index($preferred_order, $prev); + if ($curindex < 0) { + WARN("MAINTAINERS_STYLE", + "Unknown MAINTAINERS entry type: '$cur'\n" . $herecurr); + } else { + if ($previndex >= 0 && $curindex < $previndex) { + WARN("MAINTAINERS_STYLE", + "Misordered MAINTAINERS entry - list '$cur:' before '$prev:'\n" . $hereprev); + } elsif ((($prev eq 'F' && $cur eq 'F') || + ($prev eq 'X' && $cur eq 'X')) && + ($prevval cmp $curval) > 0) { + WARN("MAINTAINERS_STYLE", + "Misordered MAINTAINERS entry - list file patterns in alphabetic order\n" . $hereprev); + } + } } - } - -# discourage the use of boolean for type definition attributes of Kconfig options - if ($realfile =~ /Kconfig/ && - $line =~ /^\+\s*\bboolean\b/) { - WARN("CONFIG_TYPE_BOOLEAN", - "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); } if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && @@ -3284,6 +3721,12 @@ sub process { } } +# check for embedded filenames + if ($rawline =~ /^\+.*\Q$realfile\E/) { + WARN("EMBEDDED_FILENAME", + "It's generally not useful to have the filename in the file\n" . $herecurr); + } + # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /\.(h|c|s|S|sh|dtsi|dts)$/); @@ -3361,8 +3804,18 @@ sub process { # check for adding lines without a newline. if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) { - WARN("MISSING_EOF_NEWLINE", - "adding a line without newline at end of file\n" . $herecurr); + if (WARN("MISSING_EOF_NEWLINE", + "adding a line without newline at end of file\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr+1, "No newline at end of file"); + } + } + +# check for .L prefix local symbols in .S files + if ($realfile =~ /\.S$/ && + $line =~ /^\+\s*(?:[A-Z]+_)?SYM_[A-Z]+_(?:START|END)(?:_[A-Z_]+)?\s*\(\s*\.L/) { + WARN("AVOID_L_PREFIX", + "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/asm-annotations.rst\n" . $herecurr); } if ($u_boot) { @@ -3400,14 +3853,28 @@ sub process { # check for assignments on the start of a line if ($sline =~ /^\+\s+($Assignment)[^=]/) { - CHK("ASSIGNMENT_CONTINUATIONS", - "Assignment operator '$1' should be on the previous line\n" . $hereprev); + my $operator = $1; + if (CHK("ASSIGNMENT_CONTINUATIONS", + "Assignment operator '$1' should be on the previous line\n" . $hereprev) && + $fix && $prevrawline =~ /^\+/) { + # add assignment operator to the previous line, remove from current line + $fixed[$fixlinenr - 1] .= " $operator"; + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//; + } } # check for && or || at the start of a line if ($rawline =~ /^\+\s*(&&|\|\|)/) { - CHK("LOGICAL_CONTINUATIONS", - "Logical continuations should be on the previous line\n" . $hereprev); + my $operator = $1; + if (CHK("LOGICAL_CONTINUATIONS", + "Logical continuations should be on the previous line\n" . $hereprev) && + $fix && $prevrawline =~ /^\+/) { + # insert logical operator at last non-comment, non-whitepsace char on previous line + $prevline =~ /[\s$;]*$/; + my $line_end = substr($prevrawline, $-[0]); + $fixed[$fixlinenr - 1] =~ s/\Q$line_end\E$/ $operator$line_end/; + $fixed[$fixlinenr] =~ s/\Q$operator\E\s*//; + } } # check indentation starts on a tab stop @@ -3475,7 +3942,7 @@ sub process { if ($realfile =~ m@^(drivers/net/|net/)@ && $prevrawline =~ /^\+[ \t]*\/\*[ \t]*$/ && $rawline =~ /^\+[ \t]*\*/ && - $realline > 2) { + $realline > 3) { # Do not warn about the initial copyright comment block after SPDX-License-Identifier WARN("NETWORKING_BLOCK_COMMENT_STYLE", "networking block comments don't use an empty /* line, use /* Comment...\n" . $hereprev); } @@ -3557,43 +4024,48 @@ sub process { } # check for missing blank lines after declarations - if ($sline =~ /^\+\s+\S/ && #Not at char 1 - # actual declarations - ($prevline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || +# (declarations must have the same indentation and not be at the start of line) + if (($prevline =~ /\+(\s+)\S/) && $sline =~ /^\+$1\S/) { + # use temporaries + my $sl = $sline; + my $pl = $prevline; + # remove $Attribute/$Sparse uses to simplify comparisons + $sl =~ s/\b(?:$Attribute|$Sparse)\b//g; + $pl =~ s/\b(?:$Attribute|$Sparse)\b//g; + if (($pl =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || # function pointer declarations - $prevline =~ /^\+\s+$Declare\s*$\s*\*\s*$Ident\s*$\s*[=,;:\[$]/ || + $pl =~ /^\+\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*[=,;\[]/ || + $pl =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || # known declaration macros - $prevline =~ /^\+\s+$declaration_macros/) && + $pl =~ /^\+\s+$declaration_macros/) && # for "else if" which can look like "$Ident $Ident" - !($prevline =~ /^\+\s+$c90_Keywords\b/ || + !($pl =~ /^\+\s+$c90_Keywords\b/ || # other possible extensions of declaration lines - $prevline =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || + $pl =~ /(?:$Compare|$Assignment|$Operators)\s*$/ || # not starting a section or a macro "\" extended line - $prevline =~ /(?:\{\s*|\$$/) && + $pl =~ /(?:\{\s*|\\)$/) && # looks like a declaration - !($sline =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || + !($sl =~ /^\+\s+$Declare\s*$Ident\s*[=,;:\[]/ || # function pointer declarations - $sline =~ /^\+\s+$Declare\s*$\s*\*\s*$Ident\s*$\s*[=,;:\[$]/ || + $sl =~ /^\+\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*[=,;\[]/ || + $sl =~ /^\+\s+$Ident(?:\s+|\s*\*\s*)$Ident\s*[=,;\[]/ || # known declaration macros - $sline =~ /^\+\s+$declaration_macros/ || + $sl =~ /^\+\s+$declaration_macros/ || # start of struct or union or enum - $sline =~ /^\+\s+(?:static\s+)?(?:const\s+)?(?:union|struct|enum|typedef)\b/ || + $sl =~ /^\+\s+(?:static\s+)?(?:const\s+)?(?:union|struct|enum|typedef)\b/ || # start or end of block or continuation of declaration - $sline =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || + $sl =~ /^\+\s+(?:$|[\{\}\.\#\"\?\:\(\[])/ || # bitfield continuation - $sline =~ /^\+\s+$Ident\s*:\s*\d+\s*[,;]/ || + $sl =~ /^\+\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, "\+"); + $sl =~ /^\+\s+\(?\s*(?:$Compare|$Assignment|$Operators)/)) { + if (WARN("LINE_SPACING", + "Missing a blank line after declarations\n" . $hereprev) && + $fix) { + fix_insert_line($fixlinenr, "\+"); + } } } @@ -3646,12 +4118,16 @@ sub process { } # 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 the previous line is a goto, return or break +# 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); + if ($prevline =~ /^\+$tabs(goto|return|break)\b/) { + if (WARN("UNNECESSARY_BREAK", + "break is not useful after a $1\n" . $hereprev) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } } } @@ -3934,6 +4410,17 @@ sub process { #ignore lines not being added next if ($line =~ /^[^\+]/); +# check for self assignments used to avoid compiler warnings +# e.g.: int foo = foo, *bar = NULL; +# struct foo bar = *(&(bar)); + if ($line =~ /^\+\s*(?:$Declare)?([A-Za-z_][A-Za-z\d_]*)\s*=/) { + my $var = $1; + if ($line =~ /^\+\s*(?:$Declare)?$var\s*=\s*(?:$var|\*\s*$?\s*&\s*\(?\s*$var\s*$?\s*\)?)\s*[;,]/) { + WARN("SELF_ASSIGNMENT", + "Do not use self-assignments to avoid compiler warnings\n" . $herecurr); + } + } + # check for dereferences that span multiple lines if ($prevline =~ /^\+.*$Lval\s*(?:\.|->)\s*$/ && $line =~ /^\+\s*(?!\#\s*(?!define\s+|if))\s*$Lval/) { @@ -4049,8 +4536,7 @@ sub process { if (defined $realline_next && exists $lines[$realline_next - 1] && !defined $suppress_export{$realline_next} && - ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*$(.*)$/ || - $lines[$realline_next - 1] =~ /EXPORT_UNUSED_SYMBOL.*$(.*)$/)) { + ($lines[$realline_next - 1] =~ /EXPORT_SYMBOL.*$(.*)$/)) { # Handle definitions which produce identifiers with # a prefix: # XXX(foo); @@ -4077,8 +4563,7 @@ sub process { } if (!defined $suppress_export{$linenr} && $prevline =~ /^.\s*$/ && - ($line =~ /EXPORT_SYMBOL.*$(.*)$/ || - $line =~ /EXPORT_UNUSED_SYMBOL.*$(.*)$/)) { + ($line =~ /EXPORT_SYMBOL.*$(.*)$/)) { #print "FOO B <$lines[$linenr - 1]>\n"; $suppress_export{$linenr} = 2; } @@ -4089,7 +4574,8 @@ sub process { } # check for global initialisers. - if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) { + if ($line =~ /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/ && + !exclude_global_initialisers($realfile)) { if (ERROR("GLOBAL_INITIALISERS", "do not initialise globals to $1\n" . $herecurr) && $fix) { @@ -4168,12 +4654,24 @@ sub process { } } +# check for const static or static <non ptr type> const declarations +# prefer 'static const <foo>' over 'const static <foo>' and 'static <foo> const' + if ($sline =~ /^\+\s*const\s+static\s+($Type)\b/ || + $sline =~ /^\+\s*static\s+($BasicType)\s+const\b/) { + if (WARN("STATIC_CONST", + "Move const after static - use 'static const $1'\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bconst\s+static\b/static const/; + $fixed[$fixlinenr] =~ s/\bstatic\s+($BasicType)\s+const\b/static const $1/; + } + } + # 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*$@) { @@ -4290,16 +4788,23 @@ sub process { "printk() should include KERN_<LEVEL> facility level\n" . $herecurr); } - if ($line =~ /\bprintk\s*\(\s*KERN_([A-Z]+)/) { - my $orig = $1; +# prefer variants of (subsystem|netdev|dev|pr)_<level> to printk(KERN_<LEVEL> + if ($line =~ /\b(printk(_once|_ratelimited)?)\s*\(\s*KERN_([A-Z]+)/) { + my $printk = $1; + my $modifier = $2; + my $orig = $3; + $modifier = "" if (!defined($modifier)); my $level = lc($orig); $level = "warn" if ($level eq "warning"); my $level2 = $level; $level2 = "dbg" if ($level eq "debug"); + $level .= $modifier; + $level2 .= $modifier; WARN("PREFER_PR_LEVEL", - "Prefer [subsystem eg: netdev]_$level2([subsystem]dev, ... 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); } +# prefer dev_<level> to dev_printk(KERN_<LEVEL> if ($line =~ /\bdev_printk\s*\(\s*KERN_([A-Z]+)/) { my $orig = $1; my $level = lc($orig); @@ -4309,6 +4814,12 @@ sub process { "Prefer dev_$level(... to dev_printk(KERN_$orig, ...\n" . $herecurr); } +# trace_printk should not be used in production code. + if ($line =~ /\b(trace_printk|trace_puts|ftrace_vprintk)\s*\(/) { + WARN("TRACE_PRINTK", + "Do not use $1() in production code (this can be ignored if built only with a debug config option)\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. @@ -4317,6 +4828,17 @@ sub process { "ENOSYS means 'invalid syscall nr' and nothing else\n" . $herecurr); } +# ENOTSUPP is not a standard error code and should be avoided in new patches. +# Folks usually mean EOPNOTSUPP (also called ENOTSUP), when they type ENOTSUPP. +# Similarly to ENOSYS warning a small number of false positives is expected. + if (!$file && $line =~ /\bENOTSUPP\b/) { + if (WARN("ENOTSUPP", + "ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP\n" . $herecurr) && + $fix) { + $fixed[$fixlinenr] =~ s/\bENOTSUPP\b/EOPNOTSUPP/; + } + } + # function brace can't be on same line, except for #defines of do while, # or if closed on same line if ($perl_version_ok && @@ -4328,7 +4850,7 @@ sub process { $fix) { fix_delete_line($fixlinenr, $rawline); my $fixed_line = $rawline; - $fixed_line =~ /(^..*$Type\s*$Ident$.*$\s*){(.*)$/; + $fixed_line =~ /(^..*$Type\s*$Ident$.*$\s*)\{(.*)$/; my $line1 = $1; my $line2 = $2; fix_insert_line($fixlinenr, ltrim($line1)); @@ -4739,7 +5261,7 @@ sub process { # A colon needs no spaces before when it is # terminating a case value or a label. } elsif ($opv eq ':C' || $opv eq ':L') { - if ($ctx =~ /Wx./) { + if ($ctx =~ /Wx./ and $realfile !~ m@.*\.lds\.h$@) { if (ERROR("SPACING", "space prohibited before that '$op' $at\n" . $hereptr)) { $good = rtrim($fix_elements[$n]) . trim($fix_elements[$n + 1]); @@ -4823,7 +5345,7 @@ sub process { ## $line !~ /^.\s*$Type\s+$Ident(?:\s*=[^,{]*)?\s*,\s*$Type\s*$Ident.*/) { ## ## # Remove any bracketed sections to ensure we do not -## # falsly report the parameters of functions. +## # falsely report the parameters of functions. ## my $ln = $line; ## while ($ln =~ s/$[^\($]*\)//g) { ## } @@ -4964,6 +5486,17 @@ sub process { } } +# check if a statement with a comma should be two statements like: +# foo = bar(), /* comma should be semicolon */ +# bar = baz(); + if (defined($stat) && + $stat =~ /^\+\s*(?:$Lval\s*$Assignment\s*)?$FuncArg\s*,\s*(?:$Lval\s*$Assignment\s*)?$FuncArg\s*;\s*$/) { + my $cnt = statement_rawlines($stat); + my $herectx = get_stat_here($linenr, $cnt, $here); + WARN("SUSPECT_COMMA_SEMICOLON", + "Possible comma where semicolon could be used\n" . $herectx); + } + # return is not a function if (defined($stat) && $stat =~ /^.\s*return(\s*)$/s) { my $spacing = $1; @@ -4991,7 +5524,7 @@ sub process { $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 ($perl_version_ok && @@ -5084,8 +5617,30 @@ sub process { my ($s, $c) = ($stat, $cond); if ($c =~ /\bif\s*\(.*[^<>!=]=[^=].*/s) { - ERROR("ASSIGN_IN_IF", - "do not use assignment in if condition\n" . $herecurr); + if (ERROR("ASSIGN_IN_IF", + "do not use assignment in if condition\n" . $herecurr) && + $fix && $perl_version_ok) { + if ($rawline =~ /^\+(\s+)if\s*\(\s*(\!)?\s*\(\s*(($Lval)\s*=\s*$LvalOrFunc)\s*$\s*(?:($Compare)\s*($FuncArg))?\s*\)\s*(\{)?\s*$/) { + my $space = $1; + my $not = $2; + my $statement = $3; + my $assigned = $4; + my $test = $8; + my $against = $9; + my $brace = $15; + fix_delete_line($fixlinenr, $rawline); + fix_insert_line($fixlinenr, "$space$statement;"); + my $newline = "${space}if ("; + $newline .= '!' if defined($not); + $newline .= '(' if (defined $not && defined($test) && defined($against)); + $newline .= "$assigned"; + $newline .= " $test $against" if (defined($test) && defined($against)); + $newline .= ')' if (defined $not && defined($test) && defined($against)); + $newline .= ')'; + $newline .= " {" if (defined($brace)); + fix_insert_line($fixlinenr + 1, $newline); + } + } } # Find out what is on the end of the line after the @@ -5206,6 +5761,8 @@ sub process { #CamelCase if ($var !~ /^$Constant$/ && $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && +#Ignore some autogenerated defines and enum values + $var !~ /^(?:[A-Z]+_){1,5}[A-Z]{1,3}[a-z]/ && #Ignore Page<foo> variants $var !~ /^(?:Clear|Set|TestClear|TestSet|)Page[A-Z]/ && #Ignore SI style variants like nS, mV and dB @@ -5301,9 +5858,9 @@ sub process { $dstat =~ s/\s*$//s; # Flatten any parentheses and braces - while ($dstat =~ s/$[^\($]*\)/1/ || - $dstat =~ s/\{[^\{\}]*\}/1/ || - $dstat =~ s/.\[[^\[\]]*\]/1/) + while ($dstat =~ s/$[^\($]*\)/1u/ || + $dstat =~ s/\{[^\{\}]*\}/1u/ || + $dstat =~ s/.\[[^\[\]]*\]/1u/) { } @@ -5344,6 +5901,7 @@ sub process { $dstat !~ /^\.$Ident\s*=/ && # .foo = $dstat !~ /^(?:\#\s*$Ident|\#\s*$Constant)\s*$/ && # stringification #foo $dstat !~ /^do\s*$Constant\s*while\s*$Constant;?$/ && # do {...} while (...); // do {...} while (...) + $dstat !~ /^while\s*$Constant\s*$Constant\s*$/ && # while (...) {...} $dstat !~ /^for\s*$Constant$/ && # for (...) $dstat !~ /^for\s*$Constant\s+(?:$Ident|-?$Constant)$/ && # for (...) bar() $dstat !~ /^do\s*{/ && # do {... @@ -5385,7 +5943,7 @@ sub process { next if ($arg =~ /\.\.\./); next if ($arg =~ /^type$/i); my $tmp_stmt = $define_stmt; - $tmp_stmt =~ s/\b(sizeof|typeof|__typeof__|__builtin\w+|typecheck\s*$\s*$Type\s*,|\#+)\s*\(*\s*$arg\s*$*\b//g; + $tmp_stmt =~ s/\b(__must_be_array|offsetof|sizeof|sizeof_field|__stringify|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 =~ /\b$arg\b/g; @@ -5662,6 +6220,17 @@ sub process { "Prefer using '\"%s...\", __func__' to using '$context_function', this function's name, in a string\n" . $herecurr); } +# check for unnecessary function tracing like uses +# This does not use $logFunctions because there are many instances like +# 'dprintk(FOO, "%s()\n", __func__);' which do not match $logFunctions + if ($rawline =~ /^\+.*$[^"]*"$tracing_logging_tags{0,3}%s(?:\s*\(\s*$\s*)?$tracing_logging_tags{0,3}(?:\\n)?"\s*,\s*__func__\s*\)\s*;/) { + if (WARN("TRACING_LOGGING", + "Unnecessary ftrace-like logging - prefer using ftrace\n" . $herecurr) && + $fix) { + fix_delete_line($fixlinenr, $rawline); + } + } + # check for spaces before a quoted newline if ($rawline =~ /^.*\".*\s\\n/) { if (WARN("QUOTED_WHITESPACE_BEFORE_NEWLINE", @@ -5808,6 +6377,28 @@ sub process { "Avoid logging continuation uses where feasible\n" . $herecurr); } +# check for unnecessary use of %h[xudi] and %hh[xudi] in logging functions + if (defined $stat && + $line =~ /\b$logFunctions\s*\(/ && + index($stat, '"') >= 0) { + my $lc = $stat =~ tr@\n@@; + $lc = $lc + $linenr; + my $stat_real = get_stat_real($linenr, $lc); + pos($stat_real) = index($stat_real, '"'); + while ($stat_real =~ /[^\"%]*(%[\#\d\.\*\-]*(h+)[idux])/g) { + my $pspec = $1; + my $h = $2; + my $lineoff = substr($stat_real, 0, $-[1]) =~ tr@\n@@; + if (WARN("UNNECESSARY_MODIFIER", + "Integer promotion: Using '$h' in '$pspec' is unnecessary\n" . "$here\n$stat_real\n") && + $fix && $fixed[$fixlinenr + $lineoff] =~ /^\+/) { + my $nspec = $pspec; + $nspec =~ s/h//g; + $fixed[$fixlinenr + $lineoff] =~ s/\Q$pspec\E/$nspec/; + } + } + } + # check for mask then right shift without a parentheses if ($perl_version_ok && $line =~ /$LvalOrFunc\s*\&\s*($LvalOrFunc)\s*>>/ && @@ -5966,8 +6557,7 @@ sub process { my $barriers = qr{ mb| rmb| - wmb| - read_barrier_depends + wmb }x; my $barrier_stems = qr{ mb__before_atomic| @@ -6008,10 +6598,12 @@ sub process { } } -# check for smp_read_barrier_depends and read_barrier_depends - if (!$file && $line =~ /\b(smp_|)read_barrier_depends\s*\(/) { - WARN("READ_BARRIER_DEPENDS", - "$1read_barrier_depends should only be used in READ_ONCE or DEC Alpha code\n" . $herecurr); +# check for data_race without a comment. + if ($line =~ /\bdata_race\s*\(/) { + if (!ctx_has_comment($first_line, $linenr)) { + WARN("DATA_RACE", + "data_race without comment\n" . $herecurr); + } } # check of hardware specific defines @@ -6053,50 +6645,68 @@ sub process { } } -# Check for __attribute__ packed, prefer __packed - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*$\s*\(.*\bpacked\b/) { - WARN("PREFER_PACKED", - "__packed is preferred over __attribute__((packed))\n" . $herecurr); - } - -# Check for __attribute__ aligned, prefer __aligned - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*\(\s*\(.*aligned/) { - WARN("PREFER_ALIGNED", - "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr); - } - -# Check for __attribute__ section, prefer __section - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*\(\s*\(.*_*section_*\s*\(\s*("[^"]*")/) { - my $old = substr($rawline, $-[1], $+[1] - $-[1]); - my $new = substr($old, 1, -1); - if (WARN("PREFER_SECTION", - "__section(\"$new\") is preferred over __attribute__((section($old)))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*_*section_*\s*\(\s*\Q$old\E\s*$\s*\)\s*\)/__section($new)/; - } - } - -# Check for __attribute__ format(printf, prefer __printf +# Check for compiler attributes if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*$\s*\(\s*format\s*\(\s*printf/) { - 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[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*printf\s*,\s*(.*)$\s*\)\s*\)/"__printf(" . trim($1) . ")"/ex; - + $rawline =~ /\b__attribute__\s*$\s*($balanced_parens)\s*$/) { + my $attr = $1; + $attr =~ s/\s*$\s*(.*)$\s*/$1/; + + my %attr_list = ( + "alias" => "__alias", + "aligned" => "__aligned", + "always_inline" => "__always_inline", + "assume_aligned" => "__assume_aligned", + "cold" => "__cold", + "const" => "__attribute_const__", + "copy" => "__copy", + "designated_init" => "__designated_init", + "externally_visible" => "__visible", + "format" => "printf|scanf", + "gnu_inline" => "__gnu_inline", + "malloc" => "__malloc", + "mode" => "__mode", + "no_caller_saved_registers" => "__no_caller_saved_registers", + "noclone" => "__noclone", + "noinline" => "noinline", + "nonstring" => "__nonstring", + "noreturn" => "__noreturn", + "packed" => "__packed", + "pure" => "__pure", + "section" => "__section", + "used" => "__used", + "weak" => "__weak" + ); + + while ($attr =~ /\s*(\w+)\s*(${balanced_parens})?/g) { + my $orig_attr = $1; + my $params = ''; + $params = $2 if defined($2); + my $curr_attr = $orig_attr; + $curr_attr =~ s/^[\s_]+|[\s_]+$//g; + if (exists($attr_list{$curr_attr})) { + my $new = $attr_list{$curr_attr}; + if ($curr_attr eq "format" && $params) { + $params =~ /^\s*$\s*(\w+)\s*,\s*(.*)/; + $new = "__$1\($2"; + } else { + $new = "$new$params"; + } + if (WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "Prefer $new over __attribute__(($orig_attr$params))\n" . $herecurr) && + $fix) { + my $remove = "\Q$orig_attr\E" . '\s*' . "\Q$params\E" . '(?:\s*,\s*)?'; + $fixed[$fixlinenr] =~ s/$remove//; + $fixed[$fixlinenr] =~ s/\b__attribute__/$new __attribute__/; + $fixed[$fixlinenr] =~ s/\}\Q$new\E/} $new/; + $fixed[$fixlinenr] =~ s/ __attribute__\s*\(\s*\(\s*$\s*\)//; + } + } } - } -# Check for __attribute__ format(scanf, prefer __scanf - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*$\s*\(\s*format\s*\(\s*scanf\b/) { - 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[$fixlinenr] =~ s/\b__attribute__\s*\(\s*\(\s*format\s*\(\s*scanf\s*,\s*(.*)$\s*\)\s*\)/"__scanf(" . trim($1) . ")"/ex; + # Check for __attribute__ unused, prefer __always_unused or __maybe_unused + if ($attr =~ /^_*unused/) { + WARN("PREFER_DEFINED_ATTRIBUTE_MACRO", + "__always_unused or __maybe_unused is preferred over __attribute__((__unused__))\n" . $herecurr); } } @@ -6132,18 +6742,18 @@ sub process { if ($line =~ /($\s*$C90_int_types\s*$\s*)($Constant)\b/) { my $cast = $1; my $const = $2; + 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'; + } if (WARN("TYPECAST_INT_CONSTANT", - "Unnecessary typecast of c90 int constant\n" . $herecurr) && + "Unnecessary typecast of c90 int constant - '$cast$const' could be '$const$suffix'\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/; } } @@ -6203,9 +6813,11 @@ sub process { $specifier = $1; $extension = $2; $qualifier = $3; - if ($extension !~ /[SsBKRraEehMmIiUDdgVCbGNOxtf]/ || + if ($extension !~ /[4SsBKRraEehMmIiUDdgVCbGNOxtf]/ || ($extension eq "f" && - defined $qualifier && $qualifier !~ /^w/)) { + defined $qualifier && $qualifier !~ /^w/) || + ($extension eq "4" && + defined $qualifier && $qualifier !~ /^cc/)) { $bad_specifier = $specifier; last; } @@ -6385,8 +6997,7 @@ sub process { if (defined $cond) { substr($s, 0, length($cond), ''); } - if ($s =~ /^\s*;/ && - $function_name ne 'uninitialized_var') + if ($s =~ /^\s*;/) { WARN("AVOID_EXTERNS", "externs should be avoided in .c files\n" . $herecurr); @@ -6405,17 +7016,13 @@ sub process { } # check for function declarations that have arguments without identifier names -# while avoiding uninitialized_var(x) if (defined $stat && - $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:($Ident)|$\s*\*\s*$Ident\s*$)\s*$\s*([^{]+)\s*$\s*;/s && - (!defined($1) || - (defined($1) && $1 ne "uninitialized_var")) && - $2 ne "void") { - my $args = trim($2); + $stat =~ /^.\s*(?:extern\s+)?$Type\s*(?:$Ident|$\s*\*\s*$Ident\s*$)\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$/) { + if ($arg =~ /^$Type$/ && $arg !~ /enum\s+$Ident$/) { WARN("FUNCTION_ARGUMENTS", "function definition argument '$arg' should also have an identifier name\n" . $herecurr); } @@ -6451,7 +7058,7 @@ sub process { if (!grep(/$name/, @setup_docs)) { CHK("UNDOCUMENTED_SETUP", - "__setup appears un-documented -- check Documentation/admin-guide/kernel-parameters.rst\n" . $herecurr); + "__setup appears un-documented -- check Documentation/admin-guide/kernel-parameters.txt\n" . $herecurr); } } @@ -6507,7 +7114,7 @@ sub process { } # check for alloc argument mismatch - if ($line =~ /\b(kcalloc|kmalloc_array)\s*$\s*sizeof\b/) { + if ($line =~ /\b((?:devm_)?(?:kcalloc|kmalloc_array))\s*\(\s*sizeof\b/) { WARN("ALLOC_ARRAY_ARGS", "$1 uses number as first arg, sizeof is generally wrong\n" . $herecurr); } @@ -6533,41 +7140,22 @@ sub process { } } +# check for IS_ENABLED() without CONFIG_<FOO> ($rawline for comments too) + if ($rawline =~ /\bIS_ENABLED\s*\(\s*(\w+)\s*$/ && $1 !~ /^${CONFIG_}/) { + WARN("IS_ENABLED_CONFIG", + "IS_ENABLED($1) is normally used as IS_ENABLED(${CONFIG_}$1)\n" . $herecurr); + } + # 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*$/) { + 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) && + "Prefer IS_ENABLED(<FOO>) to ${CONFIG_}<FOO> || ${CONFIG_}<FOO>_MODULE\n" . $herecurr) && $fix) { $fixed[$fixlinenr] = "\+#if IS_ENABLED($config)"; } } -# check for case / default statements not preceded by break/fallthrough/switch - if ($line =~ /^.\s*(?:case\s+(?:$Ident|$Constant)\s*|default):/) { - my $has_break = 0; - my $has_statement = 0; - my $count = 0; - my $prevline = $linenr; - while ($prevline > 1 && ($file || $count < 3) && !$has_break) { - $prevline--; - my $rline = $rawlines[$prevline - 1]; - my $fline = $lines[$prevline - 1]; - last if ($fline =~ /^\@\@/); - next if ($fline =~ /^\-/); - next if ($fline =~ /^.(?:\s*(?:case\s+(?:$Ident|$Constant)[\s$;]*|default):[\s$;]*)*$/); - $has_break = 1 if ($rline =~ /fall[\s_-]*(through|thru)/i); - next if ($fline =~ /^.[\s$;]*$/); - $has_statement = 1; - $count++; - $has_break = 1 if ($fline =~ /\bswitch\b|\b(?:break\s*;[\s$;]*$|exit\s*\(\b|return\b|goto\b|continue\b)/); - } - if (!$has_break && $has_statement) { - WARN("MISSING_BREAK", - "Possible switch case/default not preceded by break or fallthrough comment\n" . $herecurr); - } - } - # check for /* fallthrough */ like comment, prefer fallthrough; my @fallthroughs = ( 'fallthrough', @@ -6683,7 +7271,8 @@ sub process { # 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/ && + if (defined($const_structs) && + $line !~ /\bconst\b/ && $line =~ /\bstruct\s+($const_structs)\b(?!\s*\{)/) { WARN("CONST_STRUCT", "struct $1 should normally be const\n" . $herecurr); @@ -6691,12 +7280,14 @@ sub process { # use of NR_CPUS is usually wrong # ignore definitions of NR_CPUS and usage to define arrays as likely right +# ignore designated initializers using NR_CPUS if ($line =~ /\bNR_CPUS\b/ && $line !~ /^.\s*\s*#\s*if\b.*\bNR_CPUS\b/ && $line !~ /^.\s*\s*#\s*define\b.*\bNR_CPUS\b/ && $line !~ /^.\s*$Declare\s.*\[[^\]]*NR_CPUS[^\]]*\]/ && $line !~ /\[[^\]]*\.\.\.[^\]]*NR_CPUS[^\]]*\]/ && - $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/) + $line !~ /\[[^\]]*NR_CPUS[^\]]*\.\.\.[^\]]*\]/ && + $line !~ /^.\s*\.\w+\s*=\s*.*\bNR_CPUS\b/) { WARN("NR_CPUS", "usage of NR_CPUS is often wrong - consider using cpu_possible(), num_possible_cpus(), for_each_possible_cpu(), etc\n" . $herecurr); @@ -6715,6 +7306,17 @@ sub process { "Using $1 should generally have parentheses around the comparison\n" . $herecurr); } +# return sysfs_emit(foo, fmt, ...) fmt without newline + if ($line =~ /\breturn\s+sysfs_emit\s*$\s*$FuncArg\s*,\s*($String)/ && + substr($rawline, $-[6], $+[6] - $-[6]) !~ /\\n"$/) { + my $offset = $+[6] - 1; + if (WARN("SYSFS_EMIT", + "return sysfs_emit(...) formats should include a terminating newline\n" . $herecurr) && + $fix) { + substr($fixed[$fixlinenr], $offset, 0) = '\\n'; + } + } + # nested likely/unlikely calls if ($line =~ /\b(?:(?:un)?likely)\s*\(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) { WARN("LIKELY_MISUSE", @@ -6732,12 +7334,6 @@ sub process { } } -# 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*$/ ) { @@ -6900,7 +7496,7 @@ sub process { exit(0); } - # This is not a patch, and we are are in 'no-patch' mode so + # This is not a patch, and we are in 'no-patch' mode so # just keep quiet. if (!$chk_patch && !$is_patch) { exit(0); @@ -6914,9 +7510,33 @@ sub process { if ($signoff == 0) { ERROR("MISSING_SIGN_OFF", "Missing Signed-off-by: line(s)\n"); - } elsif (!$authorsignoff) { - WARN("NO_AUTHOR_SIGN_OFF", - "Missing Signed-off-by: line by nominal patch author '$author'\n"); + } elsif ($authorsignoff != 1) { + # authorsignoff values: + # 0 -> missing sign off + # 1 -> sign off identical + # 2 -> names and addresses match, comments mismatch + # 3 -> addresses match, names different + # 4 -> names match, addresses different + # 5 -> names match, addresses excluding subaddress details (refer RFC 5233) match + + my $sob_msg = "'From: $author' != 'Signed-off-by: $author_sob'"; + + if ($authorsignoff == 0) { + ERROR("NO_AUTHOR_SIGN_OFF", + "Missing Signed-off-by: line by nominal patch author '$author'\n"); + } elsif ($authorsignoff == 2) { + CHK("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email comments mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 3) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email name mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 4) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email address mismatch: $sob_msg\n"); + } elsif ($authorsignoff == 5) { + WARN("FROM_SIGN_OFF_MISMATCH", + "From:/Signed-off-by: email subaddress mismatch: $sob_msg\n"); + } } } diff --git a/scripts/spdxcheck.py b/scripts/spdxcheck.py new file mode 100755 index 000000000000..3e784cf9f401 --- /dev/null +++ b/scripts/spdxcheck.py @@ -0,0 +1,296 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: GPL-2.0 +# Copyright Thomas Gleixner <tglx(a)linutronix.de> + +from argparse import ArgumentParser +from ply import lex, yacc +import locale +import traceback +import sys +import git +import re +import os + +class ParserException(Exception): + def __init__(self, tok, txt): + self.tok = tok + self.txt = txt + +class SPDXException(Exception): + def __init__(self, el, txt): + self.el = el + self.txt = txt + +class SPDXdata(object): + def __init__(self): + self.license_files = 0 + self.exception_files = 0 + self.licenses = [ ] + self.exceptions = { } + +# Read the spdx data from the LICENSES directory +def read_spdxdata(repo): + + # The subdirectories of LICENSES in the kernel source + # Note: exceptions needs to be parsed as last directory. + license_dirs = [ "preferred", "dual", "deprecated", "exceptions" ] + lictree = repo.head.commit.tree['LICENSES'] + + spdx = SPDXdata() + + for d in license_dirs: + for el in lictree[d].traverse(): + if not os.path.isfile(el.path): + continue + + exception = None + for l in open(el.path).readlines(): + if l.startswith('Valid-License-Identifier:'): + lid = l.split(':')[1].strip().upper() + if lid in spdx.licenses: + raise SPDXException(el, 'Duplicate License Identifier: %s' %lid) + else: + spdx.licenses.append(lid) + + elif l.startswith('SPDX-Exception-Identifier:'): + exception = l.split(':')[1].strip().upper() + spdx.exceptions[exception] = [] + + elif l.startswith('SPDX-Licenses:'): + for lic in l.split(':')[1].upper().strip().replace(' ', '').replace('\t', '').split(','): + if not lic in spdx.licenses: + raise SPDXException(None, 'Exception %s missing license %s' %(exception, lic)) + spdx.exceptions[exception].append(lic) + + elif l.startswith("License-Text:"): + if exception: + if not len(spdx.exceptions[exception]): + raise SPDXException(el, 'Exception %s is missing SPDX-Licenses' %exception) + spdx.exception_files += 1 + else: + spdx.license_files += 1 + break + return spdx + +class id_parser(object): + + reserved = [ 'AND', 'OR', 'WITH' ] + tokens = [ 'LPAR', 'RPAR', 'ID', 'EXC' ] + reserved + + precedence = ( ('nonassoc', 'AND', 'OR'), ) + + t_ignore = ' \t' + + def __init__(self, spdx): + self.spdx = spdx + self.lasttok = None + self.lastid = None + self.lexer = lex.lex(module = self, reflags = re.UNICODE) + # Initialize the parser. No debug file and no parser rules stored on disk + # The rules are small enough to be generated on the fly + self.parser = yacc.yacc(module = self, write_tables = False, debug = False) + self.lines_checked = 0 + self.checked = 0 + self.spdx_valid = 0 + self.spdx_errors = 0 + self.curline = 0 + self.deepest = 0 + + # Validate License and Exception IDs + def validate(self, tok): + id = tok.value.upper() + if tok.type == 'ID': + if not id in self.spdx.licenses: + raise ParserException(tok, 'Invalid License ID') + self.lastid = id + elif tok.type == 'EXC': + if id not in self.spdx.exceptions: + raise ParserException(tok, 'Invalid Exception ID') + if self.lastid not in self.spdx.exceptions[id]: + raise ParserException(tok, 'Exception not valid for license %s' %self.lastid) + self.lastid = None + elif tok.type != 'WITH': + self.lastid = None + + # Lexer functions + def t_RPAR(self, tok): + r'\)' + self.lasttok = tok.type + return tok + + def t_LPAR(self, tok): + r'\(' + self.lasttok = tok.type + return tok + + def t_ID(self, tok): + r'[A-Za-z.0-9\-+]+' + + if self.lasttok == 'EXC': + print(tok) + raise ParserException(tok, 'Missing parentheses') + + tok.value = tok.value.strip() + val = tok.value.upper() + + if val in self.reserved: + tok.type = val + elif self.lasttok == 'WITH': + tok.type = 'EXC' + + self.lasttok = tok.type + self.validate(tok) + return tok + + def t_error(self, tok): + raise ParserException(tok, 'Invalid token') + + def p_expr(self, p): + '''expr : ID + | ID WITH EXC + | expr AND expr + | expr OR expr + | LPAR expr RPAR''' + pass + + def p_error(self, p): + if not p: + raise ParserException(None, 'Unfinished license expression') + else: + raise ParserException(p, 'Syntax error') + + def parse(self, expr): + self.lasttok = None + self.lastid = None + self.parser.parse(expr, lexer = self.lexer) + + def parse_lines(self, fd, maxlines, fname): + self.checked += 1 + self.curline = 0 + try: + for line in fd: + line = line.decode(locale.getpreferredencoding(False), errors='ignore') + self.curline += 1 + if self.curline > maxlines: + break + self.lines_checked += 1 + if line.find("SPDX-License-Identifier:") < 0: + continue + expr = line.split(':')[1].strip() + # Remove trailing comment closure + if line.strip().endswith('*/'): + expr = expr.rstrip('*/').strip() + # Remove trailing xml comment closure + if line.strip().endswith('-->'): + expr = expr.rstrip('-->').strip() + # Special case for SH magic boot code files + if line.startswith('LIST \"'): + expr = expr.rstrip('\"').strip() + self.parse(expr) + self.spdx_valid += 1 + # + # Should we check for more SPDX ids in the same file and + # complain if there are any? + # + break + + except ParserException as pe: + if pe.tok: + col = line.find(expr) + pe.tok.lexpos + tok = pe.tok.value + sys.stdout.write('%s: %d:%d %s: %s\n' %(fname, self.curline, col, pe.txt, tok)) + else: + sys.stdout.write('%s: %d:0 %s\n' %(fname, self.curline, col, pe.txt)) + self.spdx_errors += 1 + +def scan_git_tree(tree): + for el in tree.traverse(): + # Exclude stuff which would make pointless noise + # FIXME: Put this somewhere more sensible + if el.path.startswith("LICENSES"): + continue + if el.path.find("license-rules.rst") >= 0: + continue + if not os.path.isfile(el.path): + continue + with open(el.path, 'rb') as fd: + parser.parse_lines(fd, args.maxlines, el.path) + +def scan_git_subtree(tree, path): + for p in path.strip('/').split('/'): + tree = tree[p] + scan_git_tree(tree) + +if __name__ == '__main__': + + ap = ArgumentParser(description='SPDX expression checker') + ap.add_argument('path', nargs='*', help='Check path or file. If not given full git tree scan. For stdin use "-"') + ap.add_argument('-m', '--maxlines', type=int, default=15, + help='Maximum number of lines to scan in a file. Default 15') + ap.add_argument('-v', '--verbose', action='store_true', help='Verbose statistics output') + args = ap.parse_args() + + # Sanity check path arguments + if '-' in args.path and len(args.path) > 1: + sys.stderr.write('stdin input "-" must be the only path argument\n') + sys.exit(1) + + try: + # Use git to get the valid license expressions + repo = git.Repo(os.getcwd()) + assert not repo.bare + + # Initialize SPDX data + spdx = read_spdxdata(repo) + + # Initialize the parser + parser = id_parser(spdx) + + except SPDXException as se: + if se.el: + sys.stderr.write('%s: %s\n' %(se.el.path, se.txt)) + else: + sys.stderr.write('%s\n' %se.txt) + sys.exit(1) + + except Exception as ex: + sys.stderr.write('FAIL: %s\n' %ex) + sys.stderr.write('%s\n' %traceback.format_exc()) + sys.exit(1) + + try: + if len(args.path) and args.path[0] == '-': + stdin = os.fdopen(sys.stdin.fileno(), 'rb') + parser.parse_lines(stdin, args.maxlines, '-') + else: + if args.path: + for p in args.path: + if os.path.isfile(p): + parser.parse_lines(open(p, 'rb'), args.maxlines, p) + elif os.path.isdir(p): + scan_git_subtree(repo.head.reference.commit.tree, p) + else: + sys.stderr.write('path %s does not exist\n' %p) + sys.exit(1) + else: + # Full git tree scan + scan_git_tree(repo.head.commit.tree) + + if args.verbose: + sys.stderr.write('\n') + sys.stderr.write('License files: %12d\n' %spdx.license_files) + sys.stderr.write('Exception files: %12d\n' %spdx.exception_files) + sys.stderr.write('License IDs %12d\n' %len(spdx.licenses)) + sys.stderr.write('Exception IDs %12d\n' %len(spdx.exceptions)) + sys.stderr.write('\n') + sys.stderr.write('Files checked: %12d\n' %parser.checked) + sys.stderr.write('Lines checked: %12d\n' %parser.lines_checked) + sys.stderr.write('Files with SPDX: %12d\n' %parser.spdx_valid) + sys.stderr.write('Files with errors: %12d\n' %parser.spdx_errors) + + sys.exit(0) + + except Exception as ex: + sys.stderr.write('FAIL: %s\n' %ex) + sys.stderr.write('%s\n' %traceback.format_exc()) + sys.exit(1) -- 2.17.1

1 1

[PATCH] mkimage: clarify error message for empty input files
by Thomas Hebb 02 Sep '21

02 Sep '21

Currently, an empty imput file causes `mmap()` to fail, and you get an error like "mkimage: Can't read file.img: Invalid argument", which is extremely unintuitive and hard to diagnose if you don't know what to look for. Add an explicit check for an empty file and provide a clear error message instead. We already bounds check the image size when listing and re-signing existing images, so we only need this check here, when opening data files going into a image. Signed-off-by: Thomas Hebb <tommyhebb(a)gmail.com> --- tools/mkimage.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/mkimage.c b/tools/mkimage.c index 302bfcf971..fbe883ce36 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -732,6 +732,12 @@ copy_file (int ifd, const char *datafile, int pad) exit (EXIT_FAILURE); } + if (sbuf.st_size == 0) { + fprintf (stderr, "%s: Input file %s is empty, bailing out\n", + params.cmdname, datafile); + exit (EXIT_FAILURE); + } + ptr = mmap(0, sbuf.st_size, PROT_READ, MAP_SHARED, dfd, 0); if (ptr == MAP_FAILED) { fprintf (stderr, "%s: Can't read %s: %s\n", -- 2.32.0

4 3