]>
Commit | Line | Data |
---|---|---|
064af421 BP |
1 | Open vSwitch Coding Style |
2 | ========================= | |
3 | ||
4 | This file describes the coding style used in most C files in the Open | |
5 | vSwitch distribution. However, Linux kernel code datapath directory | |
6 | follows the Linux kernel's established coding conventions. | |
7 | ||
8 | BASICS | |
9 | ||
10 | Limit lines to 79 characters. | |
11 | ||
12 | Use form feeds (control+L) to divide long source files into logical | |
13 | pieces. A form feed should appear as the only character on a line. | |
14 | ||
15 | Do not use tabs for indentation. | |
16 | ||
17 | Avoid trailing spaces on lines. | |
18 | ||
19 | ||
20 | NAMING | |
21 | ||
22 | Use names that explain the purpose of a function or object. | |
23 | ||
24 | Use underscores to separate words in an identifier: multi_word_name. | |
25 | ||
26 | Use lowercase for most names. Use uppercase for macros, macro | |
27 | parameters, and members of enumerations. | |
28 | ||
29 | Give arrays names that are plural. | |
30 | ||
31 | Pick a unique name prefix (ending with an underscore) for each | |
32 | module, and apply that prefix to all of that module's externally | |
33 | visible names. Names of macro parameters, struct and union members, | |
34 | and parameters in function prototypes are not considered externally | |
35 | visible for this purpose. | |
36 | ||
37 | Do not use names that begin with _. If you need a name for | |
38 | "internal use only", use __ as a suffix instead of a prefix. | |
39 | ||
40 | Avoid negative names: "found" is a better name than "not_found". | |
41 | ||
42 | In names, a "size" is a count of bytes, a "length" is a count of | |
43 | characters. A buffer has size, but a string has length. The length | |
44 | of a string does not include the null terminator, but the size of the | |
45 | buffer that contains the string does. | |
46 | ||
47 | ||
48 | COMMENTS | |
49 | ||
50 | Comments should be written as full sentences that start with a | |
51 | capital letter and end with a period. Put two spaces between | |
52 | sentences. | |
53 | ||
54 | Write block comments as shown below. You may put the /* and */ on | |
55 | the same line as comment text if you prefer. | |
56 | ||
57 | /* | |
58 | * We redirect stderr to /dev/null because we often want to remove all | |
59 | * traffic control configuration on a port so its in a known state. If | |
60 | * this done when there is no such configuration, tc complains, so we just | |
61 | * always ignore it. | |
62 | */ | |
63 | ||
64 | Each function and each variable declared outside a function, and | |
65 | each struct, union, and typedef declaration should be preceded by a | |
66 | comment. See FUNCTION DEFINITIONS below for function comment | |
67 | guidelines. | |
68 | ||
69 | Each struct and union member should each have an inline comment that | |
70 | explains its meaning. structs and unions with many members should be | |
71 | additionally divided into logical groups of members by block comments, | |
72 | e.g.: | |
73 | ||
74 | /* An event that will wake the following call to poll_block(). */ | |
75 | struct poll_waiter { | |
76 | /* Set when the waiter is created. */ | |
77 | struct list node; /* Element in global waiters list. */ | |
78 | int fd; /* File descriptor. */ | |
79 | short int events; /* Events to wait for (POLLIN, POLLOUT). */ | |
80 | poll_fd_func *function; /* Callback function, if any, or null. */ | |
81 | void *aux; /* Argument to callback function. */ | |
82 | struct backtrace *backtrace; /* Event that created waiter, or null. */ | |
83 | ||
84 | /* Set only when poll_block() is called. */ | |
85 | struct pollfd *pollfd; /* Pointer to element of the pollfds array | |
86 | (null if added from a callback). */ | |
87 | }; | |
88 | ||
89 | Use XXX or FIXME comments to mark code that needs work. | |
90 | ||
91 | Don't use // comments. | |
92 | ||
93 | Don't comment out or #if 0 out code. Just remove it. The code that | |
94 | was there will still be in version control history. | |
95 | ||
96 | ||
97 | FUNCTIONS | |
98 | ||
99 | Put the return type, function name, and the braces that surround the | |
100 | function's code on separate lines, all starting in column 0. | |
101 | ||
102 | Before each function definition, write a comment that describes the | |
103 | function's purpose, including each parameter, the return value, and | |
104 | side effects. References to argument names should be given in | |
105 | single-quotes, e.g. 'arg'. The comment should not include the | |
106 | function name, nor need it follow any formal structure. The comment | |
107 | does not need to describe how a function does its work, unless this | |
108 | information is needed to use the function correctly (this is often | |
109 | better done with comments *inside* the function). | |
110 | ||
111 | Simple static functions do not need a comment. | |
112 | ||
113 | Within a file, non-static functions should come first, in the order | |
114 | that they are declared in the header file, followed by static | |
115 | functions. Static functions should be in one or more separate pages | |
116 | (separated by form feed characters) in logical groups. A commonly | |
117 | useful way to divide groups is by "level", with high-level functions | |
118 | first, followed by groups of progressively lower-level functions. | |
119 | This makes it easy for the program's reader to see the top-down | |
120 | structure by reading from top to bottom. | |
121 | ||
122 | All function declarations and definitions should include a | |
123 | prototype. Empty parentheses, e.g. "int foo();", do not include a | |
124 | prototype (they state that the function's parameters are unknown); | |
125 | write "void" in parentheses instead, e.g. "int foo(void);". | |
126 | ||
127 | Prototypes for static functions should either all go at the top of | |
128 | the file, separated into groups by blank lines, or they should appear | |
129 | at the top of each page of functions. Don't comment individual | |
130 | prototypes, but a comment on each group of prototypes is often | |
131 | appropriate. | |
132 | ||
133 | In the absence of good reasons for another order, the following | |
134 | parameter order is preferred. One notable exception is that data | |
135 | parameters and their corresponding size parameters should be paired. | |
136 | ||
137 | 1. The primary object being manipulated, if any (equivalent to the | |
138 | "this" pointer in C++). | |
139 | 2. Input-only parameters. | |
140 | 3. Input/output parameters. | |
141 | 4. Output-only parameters. | |
142 | 5. Status parameter. | |
143 | ||
144 | Example: | |
145 | ||
146 | /* Stores the features supported by 'netdev' into each of '*current', | |
147 | * '*advertised', '*supported', and '*peer' that are non-null. Each value | |
148 | * is a bitmap of "enum ofp_port_features" bits, in host byte order. | |
149 | * Returns 0 if successful, otherwise a positive errno value. On failure, | |
150 | * all of the passed-in values are set to 0. */ | |
151 | int | |
152 | netdev_get_features(struct netdev *netdev, | |
153 | uint32_t *current, uint32_t *advertised, | |
154 | uint32_t *supported, uint32_t *peer) | |
155 | { | |
156 | ... | |
157 | } | |
158 | ||
b93e6983 BP |
159 | Functions that destroy an instance of a dynamically-allocated type |
160 | should accept and ignore a null pointer argument. Code that calls | |
161 | such a function (including the C standard library function free()) | |
162 | should omit a null-pointer check. We find that this usually makes | |
163 | code easier to read. | |
164 | ||
064af421 BP |
165 | |
166 | FUNCTION PROTOTYPES | |
167 | ||
168 | Put the return type and function name on the same line in a function | |
169 | prototype: | |
170 | ||
171 | static const struct option_class *get_option_class(int code); | |
172 | ||
173 | ||
174 | Omit parameter names from function prototypes when the names do not | |
175 | give useful information, e.g.: | |
176 | ||
3d222126 | 177 | int netdev_get_mtu(const struct netdev *, int *mtup); |
064af421 BP |
178 | |
179 | ||
180 | STATEMENTS | |
181 | ||
182 | Indent each level of code with 4 spaces. Use BSD-style brace | |
183 | placement: | |
184 | ||
185 | if (a()) { | |
186 | b(); | |
187 | d(); | |
188 | } | |
189 | ||
190 | Put a space between "if", "while", "for", etc. and the expressions | |
191 | that follow them. | |
192 | ||
193 | Enclose single statements in braces: | |
194 | ||
195 | if (a > b) { | |
196 | return a; | |
197 | } else { | |
198 | return b; | |
199 | } | |
200 | ||
201 | Use comments and blank lines to divide long functions into logical | |
202 | groups of statements. | |
203 | ||
204 | Avoid assignments inside "if" and "while" conditions. | |
205 | ||
206 | Do not put gratuitous parentheses around the expression in a return | |
207 | statement, that is, write "return 0;" and not "return(0);" | |
208 | ||
209 | Write only one statement per line. | |
210 | ||
211 | Indent "switch" statements like this: | |
212 | ||
213 | switch (conn->state) { | |
214 | case S_RECV: | |
215 | error = run_connection_input(conn); | |
216 | break; | |
217 | ||
218 | case S_PROCESS: | |
219 | error = 0; | |
220 | break; | |
221 | ||
222 | case S_SEND: | |
223 | error = run_connection_output(conn); | |
224 | break; | |
225 | ||
226 | default: | |
227 | NOT_REACHED(); | |
228 | } | |
229 | ||
230 | "switch" statements with very short, uniform cases may use an | |
231 | abbreviated style: | |
232 | ||
233 | switch (code) { | |
234 | case 200: return "OK"; | |
235 | case 201: return "Created"; | |
236 | case 202: return "Accepted"; | |
237 | case 204: return "No Content"; | |
238 | default: return "Unknown"; | |
239 | } | |
240 | ||
241 | Use "for (;;)" to write an infinite loop. | |
242 | ||
243 | In an if/else construct where one branch is the "normal" or "common" | |
244 | case and the other branch is the "uncommon" or "error" case, put the | |
245 | common case after the "if", not the "else". This is a form of | |
246 | documentation. It also places the most important code in sequential | |
247 | order without forcing the reader to visually skip past less important | |
248 | details. (Some compilers also assume that the "if" branch is the more | |
249 | common case, so this can be a real form of optimization as well.) | |
250 | ||
251 | ||
0d067385 BP |
252 | RETURN VALUES |
253 | ||
254 | For functions that return a success or failure indication, prefer | |
255 | one of the following return value conventions: | |
256 | ||
257 | * An "int" where 0 indicates success and a positive errno value | |
258 | indicates a reason for failure. | |
259 | ||
260 | * A "bool" where true indicates success and false indicates | |
261 | failure. | |
262 | ||
263 | ||
064af421 BP |
264 | MACROS |
265 | ||
266 | Don't define an object-like macro if an enum can be used instead. | |
267 | ||
268 | Don't define a function-like macro if a "static inline" function | |
269 | can be used instead. | |
270 | ||
271 | If a macro's definition contains multiple statements, enclose them | |
272 | with "do { ... } while (0)" to allow them to work properly in all | |
273 | syntactic circumstances. | |
274 | ||
275 | Do use macros to eliminate the need to update different parts of a | |
276 | single file in parallel, e.g. a list of enums and an array that gives | |
277 | the name of each enum. For example: | |
278 | ||
279 | /* Logging importance levels. */ | |
280 | #define VLOG_LEVELS \ | |
281 | VLOG_LEVEL(EMER, LOG_ALERT) \ | |
282 | VLOG_LEVEL(ERR, LOG_ERR) \ | |
283 | VLOG_LEVEL(WARN, LOG_WARNING) \ | |
284 | VLOG_LEVEL(INFO, LOG_NOTICE) \ | |
285 | VLOG_LEVEL(DBG, LOG_DEBUG) | |
286 | enum vlog_level { | |
287 | #define VLOG_LEVEL(NAME, SYSLOG_LEVEL) VLL_##NAME, | |
288 | VLOG_LEVELS | |
289 | #undef VLOG_LEVEL | |
290 | VLL_N_LEVELS | |
291 | }; | |
292 | ||
293 | /* Name for each logging level. */ | |
294 | static const char *level_names[VLL_N_LEVELS] = { | |
295 | #define VLOG_LEVEL(NAME, SYSLOG_LEVEL) #NAME, | |
296 | VLOG_LEVELS | |
297 | #undef VLOG_LEVEL | |
298 | }; | |
299 | ||
300 | ||
301 | SOURCE FILES | |
302 | ||
303 | Each source file should state its license in a comment at the very | |
304 | top, followed by a comment explaining the purpose of the code that is | |
305 | in that file. The comment should explain how the code in the file | |
306 | relates to code in other files. The goal is to allow a programmer to | |
307 | quickly figure out where a given module fits into the larger system. | |
308 | ||
309 | The first non-comment line in a .c source file should be: | |
310 | ||
311 | #include <config.h> | |
312 | ||
313 | #include directives should appear in the following order: | |
314 | ||
315 | 1. #include <config.h> | |
316 | ||
317 | 2. The module's own headers, if any. Including this before any | |
318 | other header (besides <config.h>) ensures that the module's | |
319 | header file is self-contained (see HEADER FILES) below. | |
320 | ||
321 | 3. Standard C library headers and other system headers, preferably | |
322 | in alphabetical order. (Occasionally one encounters a set of | |
323 | system headers that must be included in a particular order, in | |
324 | which case that order must take precedence.) | |
325 | ||
326 | 4. Open vSwitch headers, in alphabetical order. Use "", not <>, | |
327 | to specify Open vSwitch header names. | |
328 | ||
329 | ||
330 | HEADER FILES | |
331 | ||
332 | Each header file should start with its license, as described under | |
333 | SOURCE FILES above, followed by a "header guard" to make the header | |
334 | file idempotent, like so: | |
335 | ||
336 | #ifndef NETDEV_H | |
337 | #define NETDEV_H 1 | |
338 | ||
339 | ... | |
340 | ||
341 | #endif /* netdev.h */ | |
342 | ||
343 | Header files should be self-contained; that is, they should #include | |
344 | whatever additional headers are required, without requiring the client | |
345 | to #include them for it. | |
346 | ||
347 | Don't define the members of a struct or union in a header file, | |
348 | unless client code is actually intended to access them directly or if | |
349 | the definition is otherwise actually needed (e.g. inline functions | |
350 | defined in the header need them). | |
351 | ||
352 | Similarly, don't #include a header file just for the declaration of | |
353 | a struct or union tag (e.g. just for "struct <name>;"). Just declare | |
354 | the tag yourself. This reduces the number of header file | |
355 | dependencies. | |
356 | ||
357 | ||
358 | TYPES | |
359 | ||
360 | Use typedefs sparingly. Code is clearer if the actual type is | |
361 | visible at the point of declaration. Do not, in general, declare a | |
362 | typedef for a struct, union, or enum. Do not declare a typedef for a | |
363 | pointer type, because this can be very confusing to the reader. | |
364 | ||
365 | A function type is a good use for a typedef because it can clarify | |
366 | code. The type should be a function type, not a pointer-to-function | |
367 | type. That way, the typedef name can be used to declare function | |
368 | prototypes. (It cannot be used for function definitions, because that | |
369 | is explicitly prohibited by C89 and C99.) | |
370 | ||
371 | You may assume that "char" is exactly 8 bits and that "int" and | |
372 | "long" are at least 32 bits. | |
373 | ||
374 | Don't assume that "long" is big enough to hold a pointer. If you | |
375 | need to cast a pointer to an integer, use "intptr_t" or "uintptr_t" | |
376 | from <stdint.h>. | |
377 | ||
378 | Use the int<N>_t and uint<N>_t types from <stdint.h> for exact-width | |
379 | integer types. Use the PRId<N>, PRIu<N>, and PRIx<N> macros from | |
380 | <inttypes.h> for formatting them with printf() and related functions. | |
381 | ||
382 | Use %zu to format size_t with printf(). | |
383 | ||
384 | Use bit-fields sparingly. Do not use bit-fields for layout of | |
385 | network protocol fields or in other circumstances where the exact | |
386 | format is important. | |
387 | ||
388 | Declare bit-fields to be type "unsigned int" or "signed int". Do | |
389 | *not* declare bit-fields of type "int": C89 allows these to be either | |
390 | signed or unsigned according to the compiler's whim. (A 1-bit | |
391 | bit-field of type "int" may have a range of -1...0!) Do not declare | |
392 | bit-fields of type _Bool or enum or any other type, because these are | |
393 | not portable. | |
394 | ||
395 | Try to order structure members such that they pack well on a system | |
396 | with 2-byte "short", 4-byte "int", and 4- or 8-byte "long" and pointer | |
397 | types. Prefer clear organization over size optimization unless you | |
398 | are convinced there is a size or speed benefit. | |
399 | ||
400 | Pointer declarators bind to the variable name, not the type name. | |
401 | Write "int *x", not "int* x" and definitely not "int * x". | |
402 | ||
403 | ||
404 | EXPRESSIONS | |
405 | ||
406 | Put one space on each side of infix binary and ternary operators: | |
407 | ||
408 | * / % | |
409 | + - | |
410 | << >> | |
411 | < <= > >= | |
412 | == != | |
413 | & | |
414 | ^ | |
415 | | | |
416 | && | |
417 | || | |
418 | ?: | |
419 | = += -= *= /= %= &= ^= |= <<= >>= | |
420 | ||
421 | Avoid comma operators. | |
422 | ||
423 | Do not put any white space around postfix, prefix, or grouping | |
424 | operators: | |
425 | ||
426 | () [] -> . | |
427 | ! ~ ++ -- + - * & | |
428 | ||
429 | Exception 1: Put a space after (but not before) the "sizeof" keyword. | |
430 | Exception 2: Put a space between the () used in a cast and the | |
431 | expression whose type is cast: (void *) 0. | |
432 | ||
ede81843 BP |
433 | Break long lines before the ternary operators ? and :, rather than |
434 | after them, e.g. | |
064af421 BP |
435 | |
436 | return (out_port != VIGP_CONTROL_PATH | |
437 | ? alpheus_output_port(dp, skb, out_port) | |
438 | : alpheus_output_control(dp, skb, fwd_save_skb(skb), | |
439 | VIGR_ACTION)); | |
440 | ||
441 | ||
442 | Do not parenthesize the operands of && and || unless operator | |
443 | precedence makes it necessary, or unless the operands are themselves | |
444 | expressions that use && and ||. Thus: | |
445 | ||
be2c418b | 446 | if (!isdigit((unsigned char)s[0]) |
568e23fc BP |
447 | || !isdigit((unsigned char)s[1]) |
448 | || !isdigit((unsigned char)s[2])) { | |
064af421 BP |
449 | printf("string %s does not start with 3-digit code\n", s); |
450 | } | |
451 | ||
452 | but | |
453 | ||
454 | if (rule && (!best || rule->priority > best->priority)) { | |
455 | best = rule; | |
456 | } | |
457 | ||
458 | Do parenthesize a subexpression that must be split across more than | |
459 | one line, e.g.: | |
460 | ||
461 | *idxp = ((l1_idx << PORT_ARRAY_L1_SHIFT) | |
462 | | (l2_idx << PORT_ARRAY_L2_SHIFT) | |
463 | | (l3_idx << PORT_ARRAY_L3_SHIFT)); | |
464 | ||
465 | Try to avoid casts. Don't cast the return value of malloc(). | |
466 | ||
467 | The "sizeof" operator is unique among C operators in that it accepts | |
468 | two very different kinds of operands: an expression or a type. In | |
469 | general, prefer to specify an expression, e.g. "int *x = | |
470 | xmalloc(sizeof *x);". When the operand of sizeof is an expression, | |
471 | there is no need to parenthesize that operand, and please don't. | |
472 | ||
473 | Use the ARRAY_SIZE macro from lib/util.h to calculate the number of | |
474 | elements in an array. | |
475 | ||
476 | When using a relational operator like "<" or "==", put an expression | |
477 | or variable argument on the left and a constant argument on the | |
478 | right, e.g. "x == 0", *not* "0 == x". | |
479 | ||
480 | ||
481 | BLANK LINES | |
482 | ||
483 | Put one blank line between top-level definitions of functions and | |
484 | global variables. | |
485 | ||
486 | ||
487 | C DIALECT | |
488 | ||
c214278b BP |
489 | Some C99 features are OK because they are widely implemented even in |
490 | older compilers: | |
064af421 BP |
491 | |
492 | * Flexible array members (e.g. struct { int foo[]; }). | |
493 | ||
494 | * "static inline" functions (but no other forms of "inline", for | |
495 | which GCC and C99 have differing interpretations). | |
496 | ||
497 | * "long long" | |
498 | ||
499 | * <stdint.h> and <inttypes.h>. | |
500 | ||
501 | * bool and <stdbool.h>, but don't assume that bool or _Bool can | |
502 | only take on the values 0 or 1, because this behavior can't be | |
503 | simulated on C89 compilers. | |
504 | ||
c214278b BP |
505 | Don't use other C99 features that are not widely implemented in |
506 | older compilers: | |
064af421 BP |
507 | |
508 | * Don't use designated initializers (e.g. don't write "struct foo | |
509 | foo = {.a = 1};" or "int a[] = {[2] = 5};"). | |
510 | ||
511 | * Don't mix declarations and code within a block. | |
512 | ||
513 | * Don't use declarations in iteration statements (e.g. don't write | |
514 | "for (int i = 0; i < 10; i++)"). | |
515 | ||
516 | * Don't put a trailing comma in an enum declaration (e.g. don't | |
517 | write "enum { x = 1, };"). | |
c214278b BP |
518 | |
519 | As a matter of style, avoid // comments. | |
520 | ||
521 | Avoid using GCC extensions unless you also add a fallback for | |
522 | non-GCC compilers. You can, however, use GCC extensions and C99 | |
523 | features in code that compiles only on GNU/Linux (such as | |
524 | lib/netdev-linux.c), because GCC is the system compiler there. |