]>
Commit | Line | Data |
---|---|---|
183d7fa5 TH |
1 | LXC Coding Style Guide |
2 | ====================== | |
3 | ||
a8e63d69 CB |
4 | In general the LXC project follows the Linux kernel coding style. However, |
5 | there are a few differences. They are outlined in this document. | |
183d7fa5 TH |
6 | |
7 | The Linux kernel coding style guide can be found within the kernel tree: | |
8 | ||
9 | Documentation/process/coding-style.rst | |
10 | ||
11 | It can be accessed online too: | |
12 | ||
13 | https://www.kernel.org/doc/html/latest/process/coding-style.html | |
14 | ||
7419c83f | 15 | ## 1) General Notes |
4a308d66 CB |
16 | |
17 | - The coding style guide refers to new code. But legacy code can be cleaned up | |
18 | and we are happy to take those patches. | |
19 | - Just because there is still code in LXC that doesn't adhere to the coding | |
20 | standards outlined here does not license not adhering to the coding style. In | |
21 | other words: please stick to the coding style. | |
22 | - Maintainers are free to ignore rules specified here when merging pull | |
23 | requests. This guideline might seem a little weird but it exits to ease new | |
24 | developers into the code base and to prevent unnecessary bikeshedding. If | |
25 | a maintainer feels hat enforcing a specific rule in a given commit would do | |
26 | more harm than good they should always feel free to ignore the rule. | |
27 | ||
28 | Furthermore, when merging pull requests that do not adhere to our coding | |
29 | style maintainers should feel free to grab the commit, adapt it to our coding | |
30 | style and add their Signed-off-by line to it. This is especially helpful to | |
31 | make it easier for first-time contributors and to prevent having pull | |
32 | requests being stuck in the merge queue because of minor details. | |
c67cb619 CB |
33 | - We currently do not provide automatic coding style checks but if a suitable |
34 | tool is found we are happy to integrate it into our test suite. It is | |
35 | possible and recommended to use the `clang-format` binary to check your code. | |
36 | The following options are an approximation of the coding style used here. | |
37 | Simply create a file called `.clang-format` in your home directory with the | |
38 | following options: | |
39 | ```sh | |
40 | cat << EOF > "${HOME}"/.clang-format | |
b629739c | 41 | AlignEscapedNewlines: Left |
c67cb619 CB |
42 | BreakBeforeBraces: Attach |
43 | AlwaysBreakBeforeMultilineStrings: false | |
44 | BreakBeforeBinaryOperators: None | |
45 | MaxEmptyLinesToKeep: 1 | |
46 | PenaltyBreakBeforeFirstCallParameter: 1000000 | |
47 | BinPackArguments: true | |
48 | BinPackParameters: true | |
49 | AllowAllParametersOfDeclarationOnNextLine: false | |
50 | AlignAfterOpenBracket: true | |
51 | SpacesInSquareBrackets: false | |
52 | SpacesInCStyleCastParentheses: false | |
53 | SpaceInEmptyParentheses: false | |
54 | SpaceBeforeParens: ControlStatements | |
55 | SpaceAfterCStyleCast: false | |
56 | SortIncludes: true | |
57 | PenaltyReturnTypeOnItsOwnLine: 10000 | |
58 | PenaltyExcessCharacter: 10 | |
59 | Language: Cpp | |
60 | ForEachMacros: ['lxc_list_for_each', 'lxc_list_for_each_safe'] | |
61 | AllowShortLoopsOnASingleLine: false | |
62 | AllowShortIfStatementsOnASingleLine: false | |
63 | AllowShortFunctionsOnASingleLine: None | |
64 | AllowShortCaseLabelsOnASingleLine: false | |
65 | AllowShortBlocksOnASingleLine: false | |
66 | BasedOnStyle: LLVM | |
67 | TabWidth: 8 | |
68 | IndentWidth: 8 | |
69 | UseTab: Always | |
70 | BreakBeforeBraces: Linux | |
71 | AllowShortIfStatementsOnASingleLine: false | |
72 | IndentCaseLabels: false | |
73 | EOF | |
74 | ``` | |
75 | However, it will not handle all cases correctly. For example, most `struct` | |
76 | initializations will not be correct. In such cases please refer to the coding | |
77 | style here. | |
4a308d66 | 78 | |
7419c83f | 79 | ## 2) Only Use Tabs |
4a308d66 CB |
80 | |
81 | - LXC uses tabs. | |
82 | ||
7419c83f | 83 | ## 3) Only use `/* */` Style Comments |
4a308d66 CB |
84 | |
85 | - Any comments that are added must use `/* */`. | |
a8e63d69 | 86 | - Single-line comments should start on the same line as the opening `/*`. |
4a308d66 | 87 | - Single-line comments should simply be placed between `/* */`. For example: |
b0c407f7 | 88 | ```C |
4a308d66 CB |
89 | /* Define pivot_root() if missing from the C library */ |
90 | ``` | |
a8e63d69 CB |
91 | - Mutli-line comment should start on the next line following the opening |
92 | `/*`and should end with the closing `*/` on a separate line. For | |
4a308d66 | 93 | example: |
b0c407f7 | 94 | ```C |
a8e63d69 CB |
95 | /* |
96 | * At this point the old-root is mounted on top of our new-root | |
4a308d66 CB |
97 | * To unmounted it we must not be chdir()ed into it, so escape back |
98 | * to old-root. | |
99 | */ | |
100 | ``` | |
101 | ||
7419c83f | 102 | ## 4) Try To Wrap At 80chars |
4a308d66 CB |
103 | |
104 | - This is not strictly enforced. It is perfectly valid to sometimes | |
105 | overflow this limit if it helps clarity. Nonetheless, try to stick to it | |
106 | and use common sense to decide when not to. | |
107 | ||
7419c83f | 108 | ## 5) Error Messages |
4a308d66 CB |
109 | |
110 | - Error messages must start with a capital letter and must **not** end with a | |
111 | punctuation sign. | |
112 | - They should be descriptive, without being needlessly long. It is best to just | |
113 | use already existing error messages as examples. | |
a8e63d69 CB |
114 | - The commit message itself is not subject to rule 4), i.e. it should not be |
115 | wrapped at 80chars. This is to make it easy to grep for it. | |
4a308d66 | 116 | - Examples of acceptable error messages are: |
b0c407f7 | 117 | ```C |
4a308d66 CB |
118 | SYSERROR("Failed to create directory \"%s\"", path); |
119 | WARN("\"/dev\" directory does not exist. Proceeding without autodev being set up"); | |
120 | ``` | |
121 | ||
a8e63d69 | 122 | ## 6) Set `errno` |
4a308d66 | 123 | |
a8e63d69 CB |
124 | - Functions that can fail in a non-binary way should return `-1` and set |
125 | `errno` to a meaningful error code. | |
126 | As a convenience LXC provides the `minus_one_set_errno` macro: | |
127 | ```C | |
128 | static int set_config_net_l2proxy(const char *key, const char *value, | |
129 | struct lxc_conf *lxc_conf, void *data) | |
130 | { | |
131 | struct lxc_netdev *netdev = data; | |
132 | unsigned int val = 0; | |
133 | int ret; | |
134 | ||
135 | if (lxc_config_value_empty(value)) | |
136 | return clr_config_net_l2proxy(key, lxc_conf, data); | |
137 | ||
138 | if (!netdev) | |
139 | return minus_one_set_errno(EINVAL); | |
140 | ||
141 | ret = lxc_safe_uint(value, &val); | |
142 | if (ret < 0) | |
143 | return minus_one_set_errno(-ret); | |
144 | ||
145 | switch (val) { | |
146 | case 0: | |
147 | netdev->l2proxy = false; | |
148 | return 0; | |
149 | case 1: | |
150 | netdev->l2proxy = true; | |
151 | return 0; | |
152 | } | |
153 | ||
154 | return minus_one_set_errno(EINVAL); | |
155 | } | |
156 | ``` | |
4a308d66 | 157 | |
7419c83f | 158 | ## 7) All Unexported Functions Must Be Declared `static` |
4a308d66 CB |
159 | |
160 | - Functions which are only used in the current file and are not exported | |
161 | within the codebase need to be declared with the `static` attribute. | |
162 | ||
7419c83f | 163 | ## 8) All Exported Functions Must Be Declared `extern` In A Header File |
4a308d66 | 164 | |
39a1e1fa TH |
165 | - Functions declared in header files (`*.h`) should use the `extern` keyword. |
166 | - Functions declared in source files (`*.c`) should not use the `extern` keyword. | |
4a308d66 | 167 | |
7419c83f | 168 | ## 9) Declaring Variables |
4a308d66 CB |
169 | |
170 | - variables should be declared at the top of the function or at the beginning | |
a8e63d69 CB |
171 | of a new scope but **never** in the middle of a scope. They should be ordered |
172 | in the following way: | |
173 | 1. automatically freed variables | |
174 | - This specifically references variables cleaned up via the `cleanup` | |
175 | attribute as supported by `gcc` and `clang`. | |
4a308d66 | 176 | 2. initialized variables |
a8e63d69 CB |
177 | 3. uninitialized variables |
178 | General rules are: | |
179 | - put base types before complex types | |
180 | - put standard types defined by libc before types defined by LXC | |
181 | - put multiple declarations of the same type on the same line | |
4a308d66 | 182 | - Examples of good declarations can be seen in the following function: |
b0c407f7 | 183 | ```C |
4a308d66 CB |
184 | int lxc_clear_procs(struct lxc_conf *c, const char *key) |
185 | { | |
186 | struct lxc_list *it, *next; | |
187 | bool all = false; | |
188 | const char *k = NULL; | |
189 | ||
190 | if (strcmp(key, "lxc.proc") == 0) | |
191 | all = true; | |
192 | else if (strncmp(key, "lxc.proc.", sizeof("lxc.proc.") - 1) == 0) | |
193 | k = key + sizeof("lxc.proc.") - 1; | |
194 | else | |
195 | return -1; | |
196 | ||
197 | lxc_list_for_each_safe(it, &c->procs, next) { | |
198 | struct lxc_proc *proc = it->elem; | |
199 | ||
200 | if (!all && strcmp(proc->filename, k) != 0) | |
201 | continue; | |
202 | lxc_list_del(it); | |
203 | free(proc->filename); | |
204 | free(proc->value); | |
205 | free(proc); | |
206 | free(it); | |
207 | } | |
208 | ||
209 | return 0; | |
210 | } | |
211 | ``` | |
212 | ||
7419c83f | 213 | ## 10) Functions Not Returning Booleans Must Assign Return Value Before Performing Checks |
4a308d66 CB |
214 | |
215 | - When checking whether a function not returning booleans was successful or not | |
216 | the returned value must be assigned before it is checked (`str{n}cmp()` | |
217 | functions being one notable exception). For example: | |
b0c407f7 | 218 | ```C |
4a308d66 CB |
219 | /* assign value to "ret" first */ |
220 | ret = mount(sourcepath, cgpath, "cgroup", remount_flags, NULL); | |
221 | /* check whether function was successful */ | |
222 | if (ret < 0) { | |
223 | SYSERROR("Failed to remount \"%s\" ro", cgpath); | |
224 | free(sourcepath); | |
225 | return -1; | |
226 | } | |
227 | ``` | |
228 | Functions returning booleans can be checked directly. For example: | |
b0c407f7 | 229 | ```C |
4a308d66 CB |
230 | extern bool lxc_string_in_array(const char *needle, const char **haystack); |
231 | ||
232 | /* check right away */ | |
233 | if (lxc_string_in_array("ns", (const char **)h->subsystems)) | |
234 | continue; | |
235 | ``` | |
236 | ||
7419c83f | 237 | ## 11) Non-Boolean Functions That Behave Like Boolean Functions Must Explicitly Check Against A Value |
ed06b69c CB |
238 | |
239 | - This rule mainly exists for `str{n}cmp()` type functions. In most cases they | |
240 | are used like a boolean function to check whether a string matches or not. | |
241 | But they return an integer. It is perfectly fine to check `str{n}cmp()` | |
242 | functions directly but you must compare explicitly against a value. That is | |
243 | to say, while they are conceptually boolean functions they shouldn't be | |
244 | treated as such since they don't really behave like boolean functions. So | |
245 | `if (!str{n}cmp())` and `if (str{n}cmp())` checks must not be used. Good | |
246 | examples are found in the following functions: | |
b0c407f7 | 247 | ```C |
ed06b69c CB |
248 | static int set_config_hooks(const char *key, const char *value, |
249 | struct lxc_conf *lxc_conf, void *data) | |
250 | ||
251 | char *copy; | |
252 | ||
253 | if (lxc_config_value_empty(value)) | |
254 | return lxc_clear_hooks(lxc_conf, key); | |
255 | ||
256 | if (strcmp(key + 4, "hook") == 0) { | |
257 | ERROR("lxc.hook must not have a value"); | |
258 | return -1; | |
259 | } | |
260 | ||
261 | copy = strdup(value); | |
262 | if (!copy) | |
263 | return -1; | |
264 | ||
265 | if (strcmp(key + 9, "pre-start") == 0) | |
266 | return add_hook(lxc_conf, LXCHOOK_PRESTART, copy); | |
267 | else if (strcmp(key + 9, "start-host") == 0) | |
268 | return add_hook(lxc_conf, LXCHOOK_START_HOST, copy); | |
269 | else if (strcmp(key + 9, "pre-mount") == 0) | |
270 | return add_hook(lxc_conf, LXCHOOK_PREMOUNT, copy); | |
271 | else if (strcmp(key + 9, "autodev") == 0) | |
272 | return add_hook(lxc_conf, LXCHOOK_AUTODEV, copy); | |
273 | else if (strcmp(key + 9, "mount") == 0) | |
274 | return add_hook(lxc_conf, LXCHOOK_MOUNT, copy); | |
275 | else if (strcmp(key + 9, "start") == 0) | |
276 | return add_hook(lxc_conf, LXCHOOK_START, copy); | |
277 | else if (strcmp(key + 9, "stop") == 0) | |
278 | return add_hook(lxc_conf, LXCHOOK_STOP, copy); | |
279 | else if (strcmp(key + 9, "post-stop") == 0) | |
280 | return add_hook(lxc_conf, LXCHOOK_POSTSTOP, copy); | |
281 | else if (strcmp(key + 9, "clone") == 0) | |
282 | return add_hook(lxc_conf, LXCHOOK_CLONE, copy); | |
283 | else if (strcmp(key + 9, "destroy") == 0) | |
284 | return add_hook(lxc_conf, LXCHOOK_DESTROY, copy); | |
285 | ||
286 | free(copy); | |
287 | return -1; | |
288 | } | |
289 | ``` | |
290 | ||
7419c83f | 291 | ## 12) Do Not Use C99 Variable Length Arrays (VLA) |
4a308d66 CB |
292 | |
293 | - They are made optional and there is no guarantee that future C standards | |
294 | will support them. | |
295 | ||
7419c83f | 296 | ## 13) Use Standard libc Macros When Exiting |
4a308d66 CB |
297 | |
298 | - libc provides `EXIT_FAILURE` and `EXIT_SUCCESS`. Use them whenever possible | |
299 | in the child of `fork()`ed process or when exiting from a `main()` function. | |
300 | ||
7419c83f | 301 | ## 14) Use `goto`s |
4a308d66 CB |
302 | |
303 | `goto`s are an essential language construct of C and are perfect to perform | |
304 | cleanup operations or simplify the logic of functions. However, here are the | |
305 | rules to use them: | |
306 | - use descriptive `goto` labels. | |
307 | For example, if you know that this label is only used as an error path you | |
308 | should use something like `on_error` instead of `out` as label name. | |
309 | - **only** jump downwards unless you are handling `EAGAIN` errors and want to | |
310 | avoid `do-while` constructs. | |
311 | - An example of a good usage of `goto` is: | |
b0c407f7 | 312 | ```C |
4a308d66 CB |
313 | static int set_config_idmaps(const char *key, const char *value, |
314 | struct lxc_conf *lxc_conf, void *data) | |
315 | { | |
316 | unsigned long hostid, nsid, range; | |
317 | char type; | |
318 | int ret; | |
319 | struct lxc_list *idmaplist = NULL; | |
320 | struct id_map *idmap = NULL; | |
321 | ||
322 | if (lxc_config_value_empty(value)) | |
323 | return lxc_clear_idmaps(lxc_conf); | |
324 | ||
325 | idmaplist = malloc(sizeof(*idmaplist)); | |
326 | if (!idmaplist) | |
327 | goto on_error; | |
328 | ||
329 | idmap = malloc(sizeof(*idmap)); | |
330 | if (!idmap) | |
331 | goto on_error; | |
332 | memset(idmap, 0, sizeof(*idmap)); | |
333 | ||
334 | ret = parse_idmaps(value, &type, &nsid, &hostid, &range); | |
335 | if (ret < 0) { | |
336 | ERROR("Failed to parse id mappings"); | |
337 | goto on_error; | |
338 | } | |
339 | ||
340 | INFO("Read uid map: type %c nsid %lu hostid %lu range %lu", type, nsid, hostid, range); | |
341 | if (type == 'u') | |
342 | idmap->idtype = ID_TYPE_UID; | |
343 | else if (type == 'g') | |
344 | idmap->idtype = ID_TYPE_GID; | |
345 | else | |
346 | goto on_error; | |
347 | ||
348 | idmap->hostid = hostid; | |
349 | idmap->nsid = nsid; | |
350 | idmap->range = range; | |
351 | idmaplist->elem = idmap; | |
352 | lxc_list_add_tail(&lxc_conf->id_map, idmaplist); | |
353 | ||
354 | if (!lxc_conf->root_nsuid_map && idmap->idtype == ID_TYPE_UID) | |
355 | if (idmap->nsid == 0) | |
356 | lxc_conf->root_nsuid_map = idmap; | |
357 | ||
358 | ||
359 | if (!lxc_conf->root_nsgid_map && idmap->idtype == ID_TYPE_GID) | |
360 | if (idmap->nsid == 0) | |
361 | lxc_conf->root_nsgid_map = idmap; | |
362 | ||
363 | idmap = NULL; | |
364 | ||
365 | return 0; | |
366 | ||
367 | on_error: | |
368 | free(idmaplist); | |
369 | free(idmap); | |
370 | ||
371 | return -1; | |
372 | } | |
373 | ``` | |
374 | ||
7419c83f | 375 | ## 15) Use Booleans instead of integers |
4a308d66 CB |
376 | |
377 | - When something can be conceptualized in a binary way use a boolean not | |
378 | an integer. | |
379 | ||
7419c83f | 380 | ## 16) Cleanup Functions Must Handle The Object's Null Type And Being Passed Already Cleaned Up Objects |
4a308d66 CB |
381 | |
382 | - If you implement a custom cleanup function to e.g. free a complex type | |
383 | you declared you must ensure that the object's null type is handled and | |
384 | treated as a NOOP. For example: | |
b0c407f7 | 385 | ```C |
4a308d66 CB |
386 | void lxc_free_array(void **array, lxc_free_fn element_free_fn) |
387 | { | |
388 | void **p; | |
389 | for (p = array; p && *p; p++) | |
390 | element_free_fn(*p); | |
391 | free((void*)array); | |
392 | } | |
393 | ``` | |
394 | - Cleanup functions should also expect to be passed already cleaned up objects. | |
395 | One way to handle this cleanly is to initialize the cleaned up variable to | |
396 | a special value that signals the function that the element has already been | |
397 | freed on the next call. For example, the following function cleans up file | |
398 | descriptors and sets the already closed file descriptors to `-EBADF`. On the | |
399 | next call it can simply check whether the file descriptor is positive and | |
400 | move on if it isn't: | |
b0c407f7 | 401 | ```C |
4a308d66 CB |
402 | static void lxc_put_attach_clone_payload(struct attach_clone_payload *p) |
403 | { | |
404 | if (p->ipc_socket >= 0) { | |
405 | shutdown(p->ipc_socket, SHUT_RDWR); | |
406 | close(p->ipc_socket); | |
407 | p->ipc_socket = -EBADF; | |
408 | } | |
409 | ||
410 | if (p->pty_fd >= 0) { | |
411 | close(p->pty_fd); | |
412 | p->pty_fd = -EBADF; | |
413 | } | |
414 | ||
415 | if (p->init_ctx) { | |
416 | lxc_proc_put_context_info(p->init_ctx); | |
417 | p->init_ctx = NULL; | |
418 | } | |
419 | } | |
420 | ``` | |
421 | ||
7419c83f | 422 | ## 17) Cast to `(void)` When Intentionally Ignoring Return Values |
4a308d66 CB |
423 | |
424 | - There are cases where you do not care about the return value of a function. | |
425 | Please cast the return value to `(void)` when doing so. | |
426 | - Standard library functions or functions which are known to be ignored by | |
427 | default do not need to be cast to `(void)`. Classical candidates are | |
428 | `close()` and `fclose()`. | |
429 | - A good example is: | |
b0c407f7 | 430 | ```C |
4a308d66 CB |
431 | for (i = 0; hierarchies[i]; i++) { |
432 | char *fullpath; | |
433 | char *path = hierarchies[i]->fullcgpath; | |
434 | ||
435 | ret = chowmod(path, destuid, nsgid, 0755); | |
436 | if (ret < 0) | |
437 | return -1; | |
438 | ||
439 | /* failures to chown() these are inconvenient but not | |
440 | * detrimental we leave these owned by the container launcher, | |
441 | * so that container root can write to the files to attach. we | |
442 | * chmod() them 664 so that container systemd can write to the | |
443 | * files (which systemd in wily insists on doing). | |
444 | */ | |
445 | ||
446 | if (hierarchies[i]->version == cgroup_super_magic) { | |
447 | fullpath = must_make_path(path, "tasks", null); | |
448 | (void)chowmod(fullpath, destuid, nsgid, 0664); | |
449 | free(fullpath); | |
450 | } | |
451 | ||
452 | fullpath = must_make_path(path, "cgroup.procs", null); | |
453 | (void)chowmod(fullpath, destuid, 0, 0664); | |
454 | free(fullpath); | |
455 | ||
456 | if (hierarchies[i]->version != cgroup2_super_magic) | |
457 | continue; | |
458 | ||
459 | fullpath = must_make_path(path, "cgroup.subtree_control", null); | |
460 | (void)chowmod(fullpath, destuid, nsgid, 0664); | |
461 | free(fullpath); | |
462 | ||
463 | fullpath = must_make_path(path, "cgroup.threads", null); | |
464 | (void)chowmod(fullpath, destuid, nsgid, 0664); | |
465 | free(fullpath); | |
466 | } | |
467 | ``` | |
468 | ||
7419c83f | 469 | ## 18) Use `for (;;)` instead of `while (1)` or `while (true)` |
4a308d66 CB |
470 | |
471 | - Let's be honest, it is really the only sensible way to do this. | |
472 | ||
7419c83f | 473 | ## 19) Use The Set Of Supported DCO Statements |
4a308d66 CB |
474 | |
475 | - Signed-off-by: Random J Developer <random@developer.org> | |
476 | - You did write this code or have the right to contribute it to LXC. | |
477 | - Acked-by: Random J Developer <random@developer.org> | |
478 | - You did read the code and think it is correct. This is usually only used by | |
479 | maintainers or developers that have made significant contributions and can | |
480 | vouch for the correctness of someone else's code. | |
481 | - Reviewed-by: Random J Developer <random@developer.org> | |
482 | - You did review the code and vouch for its correctness, i.e. you'd be | |
483 | prepared to fix bugs it might cause. This is usually only used by | |
484 | maintainers or developers that have made significant contributions and can | |
485 | vouch for the correctness of someone else's code. | |
486 | - Co-developed-by: Random J Developer <random@developer.org> | |
487 | - The code can not be reasonably attributed to a single developer, i.e. | |
488 | you worked on this together. | |
489 | - Tested-by: Random J Developer <random@developer.org> | |
490 | - You verified that the code fixes a given bug or is behaving as advertised. | |
491 | - Reported-by: Random J Developer <random@developer.org> | |
492 | - You found and reported the bug. | |
493 | - Suggested-by: Random J Developer <random@developer.org> | |
494 | - You wrote the code but someone contributed the idea. This line is usually | |
495 | overlooked but it is a sign of good etiquette and coding ethics: if someone | |
496 | helped you solve a problem or had a clever idea do not silently claim it by | |
497 | slapping your Signed-off-by underneath. Be honest and add a Suggested-by. | |
498 | ||
7419c83f | 499 | ## 20) Commit Message Outline |
4a308d66 CB |
500 | |
501 | - You **must** stick to the 80chars limit especially in the title of the commit | |
502 | message. | |
503 | - Please use English commit messages only. | |
504 | - use meaningful commit messages. | |
505 | - Use correct spelling and grammar. | |
506 | If you are not a native speaker and/or feel yourself struggling with this it | |
507 | is perfectly fine to point this out and there's no need to apologize. Usually | |
508 | developers will be happy to pull your branch and adopt the commit message. | |
509 | - Please always use the affected file (without the file type suffix) or module | |
510 | as a prefix in the commit message. | |
511 | - Examples of good commit messages are: | |
b0c407f7 | 512 | ```Diff |
4a308d66 CB |
513 | commit b87243830e3b5e95fa31a17cf1bfebe55353bf13 |
514 | Author: Felix Abecassis <fabecassis@nvidia.com> | |
515 | Date: Fri Feb 2 06:19:13 2018 -0800 | |
516 | ||
517 | hooks: change the semantic of NVIDIA_VISIBLE_DEVICES="" | |
518 | ||
519 | With LXC, you can override the value of an environment variable to | |
520 | null, but you can't unset an existing variable. | |
521 | ||
522 | The NVIDIA hook was previously activated when NVIDIA_VISIBLE_DEVICES | |
523 | was set to null. As a result, it was not possible to disable the hook | |
524 | by overriding the environment variable in the configuration. | |
525 | ||
526 | The hook can now be disabled by setting NVIDIA_VISIBLE_DEVICES to | |
527 | null or to the new special value "void". | |
528 | ||
529 | Signed-off-by: Felix Abecassis <fabecassis@nvidia.com> | |
530 | ||
531 | ||
532 | commit d6337a5f9dc7311af168aa3d586fdf239f5a10d3 | |
533 | Author: Christian Brauner <christian.brauner@ubuntu.com> | |
534 | Date: Wed Jan 31 16:25:11 2018 +0100 | |
535 | ||
536 | cgroups: get controllers on the unified hierarchy | |
537 | ||
538 | Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> | |
539 | ||
540 | ``` | |
7419c83f | 541 | ## 21) Use `_exit()` To Terminate `fork()`ed Child Processes |
4ad78f87 CB |
542 | |
543 | - When `fork()`ing off a child process use `_exit()` to terminate it instead of | |
544 | `exit()`. The `exit()` function is not thread-safe and thus not suited for | |
545 | the shared library which must ensure that it is thread-safe. | |
b629739c | 546 | |
7419c83f | 547 | ## 22) Keep Arrays of `struct`s Aligned Horizontally When Initializing |
b629739c CB |
548 | |
549 | - Arrays of `struct`s are: | |
b0c407f7 | 550 | ```C |
b629739c CB |
551 | struct foo_struct { |
552 | int n; | |
553 | int m; | |
554 | int p; | |
555 | }; | |
556 | ||
557 | struct foo_struct new_instance[] = { | |
558 | { 1, 2, 3 }, | |
559 | { 4, 5, 6 }, | |
560 | { 7, 8, 9 }, | |
561 | }; | |
562 | ``` | |
563 | - Leave a single space after the opening `{` and before closing `}` of the | |
564 | largest member of the last column. | |
565 | - Always leave a single space between the largest member of the current column | |
566 | and the member in the next column. | |
567 | - A good example is | |
b0c407f7 | 568 | ```C |
b629739c CB |
569 | struct signame { |
570 | int num; | |
571 | const char *name; | |
572 | }; | |
573 | ||
574 | static const struct signame signames[] = { | |
575 | { SIGHUP, "HUP" }, | |
576 | { SIGINT, "INT" }, | |
577 | { SIGQUIT, "QUIT" }, | |
578 | { SIGILL, "ILL" }, | |
579 | { SIGABRT, "ABRT" }, | |
580 | { SIGFPE, "FPE" }, | |
581 | { SIGKILL, "KILL" }, | |
582 | { SIGSEGV, "SEGV" }, | |
583 | { SIGPIPE, "PIPE" }, | |
584 | { SIGALRM, "ALRM" }, | |
585 | { SIGTERM, "TERM" }, | |
586 | { SIGUSR1, "USR1" }, | |
587 | { SIGUSR2, "USR2" }, | |
588 | { SIGCHLD, "CHLD" }, | |
589 | { SIGCONT, "CONT" }, | |
590 | { SIGSTOP, "STOP" }, | |
591 | { SIGTSTP, "TSTP" }, | |
592 | { SIGTTIN, "TTIN" }, | |
593 | { SIGTTOU, "TTOU" }, | |
594 | #ifdef SIGTRAP | |
595 | { SIGTRAP, "TRAP" }, | |
596 | #endif | |
597 | #ifdef SIGIOT | |
598 | { SIGIOT, "IOT" }, | |
599 | #endif | |
600 | #ifdef SIGEMT | |
601 | { SIGEMT, "EMT" }, | |
602 | #endif | |
603 | #ifdef SIGBUS | |
604 | { SIGBUS, "BUS" }, | |
605 | #endif | |
606 | #ifdef SIGSTKFLT | |
607 | { SIGSTKFLT, "STKFLT" }, | |
608 | #endif | |
609 | #ifdef SIGCLD | |
610 | { SIGCLD, "CLD" }, | |
611 | #endif | |
612 | #ifdef SIGURG | |
613 | { SIGURG, "URG" }, | |
614 | #endif | |
615 | #ifdef SIGXCPU | |
616 | { SIGXCPU, "XCPU" }, | |
617 | #endif | |
618 | #ifdef SIGXFSZ | |
619 | { SIGXFSZ, "XFSZ" }, | |
620 | #endif | |
621 | #ifdef SIGVTALRM | |
622 | { SIGVTALRM, "VTALRM" }, | |
623 | #endif | |
624 | #ifdef SIGPROF | |
625 | { SIGPROF, "PROF" }, | |
626 | #endif | |
627 | #ifdef SIGWINCH | |
628 | { SIGWINCH, "WINCH" }, | |
629 | #endif | |
630 | #ifdef SIGIO | |
631 | { SIGIO, "IO" }, | |
632 | #endif | |
633 | #ifdef SIGPOLL | |
634 | { SIGPOLL, "POLL" }, | |
635 | #endif | |
636 | #ifdef SIGINFO | |
637 | { SIGINFO, "INFO" }, | |
638 | #endif | |
639 | #ifdef SIGLOST | |
640 | { SIGLOST, "LOST" }, | |
641 | #endif | |
642 | #ifdef SIGPWR | |
643 | { SIGPWR, "PWR" }, | |
644 | #endif | |
645 | #ifdef SIGUNUSED | |
646 | { SIGUNUSED, "UNUSED" }, | |
647 | #endif | |
648 | #ifdef SIGSYS | |
649 | { SIGSYS, "SYS" }, | |
650 | #endif | |
651 | }; | |
652 | ``` | |
a3759c1b | 653 | |
7419c83f | 654 | ## 23) Use `strlcpy()` instead of `strncpy()` |
a3759c1b CB |
655 | |
656 | When copying strings always use `strlcpy()` instead of `strncpy()`. The | |
657 | advantage of `strlcpy()` is that it will always append a `\0` byte to the | |
658 | string. | |
659 | ||
660 | Unless you have a valid reason to accept truncation you must check whether | |
661 | truncation has occurred, treat it as an error, and handle the error | |
662 | appropriately. | |
77f6262f | 663 | |
7419c83f | 664 | ## 24) Use `strlcat()` instead of `strncat()` |
77f6262f CB |
665 | |
666 | When concatenating strings always use `strlcat()` instead of `strncat()`. The | |
667 | advantage of `strlcat()` is that it will always append a `\0` byte to the | |
668 | string. | |
669 | ||
670 | Unless you have a valid reason to accept truncation you must check whether | |
671 | truncation has occurred, treat it as an error, and handle the error | |
672 | appropriately. | |
cf0fd972 CB |
673 | |
674 | ## 25) Use `__fallthrough__` in switch statements | |
675 | ||
676 | If LXC detects that the compiler is new enough it will tell it to check | |
677 | `switch` statements for non-documented fallthroughs. Please always place | |
678 | a `__fallthrough__` after a `case` which falls through the next one. | |
679 | ||
680 | ```c | |
681 | int lxc_attach_run_command(void *payload) | |
682 | { | |
683 | int ret = -1; | |
684 | lxc_attach_command_t *cmd = payload; | |
685 | ||
686 | ret = execvp(cmd->program, cmd->argv); | |
687 | if (ret < 0) { | |
688 | switch (errno) { | |
689 | case ENOEXEC: | |
690 | ret = 126; | |
691 | break; | |
692 | case ENOENT: | |
693 | ret = 127; | |
694 | break; | |
695 | } | |
696 | } | |
697 | ||
698 | SYSERROR("Failed to exec \"%s\"", cmd->program); | |
699 | return ret; | |
700 | } | |
701 | ``` | |
af32408a CB |
702 | |
703 | ## 24) Never use `fgets()` | |
704 | ||
705 | LXC does not allow the use of `fgets()`. Use `getline()` or other methods | |
706 | instead. | |
707 | ||
708 | ## 25) Never allocate memory on the stack | |
709 | ||
710 | This specifically forbids any usage of `alloca()` in the codebase. | |
711 | ||
712 | ## 26) Use cleanup macros supported by `gcc` and `clang` | |
713 | ||
714 | LXC has switched from manually cleaning up resources to using cleanup macros | |
715 | supported by `gcc` and `clang`: | |
716 | ```c | |
717 | __attribute__((__cleanup__(<my-cleanup-function-wrapper>))) | |
718 | ``` | |
719 | We do not allow manually cleanups anymore if there are appropriate macros. | |
720 | Currently the following macros are supported: | |
721 | ```c | |
722 | /* close file descriptor */ | |
723 | __do_close_prot_errno | |
724 | ||
725 | /* free allocated memory */ | |
726 | __do_free __attribute__((__cleanup__(__auto_free__))) | |
727 | ||
728 | /* close FILEs */ | |
729 | __do_fclose __attribute__((__cleanup__(__auto_fclose__))) | |
730 | ||
731 | /* close DIRs */ | |
732 | __do_closedir __attribute__((__cleanup__(__auto_closedir__))) | |
733 | ``` | |
734 | For example: | |
735 | ```c | |
41e12945 | 736 | void turn_into_dependent_mounts(void) |
af32408a CB |
737 | { |
738 | __do_free char *line = NULL; | |
739 | __do_fclose FILE *f = NULL; | |
41e12945 | 740 | __do_close int memfd = -EBADF, mntinfo_fd = -EBADF; |
af32408a CB |
741 | int ret; |
742 | ssize_t copied; | |
743 | size_t len = 0; | |
744 | ||
745 | mntinfo_fd = open("/proc/self/mountinfo", O_RDONLY | O_CLOEXEC); | |
746 | if (mntinfo_fd < 0) { | |
747 | SYSERROR("Failed to open \"/proc/self/mountinfo\""); | |
748 | return; | |
749 | } | |
750 | ||
751 | memfd = memfd_create(".lxc_mountinfo", MFD_CLOEXEC); | |
752 | if (memfd < 0) { | |
753 | char template[] = P_tmpdir "/.lxc_mountinfo_XXXXXX"; | |
754 | ||
755 | if (errno != ENOSYS) { | |
756 | SYSERROR("Failed to create temporary in-memory file"); | |
757 | return; | |
758 | } | |
759 | ||
760 | memfd = lxc_make_tmpfile(template, true); | |
761 | if (memfd < 0) { | |
762 | WARN("Failed to create temporary file"); | |
763 | return; | |
764 | } | |
765 | } | |
766 | ||
767 | again: | |
768 | copied = lxc_sendfile_nointr(memfd, mntinfo_fd, NULL, LXC_SENDFILE_MAX); | |
769 | if (copied < 0) { | |
770 | if (errno == EINTR) | |
771 | goto again; | |
772 | ||
773 | SYSERROR("Failed to copy \"/proc/self/mountinfo\""); | |
774 | return; | |
775 | } | |
776 | ||
777 | ret = lseek(memfd, 0, SEEK_SET); | |
778 | if (ret < 0) { | |
779 | SYSERROR("Failed to reset file descriptor offset"); | |
780 | return; | |
781 | } | |
782 | ||
41e12945 | 783 | f = fdopen(memfd, "re"); |
af32408a CB |
784 | if (!f) { |
785 | SYSERROR("Failed to open copy of \"/proc/self/mountinfo\" to mark all shared. Continuing"); | |
786 | return; | |
787 | } | |
788 | ||
789 | /* | |
790 | * After a successful fdopen() memfd will be closed when calling | |
791 | * fclose(f). Calling close(memfd) afterwards is undefined. | |
792 | */ | |
793 | move_fd(memfd); | |
794 | ||
795 | while (getline(&line, &len, f) != -1) { | |
796 | char *opts, *target; | |
797 | ||
798 | target = get_field(line, 4); | |
799 | if (!target) | |
800 | continue; | |
801 | ||
802 | opts = get_field(target, 2); | |
803 | if (!opts) | |
804 | continue; | |
805 | ||
806 | null_endofword(opts); | |
807 | if (!strstr(opts, "shared")) | |
808 | continue; | |
809 | ||
810 | null_endofword(target); | |
811 | ret = mount(NULL, target, NULL, MS_SLAVE, NULL); | |
812 | if (ret < 0) { | |
41e12945 | 813 | SYSERROR("Failed to recursively turn old root mount tree into dependent mount. Continuing..."); |
af32408a CB |
814 | continue; |
815 | } | |
41e12945 | 816 | TRACE("Recursively turned old root mount tree into dependent mount"); |
af32408a | 817 | } |
41e12945 | 818 | TRACE("Turned all mount table entries into dependent mount"); |
af32408a CB |
819 | } |
820 | ``` |