Commit | Line | Data |
---|---|---|
2eecbab8 DR |
1 | .. SPDX-License-Identifier: GPL-2.0-only |
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 <random@developer.example.org> | |
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 | .. Types in this section are also parsed by checkpatch. | |
239 | .. The types are grouped into subsections based on use. | |
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. | |
76001b8b | 249 | |
2eecbab8 DR |
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. | |
76001b8b | 268 | |
2eecbab8 DR |
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 | ||
2eecbab8 DR |
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. | |
76001b8b | 289 | |
2eecbab8 DR |
290 | See: https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on |
291 | ||
2eecbab8 DR |
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. | |
76001b8b | 298 | |
2eecbab8 DR |
299 | See: https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull |
300 | ||
91a1265c DR |
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/20070930191054.GA15876@uranus.ravnborg.org/ | |
360 | 2. https://lore.kernel.org/lkml/1313384834-24433-12-git-send-email-lacombar@gmail.com/ | |
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/20190430180111.10688-1-mcroce@redhat.com/ | |
420 | 2. https://lore.kernel.org/lkml/20190531131422.14970-1-mcroce@redhat.com/ | |
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/20200510182252.GA411829@lunn.ch/ | |
434 | ||
435 | **EXPORT_SYMBOL** | |
436 | EXPORT_SYMBOL should immediately follow the symbol to be exported. | |
437 | ||
76001b8b DR |
438 | **IN_ATOMIC** |
439 | in_atomic() is not for driver use so any such use is reported as an ERROR. | |
91a1265c DR |
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. | |
76001b8b DR |
443 | |
444 | However, in_atomic() is ok for core kernel use. | |
445 | ||
446 | See: https://lore.kernel.org/lkml/20080320201723.b87b3732.akpm@linux-foundation.org/ | |
447 | ||
3337c3a1 DR |
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. | |
76001b8b | 452 | |
3337c3a1 DR |
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() | |
76001b8b | 463 | |
3337c3a1 DR |
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 | ||
76001b8b DR |
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 | ||
2eecbab8 | 475 | |
91a1265c DR |
476 | Comments |
477 | -------- | |
2eecbab8 DR |
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. | |
76001b8b | 500 | |
2eecbab8 DR |
501 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting |
502 | ||
91a1265c DR |
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/20200401101714.44781-1-elver@google.com/ | |
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 | ||
2eecbab8 | 518 | |
2eecbab8 DR |
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. | |
76001b8b | 525 | |
2eecbab8 DR |
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. stable@vger.kernel.org | |
533 | 2. stable@kernel.org | |
534 | ||
535 | For adding version info, the following comment style should be used:: | |
536 | ||
537 | stable@vger.kernel.org # 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. | |
76001b8b | 547 | |
2eecbab8 DR |
548 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes |
549 | ||
91a1265c DR |
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 | ||
76001b8b DR |
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 | ||
2eecbab8 DR |
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. | |
76001b8b | 573 | |
2eecbab8 DR |
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. | |
76001b8b | 582 | |
2eecbab8 DR |
583 | See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin |
584 | ||
3337c3a1 DR |
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. | |
76001b8b | 590 | |
3337c3a1 DR |
591 | See: https://lore.kernel.org/lkml/20150611134006.9df79a893e3636019ad2759e@linux-foundation.org/ |
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 <author@example.com> | |
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 | ||
2eecbab8 DR |
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. | |
76001b8b | 633 | |
2eecbab8 DR |
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 | ||
91a1265c DR |
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 | ||
d9548979 UV |
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 | ||
250a0a5b UV |
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 | ||
91a1265c DR |
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 | ||
3337c3a1 DR |
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). | |
0e7c52da | 789 | The BIT() macro is defined via include/linux/bits.h:: |
3337c3a1 DR |
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. | |
76001b8b | 809 | |
3337c3a1 DR |
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. | |
76001b8b | 820 | |
3337c3a1 DR |
821 | See: https://lore.kernel.org/lkml/CA+55aFycQ9XJvEOsiM3txHL5bjUc8CeKWJNR_H+MiicaddB42Q@mail.gmail.com/ |
822 | ||
91a1265c DR |
823 | **DO_WHILE_MACRO_WITH_TRAILING_SEMICOLON** |
824 | do {} while(0) macros should not have a trailing semicolon. | |
825 | ||
3337c3a1 DR |
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 | ||
76001b8b DR |
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 | ||
3337c3a1 DR |
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 | ||
76001b8b DR |
877 | **PREFER_FALLTHROUGH** |
878 | Use the `fallthrough;` pseudo keyword instead of | |
879 | `/* fallthrough */` like comments. | |
880 | ||
29bd0cac UV |
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 | ||
250a0a5b UV |
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 | ||
3337c3a1 DR |
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. | |
76001b8b | 923 | |
3337c3a1 DR |
924 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#naming |
925 | ||
91a1265c DR |
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 | ||
3337c3a1 DR |
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 | ||
250a0a5b UV |
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 | ||
3337c3a1 DR |
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 | ||
76001b8b DR |
1003 | Permissions |
1004 | ----------- | |
1005 | ||
91a1265c DR |
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 | ||
76001b8b DR |
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/cover.1296818921.git.segoon@openwall.com/ | |
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 | ||
3454cd56 UV |
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 | ||
76001b8b | 1041 | |
2eecbab8 DR |
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) { | |
3337c3a1 | 1055 | we do y |
2eecbab8 DR |
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 | { | |
3337c3a1 | 1064 | body of function |
2eecbab8 DR |
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 | ||
76001b8b | 1075 | int [] a; |
2eecbab8 DR |
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 | ||
2eecbab8 DR |
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 | ||
3337c3a1 DR |
1095 | **ELSE_AFTER_BRACE** |
1096 | `else {` should follow the closing block `}` on the same line. | |
76001b8b | 1097 | |
3337c3a1 DR |
1098 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces |
1099 | ||
2eecbab8 DR |
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. | |
76001b8b | 1103 | |
2eecbab8 DR |
1104 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces |
1105 | ||
3337c3a1 DR |
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. | |
76001b8b | 1110 | |
3337c3a1 DR |
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 | ||
2eecbab8 DR |
1125 | **SPACING** |
1126 | Whitespace style used in the kernel sources is described in kernel docs. | |
76001b8b | 1127 | |
2eecbab8 DR |
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. | |
76001b8b | 1134 | |
2eecbab8 DR |
1135 | See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces |
1136 | ||
76001b8b | 1137 | **UNNECESSARY_PARENTHESES** |
91a1265c | 1138 | Parentheses are not required in the following cases: |
76001b8b DR |
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 | ||
3337c3a1 DR |
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 | ||
2eecbab8 DR |
1177 | |
1178 | Others | |
1179 | ------ | |
1180 | ||
2eecbab8 DR |
1181 | **CONFIG_DESCRIPTION** |
1182 | Kconfig symbols should have a help text which fully describes | |
1183 | it. | |
3337c3a1 DR |
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 | ||
91a1265c DR |
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 | ||
3337c3a1 DR |
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 | ||
91a1265c DR |
1205 | **DT_SCHEMA_BINDING_PATCH** |
1206 | DT bindings moved to a json-schema based format instead of | |
1207 | freeform text. | |
3337c3a1 | 1208 | |
91a1265c | 1209 | See: https://www.kernel.org/doc/html/latest/devicetree/bindings/writing-schema.html |
76001b8b | 1210 | |
91a1265c DR |
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. | |
76001b8b | 1217 | |
91a1265c | 1218 | See: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters |
76001b8b | 1219 | |
91a1265c DR |
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. | |
76001b8b | 1223 | |
91a1265c DR |
1224 | **FILE_PATH_CHANGES** |
1225 | Whenever files are added, moved, or deleted, the MAINTAINERS file | |
1226 | patterns can be out of sync or outdated. | |
76001b8b | 1227 | |
91a1265c | 1228 | So MAINTAINERS might need updating in these cases. |
76001b8b DR |
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. | |
3337c3a1 DR |
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 | ||
76001b8b DR |
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 | ||
76001b8b DR |
1248 | **TYPO_SPELLING** |
1249 | Some words may have been misspelled. Consider reviewing them. |