| 1 | |
| 2 |
|
| 3 | ==========
|
| 4 | Checkpatch
|
| 5 | ==========
|
| 6 |
|
| 7 | Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial
|
| 8 | style violations in patches and optionally corrects them. Checkpatch can
|
| 9 | also be run on file contexts and without the kernel tree.
|
| 10 |
|
| 11 | Checkpatch is not always right. Your judgement takes precedence over checkpatch
|
| 12 | messages. If your code looks better with the violations, then its probably
|
| 13 | best left alone.
|
| 14 |
|
| 15 |
|
| 16 | Options
|
| 17 | =======
|
| 18 |
|
| 19 | This section will describe the options checkpatch can be run with.
|
| 20 |
|
| 21 | Usage::
|
| 22 |
|
| 23 | ./scripts/checkpatch.pl [OPTION]... [FILE]...
|
| 24 |
|
| 25 | Available options:
|
| 26 |
|
| 27 | - -q, --quiet
|
| 28 |
|
| 29 | Enable quiet mode.
|
| 30 |
|
| 31 | - -v, --verbose
|
| 32 | Enable verbose mode. Additional verbose test descriptions are output
|
| 33 | so as to provide information on why that particular message is shown.
|
| 34 |
|
| 35 | - --no-tree
|
| 36 |
|
| 37 | Run checkpatch without the kernel tree.
|
| 38 |
|
| 39 | - --no-signoff
|
| 40 |
|
| 41 | Disable the 'Signed-off-by' line check. The sign-off is a simple line at
|
| 42 | the end of the explanation for the patch, which certifies that you wrote it
|
| 43 | or otherwise have the right to pass it on as an open-source patch.
|
| 44 |
|
| 45 | Example::
|
| 46 |
|
| 47 | Signed-off-by: Random J Developer <[email protected]>
|
| 48 |
|
| 49 | Setting this flag effectively stops a message for a missing signed-off-by
|
| 50 | line in a patch context.
|
| 51 |
|
| 52 | - --patch
|
| 53 |
|
| 54 | Treat FILE as a patch. This is the default option and need not be
|
| 55 | explicitly specified.
|
| 56 |
|
| 57 | - --emacs
|
| 58 |
|
| 59 | Set output to emacs compile window format. This allows emacs users to jump
|
| 60 | from the error in the compile window directly to the offending line in the
|
| 61 | patch.
|
| 62 |
|
| 63 | - --terse
|
| 64 |
|
| 65 | Output only one line per report.
|
| 66 |
|
| 67 | - --showfile
|
| 68 |
|
| 69 | Show the diffed file position instead of the input file position.
|
| 70 |
|
| 71 | - -g, --git
|
| 72 |
|
| 73 | Treat FILE as a single commit or a git revision range.
|
| 74 |
|
| 75 | Single commit with:
|
| 76 |
|
| 77 | - <rev>
|
| 78 | - <rev>^
|
| 79 | - <rev>~n
|
| 80 |
|
| 81 | Multiple commits with:
|
| 82 |
|
| 83 | - <rev1>..<rev2>
|
| 84 | - <rev1>...<rev2>
|
| 85 | - <rev>-<count>
|
| 86 |
|
| 87 | - -f, --file
|
| 88 |
|
| 89 | Treat FILE as a regular source file. This option must be used when running
|
| 90 | checkpatch on source files in the kernel.
|
| 91 |
|
| 92 | - --subjective, --strict
|
| 93 |
|
| 94 | Enable stricter tests in checkpatch. By default the tests emitted as CHECK
|
| 95 | do not activate by default. Use this flag to activate the CHECK tests.
|
| 96 |
|
| 97 | - --list-types
|
| 98 |
|
| 99 | Every message emitted by checkpatch has an associated TYPE. Add this flag
|
| 100 | to display all the types in checkpatch.
|
| 101 |
|
| 102 | Note that when this flag is active, checkpatch does not read the input FILE,
|
| 103 | and no message is emitted. Only a list of types in checkpatch is output.
|
| 104 |
|
| 105 | - --types TYPE(,TYPE2...)
|
| 106 |
|
| 107 | Only display messages with the given types.
|
| 108 |
|
| 109 | Example::
|
| 110 |
|
| 111 | ./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,BRACES
|
| 112 |
|
| 113 | - --ignore TYPE(,TYPE2...)
|
| 114 |
|
| 115 | Checkpatch will not emit messages for the specified types.
|
| 116 |
|
| 117 | Example::
|
| 118 |
|
| 119 | ./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,BRACES
|
| 120 |
|
| 121 | - --show-types
|
| 122 |
|
| 123 | By default checkpatch doesn't display the type associated with the messages.
|
| 124 | Set this flag to show the message type in the output.
|
| 125 |
|
| 126 | - --max-line-length=n
|
| 127 |
|
| 128 | Set the max line length (default 100). If a line exceeds the specified
|
| 129 | length, a LONG_LINE message is emitted.
|
| 130 |
|
| 131 |
|
| 132 | The message level is different for patch and file contexts. For patches,
|
| 133 | a WARNING is emitted. While a milder CHECK is emitted for files. So for
|
| 134 | file contexts, the --strict flag must also be enabled.
|
| 135 |
|
| 136 | - --min-conf-desc-length=n
|
| 137 |
|
| 138 | Set the Kconfig entry minimum description length, if shorter, warn.
|
| 139 |
|
| 140 | - --tab-size=n
|
| 141 |
|
| 142 | Set the number of spaces for tab (default 8).
|
| 143 |
|
| 144 | - --root=PATH
|
| 145 |
|
| 146 | PATH to the kernel tree root.
|
| 147 |
|
| 148 | This option must be specified when invoking checkpatch from outside
|
| 149 | the kernel root.
|
| 150 |
|
| 151 | - --no-summary
|
| 152 |
|
| 153 | Suppress the per file summary.
|
| 154 |
|
| 155 | - --mailback
|
| 156 |
|
| 157 | Only produce a report in case of Warnings or Errors. Milder Checks are
|
| 158 | excluded from this.
|
| 159 |
|
| 160 | - --summary-file
|
| 161 |
|
| 162 | Include the filename in summary.
|
| 163 |
|
| 164 | - --debug KEY=[0|1]
|
| 165 |
|
| 166 | Turn on/off debugging of KEY, where KEY is one of 'values', 'possible',
|
| 167 | 'type', and 'attr' (default is all off).
|
| 168 |
|
| 169 | - --fix
|
| 170 |
|
| 171 | This is an EXPERIMENTAL feature. If correctable errors exist, a file
|
| 172 | <inputfile>.EXPERIMENTAL-checkpatch-fixes is created which has the
|
| 173 | automatically fixable errors corrected.
|
| 174 |
|
| 175 | - --fix-inplace
|
| 176 |
|
| 177 | EXPERIMENTAL - Similar to --fix but input file is overwritten with fixes.
|
| 178 |
|
| 179 | DO NOT USE this flag unless you are absolutely sure and you have a backup
|
| 180 | in place.
|
| 181 |
|
| 182 | - --ignore-perl-version
|
| 183 |
|
| 184 | Override checking of perl version. Runtime errors may be encountered after
|
| 185 | enabling this flag if the perl version does not meet the minimum specified.
|
| 186 |
|
| 187 | - --spdx-cxx-comments
|
| 188 |
|
| 189 | Don't force C comments ``/* */`` for SPDX license (required by old
|
| 190 | toolchains), allow also C++ comments ``//``.
|
| 191 |
|
| 192 | NOTE: it should *not* be used for Linux mainline.
|
| 193 |
|
| 194 | - --codespell
|
| 195 |
|
| 196 | Use the codespell dictionary for checking spelling errors.
|
| 197 |
|
| 198 | - --codespellfile
|
| 199 |
|
| 200 | Use the specified codespell file.
|
| 201 | Default is '/usr/share/codespell/dictionary.txt'.
|
| 202 |
|
| 203 | - --typedefsfile
|
| 204 |
|
| 205 | Read additional types from this file.
|
| 206 |
|
| 207 | - --color[=WHEN]
|
| 208 |
|
| 209 | Use colors 'always', 'never', or only when output is a terminal ('auto').
|
| 210 | Default is 'auto'.
|
| 211 |
|
| 212 | - --kconfig-prefix=WORD
|
| 213 |
|
| 214 | Use WORD as a prefix for Kconfig symbols (default is `CONFIG_`).
|
| 215 |
|
| 216 | - -h, --help, --version
|
| 217 |
|
| 218 | Display the help text.
|
| 219 |
|
| 220 | Configuration file
|
| 221 | ==================
|
| 222 |
|
| 223 | Default configuration options can be stored in ``.checkpatch.conf``, search
|
| 224 | path: ``.:$HOME:.scripts`` or in a directory specified by ``$CHECKPATCH_CONFIG_DIR``
|
| 225 | environment variable (falling back to the default search path).
|
| 226 |
|
| 227 | Message Levels
|
| 228 | ==============
|
| 229 |
|
| 230 | Messages in checkpatch are divided into three levels. The levels of messages
|
| 231 | in checkpatch denote the severity of the error. They are:
|
| 232 |
|
| 233 | - ERROR
|
| 234 |
|
| 235 | This is the most strict level. Messages of type ERROR must be taken
|
| 236 | seriously as they denote things that are very likely to be wrong.
|
| 237 |
|
| 238 | - WARNING
|
| 239 |
|
| 240 | This is the next stricter level. Messages of type WARNING requires a
|
| 241 | more careful review. But it is milder than an ERROR.
|
| 242 |
|
| 243 | - CHECK
|
| 244 |
|
| 245 | This is the mildest level. These are things which may require some thought.
|
| 246 |
|
| 247 | Type Descriptions
|
| 248 | =================
|
| 249 |
|
| 250 | This section contains a description of all the message types in checkpatch.
|
| 251 |
|
| 252 | |
| 253 | |
| 254 |
|
| 255 |
|
| 256 | Allocation style
|
| 257 | ----------------
|
| 258 |
|
| 259 | **ALLOC_ARRAY_ARGS**
|
| 260 | The first argument for kcalloc or kmalloc_array should be the
|
| 261 | number of elements. sizeof() as the first argument is generally
|
| 262 | wrong.
|
| 263 |
|
| 264 | See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html
|
| 265 |
|
| 266 | **ALLOC_SIZEOF_STRUCT**
|
| 267 | The allocation style is bad. In general for family of
|
| 268 | allocation functions using sizeof() to get memory size,
|
| 269 | constructs like::
|
| 270 |
|
| 271 | p = alloc(sizeof(struct foo), ...)
|
| 272 |
|
| 273 | should be::
|
| 274 |
|
| 275 | p = alloc(sizeof(*p), ...)
|
| 276 |
|
| 277 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#allocating-memory
|
| 278 |
|
| 279 | **ALLOC_WITH_MULTIPLY**
|
| 280 | Prefer kmalloc_array/kcalloc over kmalloc/kzalloc with a
|
| 281 | sizeof multiply.
|
| 282 |
|
| 283 | See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html
|
| 284 |
|
| 285 |
|
| 286 | API usage
|
| 287 | ---------
|
| 288 |
|
| 289 | **ARCH_DEFINES**
|
| 290 | Architecture specific defines should be avoided wherever
|
| 291 | possible.
|
| 292 |
|
| 293 | **ARCH_INCLUDE_LINUX**
|
| 294 | Whenever asm/file.h is included and linux/file.h exists, a
|
| 295 | conversion can be made when linux/file.h includes asm/file.h.
|
| 296 | However this is not always the case (See signal.h).
|
| 297 | This message type is emitted only for includes from arch/.
|
| 298 |
|
| 299 | **AVOID_BUG**
|
| 300 | BUG() or BUG_ON() should be avoided totally.
|
| 301 | Use WARN() and WARN_ON() instead, and handle the "impossible"
|
| 302 | error condition as gracefully as possible.
|
| 303 |
|
| 304 | See: https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on
|
| 305 |
|
| 306 | **CONSIDER_KSTRTO**
|
| 307 | The simple_strtol(), simple_strtoll(), simple_strtoul(), and
|
| 308 | simple_strtoull() functions explicitly ignore overflows, which
|
| 309 | may lead to unexpected results in callers. The respective kstrtol(),
|
| 310 | kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the
|
| 311 | correct replacements.
|
| 312 |
|
| 313 | See: https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull
|
| 314 |
|
| 315 | **CONSTANT_CONVERSION**
|
| 316 | Use of __constant_<foo> form is discouraged for the following functions::
|
| 317 |
|
| 318 | __constant_cpu_to_be[x]
|
| 319 | __constant_cpu_to_le[x]
|
| 320 | __constant_be[x]_to_cpu
|
| 321 | __constant_le[x]_to_cpu
|
| 322 | __constant_htons
|
| 323 | __constant_ntohs
|
| 324 |
|
| 325 | Using any of these outside of include/uapi/ is not preferred as using the
|
| 326 | function without __constant_ is identical when the argument is a
|
| 327 | constant.
|
| 328 |
|
| 329 | In big endian systems, the macros like __constant_cpu_to_be32(x) and
|
| 330 | cpu_to_be32(x) expand to the same expression::
|
| 331 |
|
| 332 | #define __constant_cpu_to_be32(x) ((__force __be32)(__u32)(x))
|
| 333 | #define __cpu_to_be32(x) ((__force __be32)(__u32)(x))
|
| 334 |
|
| 335 | In little endian systems, the macros __constant_cpu_to_be32(x) and
|
| 336 | cpu_to_be32(x) expand to __constant_swab32 and __swab32. __swab32
|
| 337 | has a __builtin_constant_p check::
|
| 338 |
|
| 339 | #define __swab32(x) \
|
| 340 | (__builtin_constant_p((__u32)(x)) ? \
|
| 341 | ___constant_swab32(x) : \
|
| 342 | __fswab32(x))
|
| 343 |
|
| 344 | So ultimately they have a special case for constants.
|
| 345 | Similar is the case with all of the macros in the list. Thus
|
| 346 | using the __constant_... forms are unnecessarily verbose and
|
| 347 | not preferred outside of include/uapi.
|
| 348 |
|
| 349 | See: https://lore.kernel.org/lkml/1400106425.12666.6.camel@joe-AO725/
|
| 350 |
|
| 351 | **DEPRECATED_API**
|
| 352 | Usage of a deprecated RCU API is detected. It is recommended to replace
|
| 353 | old flavourful RCU APIs by their new vanilla-RCU counterparts.
|
| 354 |
|
| 355 | The full list of available RCU APIs can be viewed from the kernel docs.
|
| 356 |
|
| 357 | See: https://www.kernel.org/doc/html/latest/RCU/whatisRCU.html#full-list-of-rcu-apis
|
| 358 |
|
| 359 | **DEVICE_ATTR_FUNCTIONS**
|
| 360 | The function names used in DEVICE_ATTR is unusual.
|
| 361 | Typically, the store and show functions are used with <attr>_store and
|
| 362 | <attr>_show, where <attr> is a named attribute variable of the device.
|
| 363 |
|
| 364 | Consider the following examples::
|
| 365 |
|
| 366 | static DEVICE_ATTR(type, 0444, type_show, NULL);
|
| 367 | static DEVICE_ATTR(power, 0644, power_show, power_store);
|
| 368 |
|
| 369 | The function names should preferably follow the above pattern.
|
| 370 |
|
| 371 | See: https://www.kernel.org/doc/html/latest/driver-api/driver-model/device.html#attributes
|
| 372 |
|
| 373 | **DEVICE_ATTR_RO**
|
| 374 | The DEVICE_ATTR_RO(name) helper macro can be used instead of
|
| 375 | DEVICE_ATTR(name, 0444, name_show, NULL);
|
| 376 |
|
| 377 | Note that the macro automatically appends _show to the named
|
| 378 | attribute variable of the device for the show method.
|
| 379 |
|
| 380 | See: https://www.kernel.org/doc/html/latest/driver-api/driver-model/device.html#attributes
|
| 381 |
|
| 382 | **DEVICE_ATTR_RW**
|
| 383 | The DEVICE_ATTR_RW(name) helper macro can be used instead of
|
| 384 | DEVICE_ATTR(name, 0644, name_show, name_store);
|
| 385 |
|
| 386 | Note that the macro automatically appends _show and _store to the
|
| 387 | named attribute variable of the device for the show and store methods.
|
| 388 |
|
| 389 | See: https://www.kernel.org/doc/html/latest/driver-api/driver-model/device.html#attributes
|
| 390 |
|
| 391 | **DEVICE_ATTR_WO**
|
| 392 | The DEVICE_AATR_WO(name) helper macro can be used instead of
|
| 393 | DEVICE_ATTR(name, 0200, NULL, name_store);
|
| 394 |
|
| 395 | Note that the macro automatically appends _store to the
|
| 396 | named attribute variable of the device for the store method.
|
| 397 |
|
| 398 | See: https://www.kernel.org/doc/html/latest/driver-api/driver-model/device.html#attributes
|
| 399 |
|
| 400 | **DUPLICATED_SYSCTL_CONST**
|
| 401 | Commit d91bff3011cf ("proc/sysctl: add shared variables for range
|
| 402 | check") added some shared const variables to be used instead of a local
|
| 403 | copy in each source file.
|
| 404 |
|
| 405 | Consider replacing the sysctl range checking value with the shared
|
| 406 | one in include/linux/sysctl.h. The following conversion scheme may
|
| 407 | be used::
|
| 408 |
|
| 409 | &zero -> SYSCTL_ZERO
|
| 410 | &one -> SYSCTL_ONE
|
| 411 | &int_max -> SYSCTL_INT_MAX
|
| 412 |
|
| 413 | See:
|
| 414 |
|
| 415 | 1. https://lore.kernel.org/lkml/[email protected]/
|
| 416 | 2. https://lore.kernel.org/lkml/[email protected]/
|
| 417 |
|
| 418 | **ENOSYS**
|
| 419 | ENOSYS means that a nonexistent system call was called.
|
| 420 | Earlier, it was wrongly used for things like invalid operations on
|
| 421 | otherwise valid syscalls. This should be avoided in new code.
|
| 422 |
|
| 423 | See: https://lore.kernel.org/lkml/5eb299021dec23c1a48fa7d9f2c8b794e967766d.1408730669.git.luto@amacapital.net/
|
| 424 |
|
| 425 | **ENOTSUPP**
|
| 426 | ENOTSUPP is not a standard error code and should be avoided in new patches.
|
| 427 | EOPNOTSUPP should be used instead.
|
| 428 |
|
| 429 | See: https://lore.kernel.org/netdev/[email protected]/
|
| 430 |
|
| 431 | **EXPORT_SYMBOL**
|
| 432 | EXPORT_SYMBOL should immediately follow the symbol to be exported.
|
| 433 |
|
| 434 | **IN_ATOMIC**
|
| 435 | in_atomic() is not for driver use so any such use is reported as an ERROR.
|
| 436 | Also in_atomic() is often used to determine if sleeping is permitted,
|
| 437 | but it is not reliable in this use model. Therefore its use is
|
| 438 | strongly discouraged.
|
| 439 |
|
| 440 | However, in_atomic() is ok for core kernel use.
|
| 441 |
|
| 442 | See: https://lore.kernel.org/lkml/[email protected]/
|
| 443 |
|
| 444 | **LOCKDEP**
|
| 445 | The lockdep_no_validate class was added as a temporary measure to
|
| 446 | prevent warnings on conversion of device->sem to device->mutex.
|
| 447 | It should not be used for any other purpose.
|
| 448 |
|
| 449 | See: https://lore.kernel.org/lkml/1268959062.9440.467.camel@laptop/
|
| 450 |
|
| 451 | **MALFORMED_INCLUDE**
|
| 452 | The #include statement has a malformed path. This has happened
|
| 453 | because the author has included a double slash "//" in the pathname
|
| 454 | accidentally.
|
| 455 |
|
| 456 | **USE_LOCKDEP**
|
| 457 | lockdep_assert_held() annotations should be preferred over
|
| 458 | assertions based on spin_is_locked()
|
| 459 |
|
| 460 | See: https://www.kernel.org/doc/html/latest/locking/lockdep-design.html#annotations
|
| 461 |
|
| 462 | **UAPI_INCLUDE**
|
| 463 | No #include statements in include/uapi should use a uapi/ path.
|
| 464 |
|
| 465 | **USLEEP_RANGE**
|
| 466 | usleep_range() should be preferred over udelay(). The proper way of
|
| 467 | using usleep_range() is mentioned in the kernel docs.
|
| 468 |
|
| 469 |
|
| 470 | Comments
|
| 471 | --------
|
| 472 |
|
| 473 | **BLOCK_COMMENT_STYLE**
|
| 474 | The comment style is incorrect. The preferred style for multi-
|
| 475 | line comments is::
|
| 476 |
|
| 477 | /*
|
| 478 | * This is the preferred style
|
| 479 | * for multi line comments.
|
| 480 | */
|
| 481 |
|
| 482 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting
|
| 483 |
|
| 484 | **C99_COMMENTS**
|
| 485 | C99 style single line comments (//) should not be used.
|
| 486 | Prefer the block comment style instead.
|
| 487 |
|
| 488 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting
|
| 489 |
|
| 490 | **DATA_RACE**
|
| 491 | Applications of data_race() should have a comment so as to document the
|
| 492 | reasoning behind why it was deemed safe.
|
| 493 |
|
| 494 | See: https://lore.kernel.org/lkml/[email protected]/
|
| 495 |
|
| 496 | **FSF_MAILING_ADDRESS**
|
| 497 | Kernel maintainers reject new instances of the GPL boilerplate paragraph
|
| 498 | directing people to write to the FSF for a copy of the GPL, since the
|
| 499 | FSF has moved in the past and may do so again.
|
| 500 | So do not write paragraphs about writing to the Free Software Foundation's
|
| 501 | mailing address.
|
| 502 |
|
| 503 | See: https://lore.kernel.org/lkml/20131006222342.GT19510@leaf/
|
| 504 |
|
| 505 | **UNCOMMENTED_RGMII_MODE**
|
| 506 | Historically, the RGMII PHY modes specified in Device Trees have been
|
| 507 | used inconsistently, often referring to the usage of delays on the PHY
|
| 508 | side rather than describing the board.
|
| 509 |
|
| 510 | PHY modes "rgmii", "rgmii-rxid" and "rgmii-txid" modes require the clock
|
| 511 | signal to be delayed on the PCB; this unusual configuration should be
|
| 512 | described in a comment. If they are not (meaning that the delay is realized
|
| 513 | internally in the MAC or PHY), "rgmii-id" is the correct PHY mode.
|
| 514 |
|
| 515 | Commit message
|
| 516 | --------------
|
| 517 |
|
| 518 | **BAD_SIGN_OFF**
|
| 519 | The signed-off-by line does not fall in line with the standards
|
| 520 | specified by the community.
|
| 521 |
|
| 522 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
|
| 523 |
|
| 524 | **BAD_STABLE_ADDRESS_STYLE**
|
| 525 | The email format for stable is incorrect.
|
| 526 | Some valid options for stable address are::
|
| 527 |
|
| 528 | 1. [email protected]
|
| 529 | 2. [email protected]
|
| 530 |
|
| 531 | For adding version info, the following comment style should be used::
|
| 532 |
|
| 533 | [email protected] # version info
|
| 534 |
|
| 535 | **COMMIT_COMMENT_SYMBOL**
|
| 536 | Commit log lines starting with a '#' are ignored by git as
|
| 537 | comments. To solve this problem addition of a single space
|
| 538 | infront of the log line is enough.
|
| 539 |
|
| 540 | **COMMIT_MESSAGE**
|
| 541 | The patch is missing a commit description. A brief
|
| 542 | description of the changes made by the patch should be added.
|
| 543 |
|
| 544 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
|
| 545 |
|
| 546 | **EMAIL_SUBJECT**
|
| 547 | Naming the tool that found the issue is not very useful in the
|
| 548 | subject line. A good subject line summarizes the change that
|
| 549 | the patch brings.
|
| 550 |
|
| 551 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
|
| 552 |
|
| 553 | **FROM_SIGN_OFF_MISMATCH**
|
| 554 | The author's email does not match with that in the Signed-off-by:
|
| 555 | line(s). This can be sometimes caused due to an improperly configured
|
| 556 | email client.
|
| 557 |
|
| 558 | This message is emitted due to any of the following reasons::
|
| 559 |
|
| 560 | - The email names do not match.
|
| 561 | - The email addresses do not match.
|
| 562 | - The email subaddresses do not match.
|
| 563 | - The email comments do not match.
|
| 564 |
|
| 565 | **MISSING_SIGN_OFF**
|
| 566 | The patch is missing a Signed-off-by line. A signed-off-by
|
| 567 | line should be added according to Developer's certificate of
|
| 568 | Origin.
|
| 569 |
|
| 570 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
|
| 571 |
|
| 572 | **NO_AUTHOR_SIGN_OFF**
|
| 573 | The author of the patch has not signed off the patch. It is
|
| 574 | required that a simple sign off line should be present at the
|
| 575 | end of explanation of the patch to denote that the author has
|
| 576 | written it or otherwise has the rights to pass it on as an open
|
| 577 | source patch.
|
| 578 |
|
| 579 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
|
| 580 |
|
| 581 | **DIFF_IN_COMMIT_MSG**
|
| 582 | Avoid having diff content in commit message.
|
| 583 | This causes problems when one tries to apply a file containing both
|
| 584 | the changelog and the diff because patch(1) tries to apply the diff
|
| 585 | which it found in the changelog.
|
| 586 |
|
| 587 | See: https://lore.kernel.org/lkml/[email protected]/
|
| 588 |
|
| 589 | **GERRIT_CHANGE_ID**
|
| 590 | To be picked up by gerrit, the footer of the commit message might
|
| 591 | have a Change-Id like::
|
| 592 |
|
| 593 | Change-Id: Ic8aaa0728a43936cd4c6e1ed590e01ba8f0fbf5b
|
| 594 | Signed-off-by: A. U. Thor <[email protected]>
|
| 595 |
|
| 596 | The Change-Id line must be removed before submitting.
|
| 597 |
|
| 598 | **GIT_COMMIT_ID**
|
| 599 | The proper way to reference a commit id is:
|
| 600 | commit <12+ chars of sha1> ("<title line>")
|
| 601 |
|
| 602 | An example may be::
|
| 603 |
|
| 604 | Commit e21d2170f36602ae2708 ("video: remove unnecessary
|
| 605 | platform_set_drvdata()") removed the unnecessary
|
| 606 | platform_set_drvdata(), but left the variable "dev" unused,
|
| 607 | delete it.
|
| 608 |
|
| 609 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
|
| 610 |
|
| 611 | **BAD_FIXES_TAG**
|
| 612 | The Fixes: tag is malformed or does not follow the community conventions.
|
| 613 | This can occur if the tag have been split into multiple lines (e.g., when
|
| 614 | pasted in an email program with word wrapping enabled).
|
| 615 |
|
| 616 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
|
| 617 |
|
| 618 | **BAD_COMMIT_SEPARATOR**
|
| 619 | The commit separator is a single line with 3 dashes.
|
| 620 | The regex match is '^---$'
|
| 621 | Lines that start with 3 dashes and have more content on the same line
|
| 622 | may confuse tools that apply patches.
|
| 623 |
|
| 624 | Comparison style
|
| 625 | ----------------
|
| 626 |
|
| 627 | **ASSIGN_IN_IF**
|
| 628 | Do not use assignments in if condition.
|
| 629 | Example::
|
| 630 |
|
| 631 | if ((foo = bar(...)) < BAZ) {
|
| 632 |
|
| 633 | should be written as::
|
| 634 |
|
| 635 | foo = bar(...);
|
| 636 | if (foo < BAZ) {
|
| 637 |
|
| 638 | **BOOL_COMPARISON**
|
| 639 | Comparisons of A to true and false are better written
|
| 640 | as A and !A.
|
| 641 |
|
| 642 | See: https://lore.kernel.org/lkml/1365563834.27174.12.camel@joe-AO722/
|
| 643 |
|
| 644 | **COMPARISON_TO_NULL**
|
| 645 | Comparisons to NULL in the form (foo == NULL) or (foo != NULL)
|
| 646 | are better written as (!foo) and (foo).
|
| 647 |
|
| 648 | **CONSTANT_COMPARISON**
|
| 649 | Comparisons with a constant or upper case identifier on the left
|
| 650 | side of the test should be avoided.
|
| 651 |
|
| 652 |
|
| 653 | Indentation and Line Breaks
|
| 654 | ---------------------------
|
| 655 |
|
| 656 | **CODE_INDENT**
|
| 657 | Code indent should use tabs instead of spaces.
|
| 658 | Outside of comments, documentation and Kconfig,
|
| 659 | spaces are never used for indentation.
|
| 660 |
|
| 661 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation
|
| 662 |
|
| 663 | **DEEP_INDENTATION**
|
| 664 | Indentation with 6 or more tabs usually indicate overly indented
|
| 665 | code.
|
| 666 |
|
| 667 | It is suggested to refactor excessive indentation of
|
| 668 | if/else/for/do/while/switch statements.
|
| 669 |
|
| 670 | See: https://lore.kernel.org/lkml/1328311239.21255.24.camel@joe2Laptop/
|
| 671 |
|
| 672 | **SWITCH_CASE_INDENT_LEVEL**
|
| 673 | switch should be at the same indent as case.
|
| 674 | Example::
|
| 675 |
|
| 676 | switch (suffix) {
|
| 677 | case 'G':
|
| 678 | case 'g':
|
| 679 | mem <<= 30;
|
| 680 | break;
|
| 681 | case 'M':
|
| 682 | case 'm':
|
| 683 | mem <<= 20;
|
| 684 | break;
|
| 685 | case 'K':
|
| 686 | case 'k':
|
| 687 | mem <<= 10;
|
| 688 | fallthrough;
|
| 689 | default:
|
| 690 | break;
|
| 691 | }
|
| 692 |
|
| 693 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation
|
| 694 |
|
| 695 | **LONG_LINE**
|
| 696 | The line has exceeded the specified maximum length.
|
| 697 | To use a different maximum line length, the --max-line-length=n option
|
| 698 | may be added while invoking checkpatch.
|
| 699 |
|
| 700 | Earlier, the default line length was 80 columns. Commit bdc48fa11e46
|
| 701 | ("checkpatch/coding-style: deprecate 80-column warning") increased the
|
| 702 | limit to 100 columns. This is not a hard limit either and it's
|
| 703 | preferable to stay within 80 columns whenever possible.
|
| 704 |
|
| 705 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
|
| 706 |
|
| 707 | **LONG_LINE_STRING**
|
| 708 | A string starts before but extends beyond the maximum line length.
|
| 709 | To use a different maximum line length, the --max-line-length=n option
|
| 710 | may be added while invoking checkpatch.
|
| 711 |
|
| 712 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
|
| 713 |
|
| 714 | **LONG_LINE_COMMENT**
|
| 715 | A comment starts before but extends beyond the maximum line length.
|
| 716 | To use a different maximum line length, the --max-line-length=n option
|
| 717 | may be added while invoking checkpatch.
|
| 718 |
|
| 719 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
|
| 720 |
|
| 721 | **SPLIT_STRING**
|
| 722 | Quoted strings that appear as messages in userspace and can be
|
| 723 | grepped, should not be split across multiple lines.
|
| 724 |
|
| 725 | See: https://lore.kernel.org/lkml/20120203052727.GA15035@leaf/
|
| 726 |
|
| 727 | **MULTILINE_DEREFERENCE**
|
| 728 | A single dereferencing identifier spanned on multiple lines like::
|
| 729 |
|
| 730 | struct_identifier->member[index].
|
| 731 | member = <foo>;
|
| 732 |
|
| 733 | is generally hard to follow. It can easily lead to typos and so makes
|
| 734 | the code vulnerable to bugs.
|
| 735 |
|
| 736 | If fixing the multiple line dereferencing leads to an 80 column
|
| 737 | violation, then either rewrite the code in a more simple way or if the
|
| 738 | starting part of the dereferencing identifier is the same and used at
|
| 739 | multiple places then store it in a temporary variable, and use that
|
| 740 | temporary variable only at all the places. For example, if there are
|
| 741 | two dereferencing identifiers::
|
| 742 |
|
| 743 | member1->member2->member3.foo1;
|
| 744 | member1->member2->member3.foo2;
|
| 745 |
|
| 746 | then store the member1->member2->member3 part in a temporary variable.
|
| 747 | It not only helps to avoid the 80 column violation but also reduces
|
| 748 | the program size by removing the unnecessary dereferences.
|
| 749 |
|
| 750 | But if none of the above methods work then ignore the 80 column
|
| 751 | violation because it is much easier to read a dereferencing identifier
|
| 752 | on a single line.
|
| 753 |
|
| 754 | **TRAILING_STATEMENTS**
|
| 755 | Trailing statements (for example after any conditional) should be
|
| 756 | on the next line.
|
| 757 | Statements, such as::
|
| 758 |
|
| 759 | if (x == y) break;
|
| 760 |
|
| 761 | should be::
|
| 762 |
|
| 763 | if (x == y)
|
| 764 | break;
|
| 765 |
|
| 766 |
|
| 767 | Macros, Attributes and Symbols
|
| 768 | ------------------------------
|
| 769 |
|
| 770 | **ARRAY_SIZE**
|
| 771 | The ARRAY_SIZE(foo) macro should be preferred over
|
| 772 | sizeof(foo)/sizeof(foo[0]) for finding number of elements in an
|
| 773 | array.
|
| 774 |
|
| 775 | The macro is defined in include/linux/array_size.h::
|
| 776 |
|
| 777 | #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
|
| 778 |
|
| 779 | **AVOID_EXTERNS**
|
| 780 | Function prototypes don't need to be declared extern in .h
|
| 781 | files. It's assumed by the compiler and is unnecessary.
|
| 782 |
|
| 783 | **AVOID_L_PREFIX**
|
| 784 | Local symbol names that are prefixed with `.L` should be avoided,
|
| 785 | as this has special meaning for the assembler; a symbol entry will
|
| 786 | not be emitted into the symbol table. This can prevent `objtool`
|
| 787 | from generating correct unwind info.
|
| 788 |
|
| 789 | Symbols with STB_LOCAL binding may still be used, and `.L` prefixed
|
| 790 | local symbol names are still generally usable within a function,
|
| 791 | but `.L` prefixed local symbol names should not be used to denote
|
| 792 | the beginning or end of code regions via
|
| 793 | `SYM_CODE_START_LOCAL`/`SYM_CODE_END`
|
| 794 |
|
| 795 | **BIT_MACRO**
|
| 796 | Defines like: 1 << <digit> could be BIT(digit).
|
| 797 | The BIT() macro is defined via include/linux/bits.h::
|
| 798 |
|
| 799 | #define BIT(nr) (1UL << (nr))
|
| 800 |
|
| 801 | **CONST_READ_MOSTLY**
|
| 802 | When a variable is tagged with the __read_mostly annotation, it is a
|
| 803 | signal to the compiler that accesses to the variable will be mostly
|
| 804 | reads and rarely(but NOT never) a write.
|
| 805 |
|
| 806 | const __read_mostly does not make any sense as const data is already
|
| 807 | read-only. The __read_mostly annotation thus should be removed.
|
| 808 |
|
| 809 | **DATE_TIME**
|
| 810 | It is generally desirable that building the same source code with
|
| 811 | the same set of tools is reproducible, i.e. the output is always
|
| 812 | exactly the same.
|
| 813 |
|
| 814 | The kernel does *not* use the ``__DATE__`` and ``__TIME__`` macros,
|
| 815 | and enables warnings if they are used as they can lead to
|
| 816 | non-deterministic builds.
|
| 817 |
|
| 818 | See: https://www.kernel.org/doc/html/latest/kbuild/reproducible-builds.html#timestamps
|
| 819 |
|
| 820 | **DEFINE_ARCH_HAS**
|
| 821 | The ARCH_HAS_xyz and ARCH_HAVE_xyz patterns are wrong.
|
| 822 |
|
| 823 | For big conceptual features use Kconfig symbols instead. And for
|
| 824 | smaller things where we have compatibility fallback functions but
|
| 825 | want architectures able to override them with optimized ones, we
|
| 826 | should either use weak functions (appropriate for some cases), or
|
| 827 | the symbol that protects them should be the same symbol we use.
|
| 828 |
|
| 829 | See: https://lore.kernel.org/lkml/CA+55aFycQ9XJvEOsiM3txHL5bjUc8CeKWJNR_H+MiicaddB42Q@mail.gmail.com/
|
| 830 |
|
| 831 | **DO_WHILE_MACRO_WITH_TRAILING_SEMICOLON**
|
| 832 | do {} while(0) macros should not have a trailing semicolon.
|
| 833 |
|
| 834 | **INIT_ATTRIBUTE**
|
| 835 | Const init definitions should use __initconst instead of
|
| 836 | __initdata.
|
| 837 |
|
| 838 | Similarly init definitions without const require a separate
|
| 839 | use of const.
|
| 840 |
|
| 841 | **INLINE_LOCATION**
|
| 842 | The inline keyword should sit between storage class and type.
|
| 843 |
|
| 844 | For example, the following segment::
|
| 845 |
|
| 846 | inline static int example_function(void)
|
| 847 | {
|
| 848 | ...
|
| 849 | }
|
| 850 |
|
| 851 | should be::
|
| 852 |
|
| 853 | static inline int example_function(void)
|
| 854 | {
|
| 855 | ...
|
| 856 | }
|
| 857 |
|
| 858 | **MISPLACED_INIT**
|
| 859 | It is possible to use section markers on variables in a way
|
| 860 | which gcc doesn't understand (or at least not the way the
|
| 861 | developer intended)::
|
| 862 |
|
| 863 | static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
|
| 864 |
|
| 865 | does not put exynos4_plls in the .initdata section. The __initdata
|
| 866 | marker can be virtually anywhere on the line, except right after
|
| 867 | "struct". The preferred location is before the "=" sign if there is
|
| 868 | one, or before the trailing ";" otherwise.
|
| 869 |
|
| 870 | See: https://lore.kernel.org/lkml/1377655732.3619.19.camel@joe-AO722/
|
| 871 |
|
| 872 | **MULTISTATEMENT_MACRO_USE_DO_WHILE**
|
| 873 | Macros with multiple statements should be enclosed in a
|
| 874 | do - while block. Same should also be the case for macros
|
| 875 | starting with `if` to avoid logic defects::
|
| 876 |
|
| 877 | #define macrofun(a, b, c) \
|
| 878 | do { \
|
| 879 | if (a == 5) \
|
| 880 | do_this(b, c); \
|
| 881 | } while (0)
|
| 882 |
|
| 883 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
|
| 884 |
|
| 885 | **PREFER_FALLTHROUGH**
|
| 886 | Use the `fallthrough;` pseudo keyword instead of
|
| 887 | `/* fallthrough */` like comments.
|
| 888 |
|
| 889 | **TRAILING_SEMICOLON**
|
| 890 | Macro definition should not end with a semicolon. The macro
|
| 891 | invocation style should be consistent with function calls.
|
| 892 | This can prevent any unexpected code paths::
|
| 893 |
|
| 894 | #define MAC do_something;
|
| 895 |
|
| 896 | If this macro is used within a if else statement, like::
|
| 897 |
|
| 898 | if (some_condition)
|
| 899 | MAC;
|
| 900 |
|
| 901 | else
|
| 902 | do_something;
|
| 903 |
|
| 904 | Then there would be a compilation error, because when the macro is
|
| 905 | expanded there are two trailing semicolons, so the else branch gets
|
| 906 | orphaned.
|
| 907 |
|
| 908 | See: https://lore.kernel.org/lkml/1399671106.2912.21.camel@joe-AO725/
|
| 909 |
|
| 910 | **MACRO_ARG_UNUSED**
|
| 911 | If function-like macros do not utilize a parameter, it might result
|
| 912 | in a build warning. We advocate for utilizing static inline functions
|
| 913 | to replace such macros.
|
| 914 | For example, for a macro such as the one below::
|
| 915 |
|
| 916 | #define test(a) do { } while (0)
|
| 917 |
|
| 918 | there would be a warning like below::
|
| 919 |
|
| 920 | WARNING: Argument 'a' is not used in function-like macro.
|
| 921 |
|
| 922 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
|
| 923 |
|
| 924 | **SINGLE_STATEMENT_DO_WHILE_MACRO**
|
| 925 | For the multi-statement macros, it is necessary to use the do-while
|
| 926 | loop to avoid unpredictable code paths. The do-while loop helps to
|
| 927 | group the multiple statements into a single one so that a
|
| 928 | function-like macro can be used as a function only.
|
| 929 |
|
| 930 | But for the single statement macros, it is unnecessary to use the
|
| 931 | do-while loop. Although the code is syntactically correct but using
|
| 932 | the do-while loop is redundant. So remove the do-while loop for single
|
| 933 | statement macros.
|
| 934 |
|
| 935 | **WEAK_DECLARATION**
|
| 936 | Using weak declarations like __attribute__((weak)) or __weak
|
| 937 | can have unintended link defects. Avoid using them.
|
| 938 |
|
| 939 |
|
| 940 | Functions and Variables
|
| 941 | -----------------------
|
| 942 |
|
| 943 | **CAMELCASE**
|
| 944 | Avoid CamelCase Identifiers.
|
| 945 |
|
| 946 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#naming
|
| 947 |
|
| 948 | **CONST_CONST**
|
| 949 | Using `const <type> const *` is generally meant to be
|
| 950 | written `const <type> * const`.
|
| 951 |
|
| 952 | **CONST_STRUCT**
|
| 953 | Using const is generally a good idea. Checkpatch reads
|
| 954 | a list of frequently used structs that are always or
|
| 955 | almost always constant.
|
| 956 |
|
| 957 | The existing structs list can be viewed from
|
| 958 | `scripts/const_structs.checkpatch`.
|
| 959 |
|
| 960 | See: https://lore.kernel.org/lkml/alpine.DEB.2.10.1608281509480.3321@hadrien/
|
| 961 |
|
| 962 | **EMBEDDED_FUNCTION_NAME**
|
| 963 | Embedded function names are less appropriate to use as
|
| 964 | refactoring can cause function renaming. Prefer the use of
|
| 965 | "%s", __func__ to embedded function names.
|
| 966 |
|
| 967 | Note that this does not work with -f (--file) checkpatch option
|
| 968 | as it depends on patch context providing the function name.
|
| 969 |
|
| 970 | **FUNCTION_ARGUMENTS**
|
| 971 | This warning is emitted due to any of the following reasons:
|
| 972 |
|
| 973 | 1. Arguments for the function declaration do not follow
|
| 974 | the identifier name. Example::
|
| 975 |
|
| 976 | void foo
|
| 977 | (int bar, int baz)
|
| 978 |
|
| 979 | This should be corrected to::
|
| 980 |
|
| 981 | void foo(int bar, int baz)
|
| 982 |
|
| 983 | 2. Some arguments for the function definition do not
|
| 984 | have an identifier name. Example::
|
| 985 |
|
| 986 | void foo(int)
|
| 987 |
|
| 988 | All arguments should have identifier names.
|
| 989 |
|
| 990 | **FUNCTION_WITHOUT_ARGS**
|
| 991 | Function declarations without arguments like::
|
| 992 |
|
| 993 | int foo()
|
| 994 |
|
| 995 | should be::
|
| 996 |
|
| 997 | int foo(void)
|
| 998 |
|
| 999 | **GLOBAL_INITIALISERS**
|
| 1000 | Global variables should not be initialized explicitly to
|
| 1001 | 0 (or NULL, false, etc.). Your compiler (or rather your
|
| 1002 | loader, which is responsible for zeroing out the relevant
|
| 1003 | sections) automatically does it for you.
|
| 1004 |
|
| 1005 | **INITIALISED_STATIC**
|
| 1006 | Static variables should not be initialized explicitly to zero.
|
| 1007 | Your compiler (or rather your loader) automatically does
|
| 1008 | it for you.
|
| 1009 |
|
| 1010 | **MULTIPLE_ASSIGNMENTS**
|
| 1011 | Multiple assignments on a single line makes the code unnecessarily
|
| 1012 | complicated. So on a single line assign value to a single variable
|
| 1013 | only, this makes the code more readable and helps avoid typos.
|
| 1014 |
|
| 1015 | **RETURN_PARENTHESES**
|
| 1016 | return is not a function and as such doesn't need parentheses::
|
| 1017 |
|
| 1018 | return (bar);
|
| 1019 |
|
| 1020 | can simply be::
|
| 1021 |
|
| 1022 | return bar;
|
| 1023 |
|
| 1024 | **UNINITIALIZED_PTR_WITH_FREE**
|
| 1025 | Pointers with __free attribute should be declared at the place of use
|
| 1026 | and initialized (see include/linux/cleanup.h). In this case
|
| 1027 | declarations at the top of the function rule can be relaxed. Not doing
|
| 1028 | so may lead to undefined behavior as the memory assigned (garbage,
|
| 1029 | in case not initialized) to the pointer is freed automatically when
|
| 1030 | the pointer goes out of scope.
|
| 1031 |
|
| 1032 | Also see: https://lore.kernel.org/lkml/58fd478f408a34b578ee8d949c5c4b4da4d4f41d.camel@HansenPartnership.com/
|
| 1033 |
|
| 1034 | Example::
|
| 1035 |
|
| 1036 | type var __free(free_func);
|
| 1037 | ... // var not used, but, in future someone might add a return here
|
| 1038 | var = malloc(var_size);
|
| 1039 | ...
|
| 1040 |
|
| 1041 | should be initialized as::
|
| 1042 |
|
| 1043 | ...
|
| 1044 | type var __free(free_func) = malloc(var_size);
|
| 1045 | ...
|
| 1046 |
|
| 1047 |
|
| 1048 | Permissions
|
| 1049 | -----------
|
| 1050 |
|
| 1051 | **DEVICE_ATTR_PERMS**
|
| 1052 | The permissions used in DEVICE_ATTR are unusual.
|
| 1053 | Typically only three permissions are used - 0644 (RW), 0444 (RO)
|
| 1054 | and 0200 (WO).
|
| 1055 |
|
| 1056 | See: https://www.kernel.org/doc/html/latest/filesystems/sysfs.html#attributes
|
| 1057 |
|
| 1058 | **EXECUTE_PERMISSIONS**
|
| 1059 | There is no reason for source files to be executable. The executable
|
| 1060 | bit can be removed safely.
|
| 1061 |
|
| 1062 | **EXPORTED_WORLD_WRITABLE**
|
| 1063 | Exporting world writable sysfs/debugfs files is usually a bad thing.
|
| 1064 | When done arbitrarily they can introduce serious security bugs.
|
| 1065 | In the past, some of the debugfs vulnerabilities would seemingly allow
|
| 1066 | any local user to write arbitrary values into device registers - a
|
| 1067 | situation from which little good can be expected to emerge.
|
| 1068 |
|
| 1069 | See: https://lore.kernel.org/linux-arm-kernel/[email protected]/
|
| 1070 |
|
| 1071 | **NON_OCTAL_PERMISSIONS**
|
| 1072 | Permission bits should use 4 digit octal permissions (like 0700 or 0444).
|
| 1073 | Avoid using any other base like decimal.
|
| 1074 |
|
| 1075 | **SYMBOLIC_PERMS**
|
| 1076 | Permission bits in the octal form are more readable and easier to
|
| 1077 | understand than their symbolic counterparts because many command-line
|
| 1078 | tools use this notation. Experienced kernel developers have been using
|
| 1079 | these traditional Unix permission bits for decades and so they find it
|
| 1080 | easier to understand the octal notation than the symbolic macros.
|
| 1081 | For example, it is harder to read S_IWUSR|S_IRUGO than 0644, which
|
| 1082 | obscures the developer's intent rather than clarifying it.
|
| 1083 |
|
| 1084 | See: https://lore.kernel.org/lkml/CA+55aFw5v23T-zvDZp-MmD_EYxF8WbafwwB59934FV7g21uMGQ@mail.gmail.com/
|
| 1085 |
|
| 1086 |
|
| 1087 | Spacing and Brackets
|
| 1088 | --------------------
|
| 1089 |
|
| 1090 | **ASSIGNMENT_CONTINUATIONS**
|
| 1091 | Assignment operators should not be written at the start of a
|
| 1092 | line but should follow the operand at the previous line.
|
| 1093 |
|
| 1094 | **BRACES**
|
| 1095 | The placement of braces is stylistically incorrect.
|
| 1096 | The preferred way is to put the opening brace last on the line,
|
| 1097 | and put the closing brace first::
|
| 1098 |
|
| 1099 | if (x is true) {
|
| 1100 | we do y
|
| 1101 | }
|
| 1102 |
|
| 1103 | This applies for all non-functional blocks.
|
| 1104 | However, there is one special case, namely functions: they have the
|
| 1105 | opening brace at the beginning of the next line, thus::
|
| 1106 |
|
| 1107 | int function(int x)
|
| 1108 | {
|
| 1109 | body of function
|
| 1110 | }
|
| 1111 |
|
| 1112 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces
|
| 1113 |
|
| 1114 | **BRACKET_SPACE**
|
| 1115 | Whitespace before opening bracket '[' is prohibited.
|
| 1116 | There are some exceptions:
|
| 1117 |
|
| 1118 | 1. With a type on the left::
|
| 1119 |
|
| 1120 | int [] a;
|
| 1121 |
|
| 1122 | 2. At the beginning of a line for slice initialisers::
|
| 1123 |
|
| 1124 | [0...10] = 5,
|
| 1125 |
|
| 1126 | 3. Inside a curly brace::
|
| 1127 |
|
| 1128 | = { [0...10] = 5 }
|
| 1129 |
|
| 1130 | **CONCATENATED_STRING**
|
| 1131 | Concatenated elements should have a space in between.
|
| 1132 | Example::
|
| 1133 |
|
| 1134 | printk(KERN_INFO"bar");
|
| 1135 |
|
| 1136 | should be::
|
| 1137 |
|
| 1138 | printk(KERN_INFO "bar");
|
| 1139 |
|
| 1140 | **ELSE_AFTER_BRACE**
|
| 1141 | `else {` should follow the closing block `}` on the same line.
|
| 1142 |
|
| 1143 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces
|
| 1144 |
|
| 1145 | **LINE_SPACING**
|
| 1146 | Vertical space is wasted given the limited number of lines an
|
| 1147 | editor window can display when multiple blank lines are used.
|
| 1148 |
|
| 1149 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces
|
| 1150 |
|
| 1151 | **OPEN_BRACE**
|
| 1152 | The opening brace should be following the function definitions on the
|
| 1153 | next line. For any non-functional block it should be on the same line
|
| 1154 | as the last construct.
|
| 1155 |
|
| 1156 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces
|
| 1157 |
|
| 1158 | **POINTER_LOCATION**
|
| 1159 | When using pointer data or a function that returns a pointer type,
|
| 1160 | the preferred use of * is adjacent to the data name or function name
|
| 1161 | and not adjacent to the type name.
|
| 1162 | Examples::
|
| 1163 |
|
| 1164 | char *linux_banner;
|
| 1165 | unsigned long long memparse(char *ptr, char **retptr);
|
| 1166 | char *match_strdup(substring_t *s);
|
| 1167 |
|
| 1168 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces
|
| 1169 |
|
| 1170 | **SPACING**
|
| 1171 | Whitespace style used in the kernel sources is described in kernel docs.
|
| 1172 |
|
| 1173 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces
|
| 1174 |
|
| 1175 | **TRAILING_WHITESPACE**
|
| 1176 | Trailing whitespace should always be removed.
|
| 1177 | Some editors highlight the trailing whitespace and cause visual
|
| 1178 | distractions when editing files.
|
| 1179 |
|
| 1180 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces
|
| 1181 |
|
| 1182 | **UNNECESSARY_PARENTHESES**
|
| 1183 | Parentheses are not required in the following cases:
|
| 1184 |
|
| 1185 | 1. Function pointer uses::
|
| 1186 |
|
| 1187 | (foo->bar)();
|
| 1188 |
|
| 1189 | could be::
|
| 1190 |
|
| 1191 | foo->bar();
|
| 1192 |
|
| 1193 | 2. Comparisons in if::
|
| 1194 |
|
| 1195 | if ((foo->bar) && (foo->baz))
|
| 1196 | if ((foo == bar))
|
| 1197 |
|
| 1198 | could be::
|
| 1199 |
|
| 1200 | if (foo->bar && foo->baz)
|
| 1201 | if (foo == bar)
|
| 1202 |
|
| 1203 | 3. addressof/dereference single Lvalues::
|
| 1204 |
|
| 1205 | &(foo->bar)
|
| 1206 | *(foo->bar)
|
| 1207 |
|
| 1208 | could be::
|
| 1209 |
|
| 1210 | &foo->bar
|
| 1211 | *foo->bar
|
| 1212 |
|
| 1213 | **WHILE_AFTER_BRACE**
|
| 1214 | while should follow the closing bracket on the same line::
|
| 1215 |
|
| 1216 | do {
|
| 1217 | ...
|
| 1218 | } while(something);
|
| 1219 |
|
| 1220 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces
|
| 1221 |
|
| 1222 |
|
| 1223 | Others
|
| 1224 | ------
|
| 1225 |
|
| 1226 | **CONFIG_DESCRIPTION**
|
| 1227 | Kconfig symbols should have a help text which fully describes
|
| 1228 | it.
|
| 1229 |
|
| 1230 | **CORRUPTED_PATCH**
|
| 1231 | The patch seems to be corrupted or lines are wrapped.
|
| 1232 | Please regenerate the patch file before sending it to the maintainer.
|
| 1233 |
|
| 1234 | **CVS_KEYWORD**
|
| 1235 | Since linux moved to git, the CVS markers are no longer used.
|
| 1236 | So, CVS style keywords ($Id$, $Revision$, $Log$) should not be
|
| 1237 | added.
|
| 1238 |
|
| 1239 | **DEFAULT_NO_BREAK**
|
| 1240 | switch default case is sometimes written as "default:;". This can
|
| 1241 | cause new cases added below default to be defective.
|
| 1242 |
|
| 1243 | A "break;" should be added after empty default statement to avoid
|
| 1244 | unwanted fallthrough.
|
| 1245 |
|
| 1246 | **DOS_LINE_ENDINGS**
|
| 1247 | For DOS-formatted patches, there are extra ^M symbols at the end of
|
| 1248 | the line. These should be removed.
|
| 1249 |
|
| 1250 | **DT_SCHEMA_BINDING_PATCH**
|
| 1251 | DT bindings moved to a json-schema based format instead of
|
| 1252 | freeform text.
|
| 1253 |
|
| 1254 | See: https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-schema.html
|
| 1255 |
|
| 1256 | **DT_SPLIT_BINDING_PATCH**
|
| 1257 | Devicetree bindings should be their own patch. This is because
|
| 1258 | bindings are logically independent from a driver implementation,
|
| 1259 | they have a different maintainer (even though they often
|
| 1260 | are applied via the same tree), and it makes for a cleaner history in the
|
| 1261 | DT only tree created with git-filter-branch.
|
| 1262 |
|
| 1263 | See: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
|
| 1264 |
|
| 1265 | **EMBEDDED_FILENAME**
|
| 1266 | Embedding the complete filename path inside the file isn't particularly
|
| 1267 | useful as often the path is moved around and becomes incorrect.
|
| 1268 |
|
| 1269 | **FILE_PATH_CHANGES**
|
| 1270 | Whenever files are added, moved, or deleted, the MAINTAINERS file
|
| 1271 | patterns can be out of sync or outdated.
|
| 1272 |
|
| 1273 | So MAINTAINERS might need updating in these cases.
|
| 1274 |
|
| 1275 | **MEMSET**
|
| 1276 | The memset use appears to be incorrect. This may be caused due to
|
| 1277 | badly ordered parameters. Please recheck the usage.
|
| 1278 |
|
| 1279 | **NOT_UNIFIED_DIFF**
|
| 1280 | The patch file does not appear to be in unified-diff format. Please
|
| 1281 | regenerate the patch file before sending it to the maintainer.
|
| 1282 |
|
| 1283 | **PLACEHOLDER_USE**
|
| 1284 | Detects unhandled placeholder text left in cover letters or commit headers/logs.
|
| 1285 | Common placeholders include lines like::
|
| 1286 |
|
| 1287 | *** SUBJECT HERE ***
|
| 1288 | *** BLURB HERE ***
|
| 1289 |
|
| 1290 | These typically come from autogenerated templates. Replace them with a proper
|
| 1291 | subject and description before sending.
|
| 1292 |
|
| 1293 | **PRINTF_0XDECIMAL**
|
| 1294 | Prefixing 0x with decimal output is defective and should be corrected.
|
| 1295 |
|
| 1296 | **SPDX_LICENSE_TAG**
|
| 1297 | The source file is missing or has an improper SPDX identifier tag.
|
| 1298 | The Linux kernel requires the precise SPDX identifier in all source files,
|
| 1299 | and it is thoroughly documented in the kernel docs.
|
| 1300 |
|
| 1301 | See: https://www.kernel.org/doc/html/latest/process/license-rules.html
|
| 1302 |
|
| 1303 | **TYPO_SPELLING**
|
| 1304 | Some words may have been misspelled. Consider reviewing them.
|