Code Style
Note
Armino is based on Linux kernel coding style with some adjustments or deletions to the specifications.
Indentation
Do not use spaces for indentation except for comments and documentation. Use tabs for indentation.
The preferred way to eliminate multi-level indentation in switch statements is to align the switch
and its subordinate case labels in the same column, rather than double-indenting the case labels. For example:
switch (suffix) {
case 'K':
case 'k':
mem <<= 10;
/* fall through */
default:
break;
}
Do not put multiple statements on one line, and do not put multiple assignment statements on one line.
Do not leave spaces at the end of lines.
Breaking long lines and strings
The limit on the length of lines is 80 columns, and we strongly recommend that you follow this convention.
Statements longer than 80 columns should be broken into meaningful fragments. Unless exceeding 80 columns significantly increases readability and does not hide information. Sub-fragments should be noticeably shorter than the parent fragment and noticeably right-aligned. This also applies to function headers with very long parameter lists. However, never break user-visible strings, such as printk messages, as this makes them difficult to grep.
Placing braces and spaces
Place the opening brace at the end of the line, and place the closing brace at the beginning of the line, so:
if (x is true) {
we do y
}
This applies to all non-function statement blocks (if, switch, for, while, do). For example:
switch (action) {
case KOBJ_ADD:
return "add";
default:
return NULL;
}
However, there is one exception, and that is functions: the opening brace of a function is placed at the beginning of the next line, so:
int function(int x)
{
body of function
}
Note that the closing brace occupies a line by itself, unless it is followed by the continuation of the same statement, that is, “while” in a do statement or “else” in an if statement, like this:
do {
body of do-loop
} while (condition);
and
if (x == y) {
..
} else if (x > y) {
...
} else {
....
}
When there is only a single statement, do not add unnecessary braces.
if (condition)
action();
and
if (condition)
do_this();
else
do_that();
This does not apply when only one conditional branch is a single statement; in this case, all branches must use braces:
if (condition) {
do_this();
do_that();
} else {
otherwise();
}
Spaces
The way spaces are used (mainly) depends on whether they are used for functions or keywords. (Most) keywords should be followed by a space. Notable exceptions are sizeof, typeof, alignof, and __attribute__, which to some extent look more like functions.
So put a space after these keywords:
if, switch, case, for, do, while
But do not put a space after sizeof, typeof, alignof, or __attribute__. For example:
s = sizeof(struct file);
Do not add spaces on both sides of expressions inside parentheses. This is a counterexample:
s = sizeof( struct file );
When declaring pointer types or functions that return pointer types, the preferred way to use * is to keep it close to the variable name or function name, rather than close to the type name. Examples:
char *armino_banner;
unsigned long long memparse(char *ptr, char **retptr);
char *match_strdup(substring_t *s);
Use one space on both sides of most binary and ternary operators, such as all of these operators:
= + - < > * / % | & ^ <= >= == != ? :
But do not add a space after unary operators:
& * + - ~ ! sizeof typeof alignof __attribute__ defined
Postfix increment and decrement unary operators have no space before them:
++ --
Prefix increment and decrement unary operators have no space after them:
++ --
The . and -> structure member operators have no space before or after them.
Do not leave whitespace at the end of lines. Some editors that can automatically indent will add appropriate whitespace at the beginning of new lines, and then you can type code directly on that line. However, if you do not end up typing code on that line, some editors will not remove the whitespace that has been added, as if you intentionally left a line with only whitespace. Lines containing trailing whitespace are created in this way.
Typedef
Do not use things like vps_t.
Using typedef for structures and pointers is an error. When you see in code:
vps_t a;
What does this mean?
On the contrary, if it is like this:
struct virtual_container *a;
You know what a is.
Many people think that typedef improves readability. Actually, this is not the case. They are only useful in the following situations:
Completely opaque objects (in this case, typedef should be actively used to hide what the object actually is).
For example: Opaque objects like
pte_t, which can only be accessed using appropriate accessor functions.Note
Opacity and “accessor functions” are bad in themselves. The reason we use types like pte_t is that there really is no common accessible information at all.
Clear integer types, such that this abstraction can help eliminate confusion about whether it is
intorlong.u8/u16/u32 are perfectly fine typedefs, but they fit better into category (d) than here.
Note
To do this, there must be a reason. If a variable is
unsigned long, then there is no need fortypedef unsigned long myflags_t;
However, if there is a clear reason, such as it might be an
unsigned intin some cases andunsigned longin other cases, then do not hesitate, please use typedef.When you use sparse to literally create a new type for type checking.
Types identical to standard C99 types, in some exceptional cases.
Although it does not take much time for the eyes and brain to adapt to new standard types such as
uint32_t, some people still refuse to use them.Therefore, Armino-specific types equivalent to standard types such as
u8/u16/u32/u64and their signed versions are allowed—although they are not mandatory in your own new code.When editing existing code that already uses a certain type set, you should follow the choices already made in that code.
There may be other cases, but the basic rule is to never use typedef unless you can clearly apply one of the rules above.
In general, if a pointer or an element in a structure can reasonably be directly accessed, then it should not be a typedef.
Functions
Functions should be short and beautiful, and do only one thing. Functions should be able to be displayed on one or two screens, do one thing, and do it well.
The maximum length of a function is inversely proportional to the complexity and indentation level of that function. So, if you have a theoretically very simple function with only one long (but simple) case statement, and you need to do many small things in each case, such a function is acceptable even though it is long.
However, if you have a complex function, and you suspect that a not-so-talented high school freshman might not even understand the purpose of this function, you should strictly follow the length limit mentioned above. Use helper functions and give them descriptive names (if you think their performance is important, you can let the compiler inline them, which often works better than writing a complex function).
Another measure of functions is the number of local variables. This number should not exceed 5-10, otherwise your function has problems. Reconsider your function and split it into smaller functions. The human brain can generally easily track 7 different things at the same time. If there are more, it will get confused. Even if you are very smart, you may not remember what you did 2 weeks ago.
In source files, use blank lines to separate different functions.
In function prototypes, include function names and their data types.
Centralized exiting of functions
Although some claim it is outdated, the equivalent of goto statements is still frequently used by compilers, in the form of unconditional jump instructions.
When a function exits from multiple positions and needs to perform common operations such as cleanup, goto statements are convenient. If no cleanup is needed, then just return directly.
Choose a label name that explains the behavior of goto or why it exists. If goto is to free buffer, a good name could be out_free_buffer:. Do not use GW_BASIC names like err1: and err2:, because once you add or delete (function) exit paths, you must renumber them, which makes it difficult to verify correctness.
The reasons for using goto are:
Unconditional statements are easy to understand and track
Reduced nesting level
Can avoid errors caused by forgetting to update individual exit points when modifying
Saves the compiler from deleting redundant code ;)
int fun(int a)
{
int result = 0;
char *buffer;
buffer = malloc(SIZE);
if (!buffer)
return BK_ERR_NO_MEM;
if (condition1) {
while (loop1) {
...
}
result = 1;
goto out_free_buffer;
}
...
out_free_buffer:
free(buffer);
return result;
}
A common mistake to watch out for is one err error, like this:
err:
free(foo->bar);
free(foo);
return ret;
The error in this code is that foo is NULL on some exit paths. Usually, this error is fixed by separating it into two error labels err_free_bar: and err_free_foo::
err_free_bar:
free(foo->bar);
err_free_foo:
free(foo);
return ret;
Ideally, you should simulate errors to test all exit paths.
Macros and enumerations
Macro names used to define constants and labels in enumerations should be uppercase.
#define CONSTANT 0x12345
When defining several related constants, it is better to use enumerations.
Macro names should be uppercase, but macro names that look like functions can be lowercase.
Generally, if it can be written as an inline function, do not write it as a function-like macro.
Macros containing 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)
Things to avoid when using macros:
Macros that affect control flow:
#define FOO(x) \
do { \
if (blah(x) < 0) \
return -EBUGGERED; \
} while (0)
Very bad. It looks like a function, but can cause the function that calls it to exit; do not confuse the parser in the reader’s brain.
Macros that depend on a local variable with a fixed name:
#define FOO(val) bar(index, val)
It may look like a good thing, but it is very easy to confuse code readers and can lead to errors from seemingly unrelated changes.
Parameterized macros used as lvalues: FOO(x) = y; if someone turns FOO into an inline function, this usage will be wrong.
Forgetting precedence: macros that use expressions to define constants must place the expression within a pair of parentheses. Parameterized macros should also pay attention to this issue.
#define CONSTANT 0x4000
#define CONSTEXP (CONSTANT | 3)
Naming conflicts when defining function-like local variables in macros:
#define FOO(x) \
({ \
typeof(x) ret; \
ret = calc_ret(x); \
(ret); \
})
ret is a common name for local variables—__foo_ret is less likely to conflict with an existing variable.
Allocating memory
When allocating memory, the preferred way to pass the structure size is like this:
p = malloc(sizeof(*p), ...);
In another way of passing, the operand of sizeof is the name of the structure, which reduces readability and may introduce bugs. It is possible that when the pointer variable type is changed, the result of sizeof passed to the memory allocation function remains unchanged.
Casting a void pointer return value is redundant. The C language itself guarantees that conversion from a void pointer to any other pointer type is fine.
Inline disease
There is a common misconception that inline is an option provided by gcc that can make code run faster. Although using inline functions is sometimes appropriate, in many cases it is not. Overuse of inline will make code larger, causing it to occupy more instruction cache, thus slowing down the entire system.
A basic principle is that if a function has more than 3 lines, do not make it an inline function. An exception to this principle is if you know that a certain parameter is a compile-time constant, and because of this constant you are sure that the compiler can optimize away most of your function’s code at compile time, then you can still add the inline keyword to it.
People often advocate adding inline to static functions that are used only once, so there is no loss, because there is nothing to trade off. Although this is technically correct, in practice gcc can automatically inline it even without inline in this case. Moreover, other users may request the removal of inline, and the resulting debate will offset the potential value of inline itself, which is not worth it.
Conditional compilation
Whenever possible, do not use preprocessor conditions (#if, #ifdef) in .c files; doing so makes code harder to read and harder to track logic. The alternative is to use preprocessor conditions in header files to provide those .c files, and provide a no-op stub version for #else, and then unconditionally call those (defined in header files) functions in .c files. This way, the compiler will avoid generating any code for stub function calls, producing the same result, but the logic will be clearer.
Prefer to compile entire functions, rather than part of a function or part of an expression. Instead of putting an ifdef in an expression, factor out part or all of the expression into a separate helper function and apply preprocessor conditions to that helper function.
If you have a function or variable that may become unused in a specific configuration, the compiler will warn that it is defined but unused. Mark it as __maybe_unused rather than including it in a preprocessor condition. (However, if a function or variable is always unused, just delete it.)
In code, use the IS_ENABLED macro as much as possible to convert a Kconfig flag to a C boolean expression, and use it in general C conditions:
if (IS_ENABLED(CONFIG_SOMETHING)) {
...
}
The compiler will do constant folding, and then include or exclude code blocks just like using #ifdef, so this will not bring any runtime overhead. However, this method still allows the C compiler to view the code in the block and check its correctness (syntax, types, symbol references, etc.). Therefore, if the condition is not satisfied and the referenced symbols in the block do not exist, you must still use #ifdef.
At the end of any meaningful #if or #ifdef block (more than a few lines), write a comment on the same line after #endif to comment on this conditional expression. For example:
#ifdef CONFIG_SOMETHING
...
#endif /* CONFIG_SOMETHING */
Comments
Comments are good, but there is a danger of over-commenting. Never explain in comments how your code works: it is better to make your code so clear that people can understand it at a glance. Explaining poorly written code is a waste of time.
Generally, you want your comments to tell people what your code does, not how it does it. Also, please do not put comments inside a function body: if a function is so complex that you need to separately comment parts of it, you probably need to go back to Functions. You can make small comments to note or warn about certain clever (or bad) practices, but do not add too many. What you should do is put comments at the head of the function to tell people what it does, and you can also add the reason why it does these things.
When commenting kernel API functions, please refer to the documentation specifications.
The style for long (multi-line) comments is:
Commenting data is also important, whether it is a basic type or a derived type. To facilitate this, each line should declare only one data item (do not use commas to declare multiple data items at once). This way you have room to write a small comment for each data item explaining their purpose.