mirror of
https://github.com/openvswitch/ovs
synced 2025-08-22 09:58:01 +00:00
513 lines
17 KiB
Plaintext
513 lines
17 KiB
Plaintext
Open vSwitch Coding Style
|
|
=========================
|
|
|
|
This file describes the coding style used in most C files in the Open
|
|
vSwitch distribution. However, Linux kernel code datapath directory
|
|
follows the Linux kernel's established coding conventions.
|
|
|
|
BASICS
|
|
|
|
Limit lines to 79 characters.
|
|
|
|
Use form feeds (control+L) to divide long source files into logical
|
|
pieces. A form feed should appear as the only character on a line.
|
|
|
|
Do not use tabs for indentation.
|
|
|
|
Avoid trailing spaces on lines.
|
|
|
|
|
|
NAMING
|
|
|
|
Use names that explain the purpose of a function or object.
|
|
|
|
Use underscores to separate words in an identifier: multi_word_name.
|
|
|
|
Use lowercase for most names. Use uppercase for macros, macro
|
|
parameters, and members of enumerations.
|
|
|
|
Give arrays names that are plural.
|
|
|
|
Pick a unique name prefix (ending with an underscore) for each
|
|
module, and apply that prefix to all of that module's externally
|
|
visible names. Names of macro parameters, struct and union members,
|
|
and parameters in function prototypes are not considered externally
|
|
visible for this purpose.
|
|
|
|
Do not use names that begin with _. If you need a name for
|
|
"internal use only", use __ as a suffix instead of a prefix.
|
|
|
|
Avoid negative names: "found" is a better name than "not_found".
|
|
|
|
In names, a "size" is a count of bytes, a "length" is a count of
|
|
characters. A buffer has size, but a string has length. The length
|
|
of a string does not include the null terminator, but the size of the
|
|
buffer that contains the string does.
|
|
|
|
|
|
COMMENTS
|
|
|
|
Comments should be written as full sentences that start with a
|
|
capital letter and end with a period. Put two spaces between
|
|
sentences.
|
|
|
|
Write block comments as shown below. You may put the /* and */ on
|
|
the same line as comment text if you prefer.
|
|
|
|
/*
|
|
* We redirect stderr to /dev/null because we often want to remove all
|
|
* traffic control configuration on a port so its in a known state. If
|
|
* this done when there is no such configuration, tc complains, so we just
|
|
* always ignore it.
|
|
*/
|
|
|
|
Each function and each variable declared outside a function, and
|
|
each struct, union, and typedef declaration should be preceded by a
|
|
comment. See FUNCTION DEFINITIONS below for function comment
|
|
guidelines.
|
|
|
|
Each struct and union member should each have an inline comment that
|
|
explains its meaning. structs and unions with many members should be
|
|
additionally divided into logical groups of members by block comments,
|
|
e.g.:
|
|
|
|
/* An event that will wake the following call to poll_block(). */
|
|
struct poll_waiter {
|
|
/* Set when the waiter is created. */
|
|
struct list node; /* Element in global waiters list. */
|
|
int fd; /* File descriptor. */
|
|
short int events; /* Events to wait for (POLLIN, POLLOUT). */
|
|
poll_fd_func *function; /* Callback function, if any, or null. */
|
|
void *aux; /* Argument to callback function. */
|
|
struct backtrace *backtrace; /* Event that created waiter, or null. */
|
|
|
|
/* Set only when poll_block() is called. */
|
|
struct pollfd *pollfd; /* Pointer to element of the pollfds array
|
|
(null if added from a callback). */
|
|
};
|
|
|
|
Use XXX or FIXME comments to mark code that needs work.
|
|
|
|
Don't use // comments.
|
|
|
|
Don't comment out or #if 0 out code. Just remove it. The code that
|
|
was there will still be in version control history.
|
|
|
|
|
|
FUNCTIONS
|
|
|
|
Put the return type, function name, and the braces that surround the
|
|
function's code on separate lines, all starting in column 0.
|
|
|
|
Before each function definition, write a comment that describes the
|
|
function's purpose, including each parameter, the return value, and
|
|
side effects. References to argument names should be given in
|
|
single-quotes, e.g. 'arg'. The comment should not include the
|
|
function name, nor need it follow any formal structure. The comment
|
|
does not need to describe how a function does its work, unless this
|
|
information is needed to use the function correctly (this is often
|
|
better done with comments *inside* the function).
|
|
|
|
Simple static functions do not need a comment.
|
|
|
|
Within a file, non-static functions should come first, in the order
|
|
that they are declared in the header file, followed by static
|
|
functions. Static functions should be in one or more separate pages
|
|
(separated by form feed characters) in logical groups. A commonly
|
|
useful way to divide groups is by "level", with high-level functions
|
|
first, followed by groups of progressively lower-level functions.
|
|
This makes it easy for the program's reader to see the top-down
|
|
structure by reading from top to bottom.
|
|
|
|
All function declarations and definitions should include a
|
|
prototype. Empty parentheses, e.g. "int foo();", do not include a
|
|
prototype (they state that the function's parameters are unknown);
|
|
write "void" in parentheses instead, e.g. "int foo(void);".
|
|
|
|
Prototypes for static functions should either all go at the top of
|
|
the file, separated into groups by blank lines, or they should appear
|
|
at the top of each page of functions. Don't comment individual
|
|
prototypes, but a comment on each group of prototypes is often
|
|
appropriate.
|
|
|
|
In the absence of good reasons for another order, the following
|
|
parameter order is preferred. One notable exception is that data
|
|
parameters and their corresponding size parameters should be paired.
|
|
|
|
1. The primary object being manipulated, if any (equivalent to the
|
|
"this" pointer in C++).
|
|
2. Input-only parameters.
|
|
3. Input/output parameters.
|
|
4. Output-only parameters.
|
|
5. Status parameter.
|
|
|
|
Example:
|
|
|
|
/* Stores the features supported by 'netdev' into each of '*current',
|
|
* '*advertised', '*supported', and '*peer' that are non-null. Each value
|
|
* is a bitmap of "enum ofp_port_features" bits, in host byte order.
|
|
* Returns 0 if successful, otherwise a positive errno value. On failure,
|
|
* all of the passed-in values are set to 0. */
|
|
int
|
|
netdev_get_features(struct netdev *netdev,
|
|
uint32_t *current, uint32_t *advertised,
|
|
uint32_t *supported, uint32_t *peer)
|
|
{
|
|
...
|
|
}
|
|
|
|
Functions that destroy an instance of a dynamically-allocated type
|
|
should accept and ignore a null pointer argument. Code that calls
|
|
such a function (including the C standard library function free())
|
|
should omit a null-pointer check. We find that this usually makes
|
|
code easier to read.
|
|
|
|
|
|
FUNCTION PROTOTYPES
|
|
|
|
Put the return type and function name on the same line in a function
|
|
prototype:
|
|
|
|
static const struct option_class *get_option_class(int code);
|
|
|
|
|
|
Omit parameter names from function prototypes when the names do not
|
|
give useful information, e.g.:
|
|
|
|
int netdev_get_mtu(const struct netdev *, int *mtup);
|
|
|
|
|
|
STATEMENTS
|
|
|
|
Indent each level of code with 4 spaces. Use BSD-style brace
|
|
placement:
|
|
|
|
if (a()) {
|
|
b();
|
|
d();
|
|
}
|
|
|
|
Put a space between "if", "while", "for", etc. and the expressions
|
|
that follow them.
|
|
|
|
Enclose single statements in braces:
|
|
|
|
if (a > b) {
|
|
return a;
|
|
} else {
|
|
return b;
|
|
}
|
|
|
|
Use comments and blank lines to divide long functions into logical
|
|
groups of statements.
|
|
|
|
Avoid assignments inside "if" and "while" conditions.
|
|
|
|
Do not put gratuitous parentheses around the expression in a return
|
|
statement, that is, write "return 0;" and not "return(0);"
|
|
|
|
Write only one statement per line.
|
|
|
|
Indent "switch" statements like this:
|
|
|
|
switch (conn->state) {
|
|
case S_RECV:
|
|
error = run_connection_input(conn);
|
|
break;
|
|
|
|
case S_PROCESS:
|
|
error = 0;
|
|
break;
|
|
|
|
case S_SEND:
|
|
error = run_connection_output(conn);
|
|
break;
|
|
|
|
default:
|
|
NOT_REACHED();
|
|
}
|
|
|
|
"switch" statements with very short, uniform cases may use an
|
|
abbreviated style:
|
|
|
|
switch (code) {
|
|
case 200: return "OK";
|
|
case 201: return "Created";
|
|
case 202: return "Accepted";
|
|
case 204: return "No Content";
|
|
default: return "Unknown";
|
|
}
|
|
|
|
Use "for (;;)" to write an infinite loop.
|
|
|
|
In an if/else construct where one branch is the "normal" or "common"
|
|
case and the other branch is the "uncommon" or "error" case, put the
|
|
common case after the "if", not the "else". This is a form of
|
|
documentation. It also places the most important code in sequential
|
|
order without forcing the reader to visually skip past less important
|
|
details. (Some compilers also assume that the "if" branch is the more
|
|
common case, so this can be a real form of optimization as well.)
|
|
|
|
|
|
MACROS
|
|
|
|
Don't define an object-like macro if an enum can be used instead.
|
|
|
|
Don't define a function-like macro if a "static inline" function
|
|
can be used instead.
|
|
|
|
If a macro's definition contains multiple statements, enclose them
|
|
with "do { ... } while (0)" to allow them to work properly in all
|
|
syntactic circumstances.
|
|
|
|
Do use macros to eliminate the need to update different parts of a
|
|
single file in parallel, e.g. a list of enums and an array that gives
|
|
the name of each enum. For example:
|
|
|
|
/* Logging importance levels. */
|
|
#define VLOG_LEVELS \
|
|
VLOG_LEVEL(EMER, LOG_ALERT) \
|
|
VLOG_LEVEL(ERR, LOG_ERR) \
|
|
VLOG_LEVEL(WARN, LOG_WARNING) \
|
|
VLOG_LEVEL(INFO, LOG_NOTICE) \
|
|
VLOG_LEVEL(DBG, LOG_DEBUG)
|
|
enum vlog_level {
|
|
#define VLOG_LEVEL(NAME, SYSLOG_LEVEL) VLL_##NAME,
|
|
VLOG_LEVELS
|
|
#undef VLOG_LEVEL
|
|
VLL_N_LEVELS
|
|
};
|
|
|
|
/* Name for each logging level. */
|
|
static const char *level_names[VLL_N_LEVELS] = {
|
|
#define VLOG_LEVEL(NAME, SYSLOG_LEVEL) #NAME,
|
|
VLOG_LEVELS
|
|
#undef VLOG_LEVEL
|
|
};
|
|
|
|
|
|
SOURCE FILES
|
|
|
|
Each source file should state its license in a comment at the very
|
|
top, followed by a comment explaining the purpose of the code that is
|
|
in that file. The comment should explain how the code in the file
|
|
relates to code in other files. The goal is to allow a programmer to
|
|
quickly figure out where a given module fits into the larger system.
|
|
|
|
The first non-comment line in a .c source file should be:
|
|
|
|
#include <config.h>
|
|
|
|
#include directives should appear in the following order:
|
|
|
|
1. #include <config.h>
|
|
|
|
2. The module's own headers, if any. Including this before any
|
|
other header (besides <config.h>) ensures that the module's
|
|
header file is self-contained (see HEADER FILES) below.
|
|
|
|
3. Standard C library headers and other system headers, preferably
|
|
in alphabetical order. (Occasionally one encounters a set of
|
|
system headers that must be included in a particular order, in
|
|
which case that order must take precedence.)
|
|
|
|
4. Open vSwitch headers, in alphabetical order. Use "", not <>,
|
|
to specify Open vSwitch header names.
|
|
|
|
|
|
HEADER FILES
|
|
|
|
Each header file should start with its license, as described under
|
|
SOURCE FILES above, followed by a "header guard" to make the header
|
|
file idempotent, like so:
|
|
|
|
#ifndef NETDEV_H
|
|
#define NETDEV_H 1
|
|
|
|
...
|
|
|
|
#endif /* netdev.h */
|
|
|
|
Header files should be self-contained; that is, they should #include
|
|
whatever additional headers are required, without requiring the client
|
|
to #include them for it.
|
|
|
|
Don't define the members of a struct or union in a header file,
|
|
unless client code is actually intended to access them directly or if
|
|
the definition is otherwise actually needed (e.g. inline functions
|
|
defined in the header need them).
|
|
|
|
Similarly, don't #include a header file just for the declaration of
|
|
a struct or union tag (e.g. just for "struct <name>;"). Just declare
|
|
the tag yourself. This reduces the number of header file
|
|
dependencies.
|
|
|
|
|
|
TYPES
|
|
|
|
Use typedefs sparingly. Code is clearer if the actual type is
|
|
visible at the point of declaration. Do not, in general, declare a
|
|
typedef for a struct, union, or enum. Do not declare a typedef for a
|
|
pointer type, because this can be very confusing to the reader.
|
|
|
|
A function type is a good use for a typedef because it can clarify
|
|
code. The type should be a function type, not a pointer-to-function
|
|
type. That way, the typedef name can be used to declare function
|
|
prototypes. (It cannot be used for function definitions, because that
|
|
is explicitly prohibited by C89 and C99.)
|
|
|
|
You may assume that "char" is exactly 8 bits and that "int" and
|
|
"long" are at least 32 bits.
|
|
|
|
Don't assume that "long" is big enough to hold a pointer. If you
|
|
need to cast a pointer to an integer, use "intptr_t" or "uintptr_t"
|
|
from <stdint.h>.
|
|
|
|
Use the int<N>_t and uint<N>_t types from <stdint.h> for exact-width
|
|
integer types. Use the PRId<N>, PRIu<N>, and PRIx<N> macros from
|
|
<inttypes.h> for formatting them with printf() and related functions.
|
|
|
|
Use %zu to format size_t with printf().
|
|
|
|
Use bit-fields sparingly. Do not use bit-fields for layout of
|
|
network protocol fields or in other circumstances where the exact
|
|
format is important.
|
|
|
|
Declare bit-fields to be type "unsigned int" or "signed int". Do
|
|
*not* declare bit-fields of type "int": C89 allows these to be either
|
|
signed or unsigned according to the compiler's whim. (A 1-bit
|
|
bit-field of type "int" may have a range of -1...0!) Do not declare
|
|
bit-fields of type _Bool or enum or any other type, because these are
|
|
not portable.
|
|
|
|
Try to order structure members such that they pack well on a system
|
|
with 2-byte "short", 4-byte "int", and 4- or 8-byte "long" and pointer
|
|
types. Prefer clear organization over size optimization unless you
|
|
are convinced there is a size or speed benefit.
|
|
|
|
Pointer declarators bind to the variable name, not the type name.
|
|
Write "int *x", not "int* x" and definitely not "int * x".
|
|
|
|
|
|
EXPRESSIONS
|
|
|
|
Put one space on each side of infix binary and ternary operators:
|
|
|
|
* / %
|
|
+ -
|
|
<< >>
|
|
< <= > >=
|
|
== !=
|
|
&
|
|
^
|
|
|
|
|
&&
|
|
||
|
|
?:
|
|
= += -= *= /= %= &= ^= |= <<= >>=
|
|
|
|
Avoid comma operators.
|
|
|
|
Do not put any white space around postfix, prefix, or grouping
|
|
operators:
|
|
|
|
() [] -> .
|
|
! ~ ++ -- + - * &
|
|
|
|
Exception 1: Put a space after (but not before) the "sizeof" keyword.
|
|
Exception 2: Put a space between the () used in a cast and the
|
|
expression whose type is cast: (void *) 0.
|
|
|
|
Break long lines before the ternary operators ? and :, rather than
|
|
after them, e.g.
|
|
|
|
return (out_port != VIGP_CONTROL_PATH
|
|
? alpheus_output_port(dp, skb, out_port)
|
|
: alpheus_output_control(dp, skb, fwd_save_skb(skb),
|
|
VIGR_ACTION));
|
|
|
|
|
|
Do not parenthesize the operands of && and || unless operator
|
|
precedence makes it necessary, or unless the operands are themselves
|
|
expressions that use && and ||. Thus:
|
|
|
|
if (!isdigit((unsigned char)s[0])
|
|
|| !isdigit((unsigned char)s[1])
|
|
|| !isdigit((unsigned char)s[2])) {
|
|
printf("string %s does not start with 3-digit code\n", s);
|
|
}
|
|
|
|
but
|
|
|
|
if (rule && (!best || rule->priority > best->priority)) {
|
|
best = rule;
|
|
}
|
|
|
|
Do parenthesize a subexpression that must be split across more than
|
|
one line, e.g.:
|
|
|
|
*idxp = ((l1_idx << PORT_ARRAY_L1_SHIFT)
|
|
| (l2_idx << PORT_ARRAY_L2_SHIFT)
|
|
| (l3_idx << PORT_ARRAY_L3_SHIFT));
|
|
|
|
Try to avoid casts. Don't cast the return value of malloc().
|
|
|
|
The "sizeof" operator is unique among C operators in that it accepts
|
|
two very different kinds of operands: an expression or a type. In
|
|
general, prefer to specify an expression, e.g. "int *x =
|
|
xmalloc(sizeof *x);". When the operand of sizeof is an expression,
|
|
there is no need to parenthesize that operand, and please don't.
|
|
|
|
Use the ARRAY_SIZE macro from lib/util.h to calculate the number of
|
|
elements in an array.
|
|
|
|
When using a relational operator like "<" or "==", put an expression
|
|
or variable argument on the left and a constant argument on the
|
|
right, e.g. "x == 0", *not* "0 == x".
|
|
|
|
|
|
BLANK LINES
|
|
|
|
Put one blank line between top-level definitions of functions and
|
|
global variables.
|
|
|
|
|
|
C DIALECT
|
|
|
|
Some C99 features are OK because they are widely implemented even in
|
|
older compilers:
|
|
|
|
* Flexible array members (e.g. struct { int foo[]; }).
|
|
|
|
* "static inline" functions (but no other forms of "inline", for
|
|
which GCC and C99 have differing interpretations).
|
|
|
|
* "long long"
|
|
|
|
* <stdint.h> and <inttypes.h>.
|
|
|
|
* bool and <stdbool.h>, but don't assume that bool or _Bool can
|
|
only take on the values 0 or 1, because this behavior can't be
|
|
simulated on C89 compilers.
|
|
|
|
Don't use other C99 features that are not widely implemented in
|
|
older compilers:
|
|
|
|
* Don't use designated initializers (e.g. don't write "struct foo
|
|
foo = {.a = 1};" or "int a[] = {[2] = 5};").
|
|
|
|
* Don't mix declarations and code within a block.
|
|
|
|
* Don't use declarations in iteration statements (e.g. don't write
|
|
"for (int i = 0; i < 10; i++)").
|
|
|
|
* Don't put a trailing comma in an enum declaration (e.g. don't
|
|
write "enum { x = 1, };").
|
|
|
|
As a matter of style, avoid // comments.
|
|
|
|
Avoid using GCC extensions unless you also add a fallback for
|
|
non-GCC compilers. You can, however, use GCC extensions and C99
|
|
features in code that compiles only on GNU/Linux (such as
|
|
lib/netdev-linux.c), because GCC is the system compiler there.
|