--- zzzz-none-000/linux-3.10.107/Documentation/CodingStyle 2017-06-27 09:49:32.000000000 +0000 +++ scorpion-7490-727/linux-3.10.107/Documentation/CodingStyle 2021-02-04 17:41:59.000000000 +0000 @@ -13,7 +13,7 @@ Anyway, here goes: - Chapter 1: Indentation + Chapter 1: Indentation Tabs are 8 characters, and thus indentations are also 8 characters. There are heretic movements that try to make indentations 4 (or even 2!) @@ -56,7 +56,6 @@ break; } - Don't put multiple statements on a single line unless you have something to hide: @@ -156,25 +155,25 @@ Do not unnecessarily use braces where a single statement will do. -if (condition) - action(); + if (condition) + action(); and -if (condition) - do_this(); -else - do_that(); + if (condition) + do_this(); + else + do_that(); This does not apply if only one branch of a conditional statement is a single statement; in the latter case use braces in both branches: -if (condition) { - do_this(); - do_that(); -} else { - otherwise(); -} + if (condition) { + do_this(); + do_that(); + } else { + otherwise(); + } 3.1: Spaces @@ -186,8 +185,11 @@ "struct fileinfo info;" is declared). So use a space after these keywords: + if, switch, case, for, do, while + but not with sizeof, typeof, alignof, or __attribute__. E.g., + s = sizeof(struct file); Do not add spaces around (inside) parenthesized expressions. This example is @@ -209,12 +211,15 @@ = + - < > * / % | & ^ <= >= == != ? : but no space after unary operators: + & * + - ~ ! sizeof typeof alignof __attribute__ defined no space before the postfix increment & decrement unary operators: + ++ -- no space after the prefix increment & decrement unary operators: + ++ -- and no space around the '.' and "->" structure member operators. @@ -268,13 +273,11 @@ Chapter 5: Typedefs Please don't use things like "vps_t". - It's a _mistake_ to use typedef for structures and pointers. When you see a vps_t a; in the source, what does it mean? - In contrast, if it says struct virtual_container *a; @@ -372,11 +375,11 @@ exported, the EXPORT* macro for it should follow immediately after the closing function brace line. E.g.: -int system_is_up(void) -{ - return system_state == SYSTEM_RUNNING; -} -EXPORT_SYMBOL(system_is_up); + int system_is_up(void) + { + return system_state == SYSTEM_RUNNING; + } + EXPORT_SYMBOL(system_is_up); In function prototypes, include parameter names with their data types. Although this is not required by the C language, it is preferred in Linux @@ -389,9 +392,15 @@ used frequently by compilers in form of the unconditional jump instruction. The goto statement comes in handy when a function exits from multiple -locations and some common work such as cleanup has to be done. +locations and some common work such as cleanup has to be done. If there is no +cleanup needed then just return directly. + +Choose label names which say what the goto does or why the goto exists. An +example of a good name could be "out_buffer:" if the goto frees "buffer". Avoid +using GW-BASIC names like "err1:" and "err2:". Also don't name them after the +goto location like "err_kmalloc_failed:" -The rationale is: +The rationale for using gotos is: - unconditional statements are easier to understand and follow - nesting is reduced @@ -399,26 +408,38 @@ modifications are prevented - saves the compiler work to optimize redundant code away ;) -int fun(int a) -{ - int result = 0; - char *buffer = kmalloc(SIZE); - - if (buffer == NULL) - return -ENOMEM; - - if (condition1) { - while (loop1) { - ... + int fun(int a) + { + int result = 0; + char *buffer; + + buffer = kmalloc(SIZE, GFP_KERNEL); + if (!buffer) + return -ENOMEM; + + if (condition1) { + while (loop1) { + ... + } + result = 1; + goto out_buffer; } - result = 1; - goto out; + ... + out_buffer: + kfree(buffer); + return result; } - ... -out: - kfree(buffer); - return result; -} + +A common type of bug to be aware of it "one err bugs" which look like this: + + err: + kfree(foo->bar); + kfree(foo); + return ret; + +The bug in this code is that on some exit paths "foo" is NULL. Normally the +fix for this is to split it up into two error labels "err_bar:" and "err_foo:". + Chapter 8: Commenting @@ -485,9 +506,9 @@ (defun c-lineup-arglist-tabs-only (ignored) "Line up argument lists by tabs, not spaces" (let* ((anchor (c-langelem-pos c-syntactic-element)) - (column (c-langelem-2nd-pos c-syntactic-element)) - (offset (- (1+ column) anchor)) - (steps (floor offset c-basic-offset))) + (column (c-langelem-2nd-pos c-syntactic-element)) + (offset (- (1+ column) anchor)) + (steps (floor offset c-basic-offset))) (* (max steps 1) c-basic-offset))) @@ -509,6 +530,7 @@ (string-match (expand-file-name "~/src/linux-trees") filename)) (setq indent-tabs-mode t) + (setq show-trailing-whitespace t) (c-set-style "linux-tabs-only"))))) This will make emacs go better with the kernel coding style for C @@ -593,7 +615,7 @@ Names of macros defining constants and labels in enums are capitalized. -#define CONSTANT 0x12345 + #define CONSTANT 0x12345 Enums are preferred when defining several related constants. @@ -604,28 +626,28 @@ Macros with multiple statements should be enclosed in a do - while block: -#define macrofun(a, b, c) \ - do { \ - if (a == 5) \ - do_this(b, c); \ - } while (0) + #define macrofun(a, b, c) \ + do { \ + if (a == 5) \ + do_this(b, c); \ + } while (0) Things to avoid when using macros: 1) macros that affect control flow: -#define FOO(x) \ - do { \ - if (blah(x) < 0) \ - return -EBUGGERED; \ - } while(0) + #define FOO(x) \ + do { \ + if (blah(x) < 0) \ + return -EBUGGERED; \ + } while(0) is a _very_ bad idea. It looks like a function call but exits the "calling" function; don't break the internal parsers of those who will read the code. 2) macros that depend on having a local variable with a magic name: -#define FOO(val) bar(index, val) + #define FOO(val) bar(index, val) might look like a good thing, but it's confusing as hell when one reads the code and it's prone to breakage from seemingly innocent changes. @@ -637,8 +659,21 @@ must enclose the expression in parentheses. Beware of similar issues with macros using parameters. -#define CONSTANT 0x4000 -#define CONSTEXP (CONSTANT | 3) + #define CONSTANT 0x4000 + #define CONSTEXP (CONSTANT | 3) + +5) namespace collisions when defining local variables in macros resembling +functions: + +#define FOO(x) \ +({ \ + typeof(x) ret; \ + ret = calc_ret(x); \ + (ret); \ +}) + +ret is a common name for a local variable - __foo_ret is less likely +to collide with an existing variable. The cpp manual deals with macros exhaustively. The gcc internals manual also covers RTL which is used frequently with assembly language in the kernel. @@ -659,15 +694,23 @@ which you should use to make sure messages are matched to the right device and driver, and are tagged with the right level: dev_err(), dev_warn(), dev_info(), and so forth. For messages that aren't associated with a -particular device, defines pr_debug() and pr_info(). +particular device, defines pr_notice(), pr_info(), +pr_warn(), pr_err(), etc. Coming up with good debugging messages can be quite a challenge; and once -you have them, they can be a huge help for remote troubleshooting. Such -messages should be compiled out when the DEBUG symbol is not defined (that -is, by default they are not included). When you use dev_dbg() or pr_debug(), -that's automatic. Many subsystems have Kconfig options to turn on -DDEBUG. -A related convention uses VERBOSE_DEBUG to add dev_vdbg() messages to the -ones already enabled by DEBUG. +you have them, they can be a huge help for remote troubleshooting. However +debug message printing is handled differently than printing other non-debug +messages. While the other pr_XXX() functions print unconditionally, +pr_debug() does not; it is compiled out by default, unless either DEBUG is +defined or CONFIG_DYNAMIC_DEBUG is set. That is true for dev_dbg() also, +and a related convention uses VERBOSE_DEBUG to add dev_vdbg() messages to +the ones already enabled by DEBUG. + +Many subsystems have Kconfig debug options to turn on -DDEBUG in the +corresponding Makefile; in other cases specific files #define DEBUG. And +when a debug message should be unconditionally printed, such as if it is +already inside a debug-related #ifdef section, printk(KERN_DEBUG ...) can be +used. Chapter 14: Allocating memory @@ -769,11 +812,11 @@ For example, if you need to calculate the length of an array, take advantage of the macro - #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) + #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) Similarly, if you need to calculate the size of some structure member, use - #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f)) + #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f)) There are also min() and max() macros that do strict type checking if you need them. Feel free to peruse that header file to see what else is already @@ -786,19 +829,19 @@ indicated with special markers. For example, emacs interprets lines marked like this: --*- mode: c -*- + -*- mode: c -*- Or like this: -/* -Local Variables: -compile-command: "gcc -DMAGIC_DEBUG_FLAG foo.c" -End: -*/ + /* + Local Variables: + compile-command: "gcc -DMAGIC_DEBUG_FLAG foo.c" + End: + */ Vim interprets markers that look like this: -/* vim:set sw=8 noet */ + /* vim:set sw=8 noet */ Do not include any of these in source files. People have their own personal editor configurations, and your source files should not override them. This @@ -836,6 +879,49 @@ : /* outputs */ : /* inputs */ : /* clobbers */); + Chapter 20: Conditional Compilation + +Wherever possible, don't use preprocessor conditionals (#if, #ifdef) in .c +files; doing so makes code harder to read and logic harder to follow. Instead, +use such conditionals in a header file defining functions for use in those .c +files, providing no-op stub versions in the #else case, and then call those +functions unconditionally from .c files. The compiler will avoid generating +any code for the stub calls, producing identical results, but the logic will +remain easy to follow. + +Prefer to compile out entire functions, rather than portions of functions or +portions of expressions. Rather than putting an ifdef in an expression, factor +out part or all of the expression into a separate helper function and apply the +conditional to that function. + +If you have a function or variable which may potentially go unused in a +particular configuration, and the compiler would warn about its definition +going unused, mark the definition as __maybe_unused rather than wrapping it in +a preprocessor conditional. (However, if a function or variable *always* goes +unused, delete it.) + +Within code, where possible, use the IS_ENABLED macro to convert a Kconfig +symbol into a C boolean expression, and use it in a normal C conditional: + + if (IS_ENABLED(CONFIG_SOMETHING)) { + ... + } + +The compiler will constant-fold the conditional away, and include or exclude +the block of code just as with an #ifdef, so this will not add any runtime +overhead. However, this approach still allows the C compiler to see the code +inside the block, and check it for correctness (syntax, types, symbol +references, etc). Thus, you still have to use an #ifdef if the code inside the +block references symbols that will not exist if the condition is not met. + +At the end of any non-trivial #if or #ifdef block (more than a few lines), +place a comment after the #endif on the same line, noting the conditional +expression used. For instance: + + #ifdef CONFIG_SOMETHING + ... + #endif /* CONFIG_SOMETHING */ + Appendix I: References @@ -843,13 +929,11 @@ by Brian W. Kernighan and Dennis M. Ritchie. Prentice Hall, Inc., 1988. ISBN 0-13-110362-8 (paperback), 0-13-110370-9 (hardback). -URL: http://cm.bell-labs.com/cm/cs/cbook/ The Practice of Programming by Brian W. Kernighan and Rob Pike. Addison-Wesley, Inc., 1999. ISBN 0-201-61586-X. -URL: http://cm.bell-labs.com/cm/cs/tpop/ GNU manuals - where in compliance with K&R and this text - for cpp, gcc, gcc internals and indent, all available from http://www.gnu.org/manual/