diff options
author | Pavel Kretov <firegurafiku@gmail.com> | 2015-02-16 20:26:18 +0300 |
---|---|---|
committer | Jonathan Corbet <corbet@lwn.net> | 2015-02-27 15:02:02 -0700 |
commit | 09677e0ff8a115162cfa763b7ad2d753f11fce9f (patch) | |
tree | 17fb822a0799d53309cdd295019596cd12525167 | |
parent | 696156f03f97aa3be4ec5f8d85ff3465bbf404fe (diff) |
Documentation/CodingStyle: improve text layout
Try to make coding style documentation look a bit more readable and
consistent with the following:
- indent every code example in C to first tab-stop;
- surround every code example with empty lines, both top and bottom;
- remove empty lines where text looked way too spare;
- do not indent examples in elisp and kconfig;
- do not do any non-whitespace changes.
Signed-off-by: Pavel Kretov <firegurafiku@gmail.com>
Signed-off-by: Jonathan Corbet <corbet@lwn.net>
-rw-r--r-- | Documentation/CodingStyle | 141 |
1 files changed, 72 insertions, 69 deletions
diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle index 6e0b7b99df18..e55accfca276 100644 --- a/Documentation/CodingStyle +++ b/Documentation/CodingStyle @@ -56,7 +56,6 @@ instead of "double-indenting" the "case" labels. E.g.: break; } - Don't put multiple statements on a single line unless you have something to hide: @@ -156,25 +155,25 @@ comments on. 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 @@ although they are not required in the language, as in: "sizeof info" after "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 @@ such as any of these: = + - < > * / % | & ^ <= >= == != ? : 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 @@ See chapter 6 (Functions). 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 @@ In source files, separate functions with one blank line. If the function is 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 @@ -405,34 +408,34 @@ The rationale for using gotos is: modifications are prevented - saves the compiler work to optimize redundant code away ;) -int fun(int a) -{ - int result = 0; - char *buffer; - - buffer = kmalloc(SIZE, GFP_KERNEL); - if (!buffer) - 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_buffer; + ... + out_buffer: + kfree(buffer); + return result; } - ... -out_buffer: - 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; + 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:". @@ -612,7 +615,7 @@ have a reference count on it, you almost certainly have a bug. 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. @@ -623,28 +626,28 @@ Generally, inline functions are preferable to macros resembling functions. 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. @@ -656,8 +659,8 @@ bite you if somebody e.g. turns FOO into an inline function. 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) The cpp manual deals with macros exhaustively. The gcc internals manual also covers RTL which is used frequently with assembly language in the kernel. @@ -796,11 +799,11 @@ you should use, rather than explicitly coding some variant of them yourself. 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 @@ -813,19 +816,19 @@ Some editors can interpret configuration information embedded in source files, 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 @@ -902,9 +905,9 @@ 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 */ + #ifdef CONFIG_SOMETHING + ... + #endif /* CONFIG_SOMETHING */ Appendix I: References |