diff options
Diffstat (limited to 'doc/develop/checkpatch.rst')
| -rw-r--r-- | doc/develop/checkpatch.rst | 755 |
1 files changed, 755 insertions, 0 deletions
diff --git a/doc/develop/checkpatch.rst b/doc/develop/checkpatch.rst new file mode 100644 index 0000000000..51fed1bd72 --- /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@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-memory + + **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-on + + **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-strtol-simple-strtoll-simple-strtoul-simple-strtoull + + **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#annotations + + **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#developer-s-certificate-of-origin-1-1 + + **BAD_STABLE_ADDRESS_STYLE** + The email format for stable is incorrect. + Some valid options for stable address are:: + + 1. stable@vger.kernel.org + 2. stable@kernel.org + + For adding version info, the following comment style should be used:: + + stable@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#describe-your-changes + + **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-your-work-the-developer-s-certificate-of-origin + + **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-your-work-the-developer-s-certificate-of-origin + + **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-foundation.org/ + + **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@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#describe-your-changes + + +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#timestamps + + **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+MiicaddB42Q@mail.gmail.com/ + + **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-enums-and-rtl + + **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-braces-and-spaces + + **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-braces-and-spaces + + **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-braces-and-spaces + + **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-braces-and-spaces + + +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; |