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