]>
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 | ||
159 | ||
160 | FUNCTION PROTOTYPES | |
161 | ||
162 | Put the return type and function name on the same line in a function | |
163 | prototype: | |
164 | ||
165 | static const struct option_class *get_option_class(int code); | |
166 | ||
167 | ||
168 | Omit parameter names from function prototypes when the names do not | |
169 | give useful information, e.g.: | |
170 | ||
3d222126 | 171 | int netdev_get_mtu(const struct netdev *, int *mtup); |
064af421 BP |
172 | |
173 | ||
174 | STATEMENTS | |
175 | ||
176 | Indent each level of code with 4 spaces. Use BSD-style brace | |
177 | placement: | |
178 | ||
179 | if (a()) { | |
180 | b(); | |
181 | d(); | |
182 | } | |
183 | ||
184 | Put a space between "if", "while", "for", etc. and the expressions | |
185 | that follow them. | |
186 | ||
187 | Enclose single statements in braces: | |
188 | ||
189 | if (a > b) { | |
190 | return a; | |
191 | } else { | |
192 | return b; | |
193 | } | |
194 | ||
195 | Use comments and blank lines to divide long functions into logical | |
196 | groups of statements. | |
197 | ||
198 | Avoid assignments inside "if" and "while" conditions. | |
199 | ||
200 | Do not put gratuitous parentheses around the expression in a return | |
201 | statement, that is, write "return 0;" and not "return(0);" | |
202 | ||
203 | Write only one statement per line. | |
204 | ||
205 | Indent "switch" statements like this: | |
206 | ||
207 | switch (conn->state) { | |
208 | case S_RECV: | |
209 | error = run_connection_input(conn); | |
210 | break; | |
211 | ||
212 | case S_PROCESS: | |
213 | error = 0; | |
214 | break; | |
215 | ||
216 | case S_SEND: | |
217 | error = run_connection_output(conn); | |
218 | break; | |
219 | ||
220 | default: | |
221 | NOT_REACHED(); | |
222 | } | |
223 | ||
224 | "switch" statements with very short, uniform cases may use an | |
225 | abbreviated style: | |
226 | ||
227 | switch (code) { | |
228 | case 200: return "OK"; | |
229 | case 201: return "Created"; | |
230 | case 202: return "Accepted"; | |
231 | case 204: return "No Content"; | |
232 | default: return "Unknown"; | |
233 | } | |
234 | ||
235 | Use "for (;;)" to write an infinite loop. | |
236 | ||
237 | In an if/else construct where one branch is the "normal" or "common" | |
238 | case and the other branch is the "uncommon" or "error" case, put the | |
239 | common case after the "if", not the "else". This is a form of | |
240 | documentation. It also places the most important code in sequential | |
241 | order without forcing the reader to visually skip past less important | |
242 | details. (Some compilers also assume that the "if" branch is the more | |
243 | common case, so this can be a real form of optimization as well.) | |
244 | ||
245 | ||
246 | MACROS | |
247 | ||
248 | Don't define an object-like macro if an enum can be used instead. | |
249 | ||
250 | Don't define a function-like macro if a "static inline" function | |
251 | can be used instead. | |
252 | ||
253 | If a macro's definition contains multiple statements, enclose them | |
254 | with "do { ... } while (0)" to allow them to work properly in all | |
255 | syntactic circumstances. | |
256 | ||
257 | Do use macros to eliminate the need to update different parts of a | |
258 | single file in parallel, e.g. a list of enums and an array that gives | |
259 | the name of each enum. For example: | |
260 | ||
261 | /* Logging importance levels. */ | |
262 | #define VLOG_LEVELS \ | |
263 | VLOG_LEVEL(EMER, LOG_ALERT) \ | |
264 | VLOG_LEVEL(ERR, LOG_ERR) \ | |
265 | VLOG_LEVEL(WARN, LOG_WARNING) \ | |
266 | VLOG_LEVEL(INFO, LOG_NOTICE) \ | |
267 | VLOG_LEVEL(DBG, LOG_DEBUG) | |
268 | enum vlog_level { | |
269 | #define VLOG_LEVEL(NAME, SYSLOG_LEVEL) VLL_##NAME, | |
270 | VLOG_LEVELS | |
271 | #undef VLOG_LEVEL | |
272 | VLL_N_LEVELS | |
273 | }; | |
274 | ||
275 | /* Name for each logging level. */ | |
276 | static const char *level_names[VLL_N_LEVELS] = { | |
277 | #define VLOG_LEVEL(NAME, SYSLOG_LEVEL) #NAME, | |
278 | VLOG_LEVELS | |
279 | #undef VLOG_LEVEL | |
280 | }; | |
281 | ||
282 | ||
283 | SOURCE FILES | |
284 | ||
285 | Each source file should state its license in a comment at the very | |
286 | top, followed by a comment explaining the purpose of the code that is | |
287 | in that file. The comment should explain how the code in the file | |
288 | relates to code in other files. The goal is to allow a programmer to | |
289 | quickly figure out where a given module fits into the larger system. | |
290 | ||
291 | The first non-comment line in a .c source file should be: | |
292 | ||
293 | #include <config.h> | |
294 | ||
295 | #include directives should appear in the following order: | |
296 | ||
297 | 1. #include <config.h> | |
298 | ||
299 | 2. The module's own headers, if any. Including this before any | |
300 | other header (besides <config.h>) ensures that the module's | |
301 | header file is self-contained (see HEADER FILES) below. | |
302 | ||
303 | 3. Standard C library headers and other system headers, preferably | |
304 | in alphabetical order. (Occasionally one encounters a set of | |
305 | system headers that must be included in a particular order, in | |
306 | which case that order must take precedence.) | |
307 | ||
308 | 4. Open vSwitch headers, in alphabetical order. Use "", not <>, | |
309 | to specify Open vSwitch header names. | |
310 | ||
311 | ||
312 | HEADER FILES | |
313 | ||
314 | Each header file should start with its license, as described under | |
315 | SOURCE FILES above, followed by a "header guard" to make the header | |
316 | file idempotent, like so: | |
317 | ||
318 | #ifndef NETDEV_H | |
319 | #define NETDEV_H 1 | |
320 | ||
321 | ... | |
322 | ||
323 | #endif /* netdev.h */ | |
324 | ||
325 | Header files should be self-contained; that is, they should #include | |
326 | whatever additional headers are required, without requiring the client | |
327 | to #include them for it. | |
328 | ||
329 | Don't define the members of a struct or union in a header file, | |
330 | unless client code is actually intended to access them directly or if | |
331 | the definition is otherwise actually needed (e.g. inline functions | |
332 | defined in the header need them). | |
333 | ||
334 | Similarly, don't #include a header file just for the declaration of | |
335 | a struct or union tag (e.g. just for "struct <name>;"). Just declare | |
336 | the tag yourself. This reduces the number of header file | |
337 | dependencies. | |
338 | ||
339 | ||
340 | TYPES | |
341 | ||
342 | Use typedefs sparingly. Code is clearer if the actual type is | |
343 | visible at the point of declaration. Do not, in general, declare a | |
344 | typedef for a struct, union, or enum. Do not declare a typedef for a | |
345 | pointer type, because this can be very confusing to the reader. | |
346 | ||
347 | A function type is a good use for a typedef because it can clarify | |
348 | code. The type should be a function type, not a pointer-to-function | |
349 | type. That way, the typedef name can be used to declare function | |
350 | prototypes. (It cannot be used for function definitions, because that | |
351 | is explicitly prohibited by C89 and C99.) | |
352 | ||
353 | You may assume that "char" is exactly 8 bits and that "int" and | |
354 | "long" are at least 32 bits. | |
355 | ||
356 | Don't assume that "long" is big enough to hold a pointer. If you | |
357 | need to cast a pointer to an integer, use "intptr_t" or "uintptr_t" | |
358 | from <stdint.h>. | |
359 | ||
360 | Use the int<N>_t and uint<N>_t types from <stdint.h> for exact-width | |
361 | integer types. Use the PRId<N>, PRIu<N>, and PRIx<N> macros from | |
362 | <inttypes.h> for formatting them with printf() and related functions. | |
363 | ||
364 | Use %zu to format size_t with printf(). | |
365 | ||
366 | Use bit-fields sparingly. Do not use bit-fields for layout of | |
367 | network protocol fields or in other circumstances where the exact | |
368 | format is important. | |
369 | ||
370 | Declare bit-fields to be type "unsigned int" or "signed int". Do | |
371 | *not* declare bit-fields of type "int": C89 allows these to be either | |
372 | signed or unsigned according to the compiler's whim. (A 1-bit | |
373 | bit-field of type "int" may have a range of -1...0!) Do not declare | |
374 | bit-fields of type _Bool or enum or any other type, because these are | |
375 | not portable. | |
376 | ||
377 | Try to order structure members such that they pack well on a system | |
378 | with 2-byte "short", 4-byte "int", and 4- or 8-byte "long" and pointer | |
379 | types. Prefer clear organization over size optimization unless you | |
380 | are convinced there is a size or speed benefit. | |
381 | ||
382 | Pointer declarators bind to the variable name, not the type name. | |
383 | Write "int *x", not "int* x" and definitely not "int * x". | |
384 | ||
385 | ||
386 | EXPRESSIONS | |
387 | ||
388 | Put one space on each side of infix binary and ternary operators: | |
389 | ||
390 | * / % | |
391 | + - | |
392 | << >> | |
393 | < <= > >= | |
394 | == != | |
395 | & | |
396 | ^ | |
397 | | | |
398 | && | |
399 | || | |
400 | ?: | |
401 | = += -= *= /= %= &= ^= |= <<= >>= | |
402 | ||
403 | Avoid comma operators. | |
404 | ||
405 | Do not put any white space around postfix, prefix, or grouping | |
406 | operators: | |
407 | ||
408 | () [] -> . | |
409 | ! ~ ++ -- + - * & | |
410 | ||
411 | Exception 1: Put a space after (but not before) the "sizeof" keyword. | |
412 | Exception 2: Put a space between the () used in a cast and the | |
413 | expression whose type is cast: (void *) 0. | |
414 | ||
415 | Break long lines before binary operators and the ternary operators ? | |
416 | and :, rather than after them, e.g. | |
417 | ||
418 | if (first_long_condition() || second_long_condition() | |
419 | || third_long_condition()) | |
420 | ||
421 | and | |
422 | ||
423 | return (out_port != VIGP_CONTROL_PATH | |
424 | ? alpheus_output_port(dp, skb, out_port) | |
425 | : alpheus_output_control(dp, skb, fwd_save_skb(skb), | |
426 | VIGR_ACTION)); | |
427 | ||
428 | ||
429 | Do not parenthesize the operands of && and || unless operator | |
430 | precedence makes it necessary, or unless the operands are themselves | |
431 | expressions that use && and ||. Thus: | |
432 | ||
be2c418b JP |
433 | if (!isdigit((unsigned char)s[0]) |
434 | || !isdigit((unsigned char)s[1]) | |
435 | || !isdigit((unsigned char)s[2])) { | |
064af421 BP |
436 | printf("string %s does not start with 3-digit code\n", s); |
437 | } | |
438 | ||
439 | but | |
440 | ||
441 | if (rule && (!best || rule->priority > best->priority)) { | |
442 | best = rule; | |
443 | } | |
444 | ||
445 | Do parenthesize a subexpression that must be split across more than | |
446 | one line, e.g.: | |
447 | ||
448 | *idxp = ((l1_idx << PORT_ARRAY_L1_SHIFT) | |
449 | | (l2_idx << PORT_ARRAY_L2_SHIFT) | |
450 | | (l3_idx << PORT_ARRAY_L3_SHIFT)); | |
451 | ||
452 | Try to avoid casts. Don't cast the return value of malloc(). | |
453 | ||
454 | The "sizeof" operator is unique among C operators in that it accepts | |
455 | two very different kinds of operands: an expression or a type. In | |
456 | general, prefer to specify an expression, e.g. "int *x = | |
457 | xmalloc(sizeof *x);". When the operand of sizeof is an expression, | |
458 | there is no need to parenthesize that operand, and please don't. | |
459 | ||
460 | Use the ARRAY_SIZE macro from lib/util.h to calculate the number of | |
461 | elements in an array. | |
462 | ||
463 | When using a relational operator like "<" or "==", put an expression | |
464 | or variable argument on the left and a constant argument on the | |
465 | right, e.g. "x == 0", *not* "0 == x". | |
466 | ||
467 | ||
468 | BLANK LINES | |
469 | ||
470 | Put one blank line between top-level definitions of functions and | |
471 | global variables. | |
472 | ||
473 | ||
474 | C DIALECT | |
475 | ||
476 | Try to avoid using GCC extensions where possible. | |
477 | ||
478 | Some C99 extensions are OK: | |
479 | ||
480 | * Flexible array members (e.g. struct { int foo[]; }). | |
481 | ||
482 | * "static inline" functions (but no other forms of "inline", for | |
483 | which GCC and C99 have differing interpretations). | |
484 | ||
485 | * "long long" | |
486 | ||
487 | * <stdint.h> and <inttypes.h>. | |
488 | ||
489 | * bool and <stdbool.h>, but don't assume that bool or _Bool can | |
490 | only take on the values 0 or 1, because this behavior can't be | |
491 | simulated on C89 compilers. | |
492 | ||
493 | Don't use other C99 extensions, and especially: | |
494 | ||
495 | * Don't use // comments. | |
496 | ||
497 | * Don't use designated initializers (e.g. don't write "struct foo | |
498 | foo = {.a = 1};" or "int a[] = {[2] = 5};"). | |
499 | ||
500 | * Don't mix declarations and code within a block. | |
501 | ||
502 | * Don't use declarations in iteration statements (e.g. don't write | |
503 | "for (int i = 0; i < 10; i++)"). | |
504 | ||
505 | * Don't put a trailing comma in an enum declaration (e.g. don't | |
506 | write "enum { x = 1, };"). |