3 - The coding style guide refers to new code. But legacy code can be cleaned up
4 and we are happy to take those patches.
5 - Just because there is still code in LXC that doesn't adhere to the coding
6 standards outlined here does not license not adhering to the coding style. In
7 other words: please stick to the coding style.
8 - Maintainers are free to ignore rules specified here when merging pull
9 requests. This guideline might seem a little weird but it exits to ease new
10 developers into the code base and to prevent unnecessary bikeshedding. If
11 a maintainer feels hat enforcing a specific rule in a given commit would do
12 more harm than good they should always feel free to ignore the rule.
14 Furthermore, when merging pull requests that do not adhere to our coding
15 style maintainers should feel free to grab the commit, adapt it to our coding
16 style and add their Signed-off-by line to it. This is especially helpful to
17 make it easier for first-time contributors and to prevent having pull
18 requests being stuck in the merge queue because of minor details.
19 - We currently do not provide automatic coding style checks but if a suitable
20 tool is found we are happy to integrate it into our test suite. It is
21 possible and recommended to use the `clang-format` binary to check your code.
22 The following options are an approximation of the coding style used here.
23 Simply create a file called `.clang-format` in your home directory with the
26 cat << EOF > "${HOME}"/.clang-format
27 AlignEscapedNewlines: Left
28 BreakBeforeBraces: Attach
29 AlwaysBreakBeforeMultilineStrings: false
30 BreakBeforeBinaryOperators: None
31 MaxEmptyLinesToKeep: 1
32 PenaltyBreakBeforeFirstCallParameter: 1000000
33 BinPackArguments: true
34 BinPackParameters: true
35 AllowAllParametersOfDeclarationOnNextLine: false
36 AlignAfterOpenBracket: true
37 SpacesInSquareBrackets: false
38 SpacesInCStyleCastParentheses: false
39 SpaceInEmptyParentheses: false
40 SpaceBeforeParens: ControlStatements
41 SpaceAfterCStyleCast: false
43 PenaltyReturnTypeOnItsOwnLine: 10000
44 PenaltyExcessCharacter: 10
46 ForEachMacros: ['lxc_list_for_each', 'lxc_list_for_each_safe']
47 AllowShortLoopsOnASingleLine: false
48 AllowShortIfStatementsOnASingleLine: false
49 AllowShortFunctionsOnASingleLine: None
50 AllowShortCaseLabelsOnASingleLine: false
51 AllowShortBlocksOnASingleLine: false
56 BreakBeforeBraces: Linux
57 AllowShortIfStatementsOnASingleLine: false
58 IndentCaseLabels: false
61 However, it will not handle all cases correctly. For example, most `struct`
62 initializations will not be correct. In such cases please refer to the coding
69 #### Only use `/* */` Style Comments
71 - Any comments that are added must use `/* */`.
72 - All comments should start on the same line as the opening `/*`.
73 - Single-line comments should simply be placed between `/* */`. For example:
75 /* Define pivot_root() if missing from the C library */
77 - Multi-line comments should end with the closing `*/` on a separate line. For
80 /* At this point the old-root is mounted on top of our new-root
81 * To unmounted it we must not be chdir()ed into it, so escape back
86 #### Try To Wrap At 80chars
88 - This is not strictly enforced. It is perfectly valid to sometimes
89 overflow this limit if it helps clarity. Nonetheless, try to stick to it
90 and use common sense to decide when not to.
94 - Error messages must start with a capital letter and must **not** end with a
96 - They should be descriptive, without being needlessly long. It is best to just
97 use already existing error messages as examples.
98 - Examples of acceptable error messages are:
100 SYSERROR("Failed to create directory \"%s\"", path);
101 WARN("\"/dev\" directory does not exist. Proceeding without autodev being set up");
104 #### Return Error Codes
106 - When writing a function that can fail in a non-binary way try to return
107 meaningful negative error codes (e.g. `return -EINVAL;`).
109 #### All Unexported Functions Must Be Declared `static`
111 - Functions which are only used in the current file and are not exported
112 within the codebase need to be declared with the `static` attribute.
114 #### All Exported Functions Must Be Declared `extern` In A Header File
116 - Functions which are used in different files in the library should be declared
117 in a suitable `*.c` file and exposed in a suitable `*.h` file. When defining
118 the function in the `*.c` file the function signature should not be preceded
119 by the `extern` keyword. When declaring the function signature in the `*.h`
120 file it must be preceded by the `extern` keyword. For example:
122 /* Valid function definition in a *.c file */
123 ssize_t lxc_write_nointr(int fd, const void* buf, size_t count)
127 ret = write(fd, buf, count);
128 if (ret < 0 && errno == EINTR)
133 /* Valid function declaration in a *.h file */
134 extern ssize_t lxc_write_nointr(int fd, const void* buf, size_t count);
137 #### All Names Must Be In lower_case
139 - All functions and variable names must use lower case.
141 #### Declaring Variables
143 - variables should be declared at the top of the function or at the beginning
144 of a new scope but **never** in the middle of a scope
145 1. uninitialized variables
146 - put base types before complex types
147 - put standard types defined by libc before types defined by LXC
148 - put multiple declarations of the same type on the same line
149 2. initialized variables
150 - put base types before complex types
151 - put standard types defined by libc before types defined by LXC
152 - put multiple declarations of the same type on the same line
153 - Examples of good declarations can be seen in the following function:
155 int lxc_clear_procs(struct lxc_conf *c, const char *key)
157 struct lxc_list *it, *next;
159 const char *k = NULL;
161 if (strcmp(key, "lxc.proc") == 0)
163 else if (strncmp(key, "lxc.proc.", sizeof("lxc.proc.") - 1) == 0)
164 k = key + sizeof("lxc.proc.") - 1;
168 lxc_list_for_each_safe(it, &c->procs, next) {
169 struct lxc_proc *proc = it->elem;
171 if (!all && strcmp(proc->filename, k) != 0)
174 free(proc->filename);
184 #### Single-line `if` blocks should not be enclosed in `{}`
186 - This also affects `if-else` ladders if and only if all constituting
188 single-line conditions. If there is at least one non-single-line
189 condition `{}` must be used.
192 /* no brackets needed */
196 /* The else branch has more than one-line and so needs {}. This entails that
197 * the if branch also needs to have {}.
199 if ( errno == EROFS ) {
200 WARN("Warning: Read Only file system while creating %s", path);
202 SYSERROR("Error creating %s", path);
207 for (i = 0; list[i]; i++)
208 if (strcmp(list[i], entry) == 0)
213 WARN("Failed to set FD_CLOEXEC flag on slave fd %d of "
214 "pty device \"%s\": %s", pty_info->slave,
215 pty_info->name, strerror(errno));
219 for (i = 0; i < sizeof(limit_opt)/sizeof(limit_opt[0]); ++i) {
220 if (strcmp(res, limit_opt[i].name) == 0)
221 return limit_opt[i].value;
225 #### Functions Not Returning Booleans Must Assign Return Value Before Performing Checks
227 - When checking whether a function not returning booleans was successful or not
228 the returned value must be assigned before it is checked (`str{n}cmp()`
229 functions being one notable exception). For example:
231 /* assign value to "ret" first */
232 ret = mount(sourcepath, cgpath, "cgroup", remount_flags, NULL);
233 /* check whether function was successful */
235 SYSERROR("Failed to remount \"%s\" ro", cgpath);
240 Functions returning booleans can be checked directly. For example:
242 extern bool lxc_string_in_array(const char *needle, const char **haystack);
244 /* check right away */
245 if (lxc_string_in_array("ns", (const char **)h->subsystems))
249 #### Non-Boolean Functions That Behave Like Boolean Functions Must Explicitly Check Against A Value
251 - This rule mainly exists for `str{n}cmp()` type functions. In most cases they
252 are used like a boolean function to check whether a string matches or not.
253 But they return an integer. It is perfectly fine to check `str{n}cmp()`
254 functions directly but you must compare explicitly against a value. That is
255 to say, while they are conceptually boolean functions they shouldn't be
256 treated as such since they don't really behave like boolean functions. So
257 `if (!str{n}cmp())` and `if (str{n}cmp())` checks must not be used. Good
258 examples are found in the following functions:
260 static int set_config_hooks(const char *key, const char *value,
261 struct lxc_conf *lxc_conf, void *data)
265 if (lxc_config_value_empty(value))
266 return lxc_clear_hooks(lxc_conf, key);
268 if (strcmp(key + 4, "hook") == 0) {
269 ERROR("lxc.hook must not have a value");
273 copy = strdup(value);
277 if (strcmp(key + 9, "pre-start") == 0)
278 return add_hook(lxc_conf, LXCHOOK_PRESTART, copy);
279 else if (strcmp(key + 9, "start-host") == 0)
280 return add_hook(lxc_conf, LXCHOOK_START_HOST, copy);
281 else if (strcmp(key + 9, "pre-mount") == 0)
282 return add_hook(lxc_conf, LXCHOOK_PREMOUNT, copy);
283 else if (strcmp(key + 9, "autodev") == 0)
284 return add_hook(lxc_conf, LXCHOOK_AUTODEV, copy);
285 else if (strcmp(key + 9, "mount") == 0)
286 return add_hook(lxc_conf, LXCHOOK_MOUNT, copy);
287 else if (strcmp(key + 9, "start") == 0)
288 return add_hook(lxc_conf, LXCHOOK_START, copy);
289 else if (strcmp(key + 9, "stop") == 0)
290 return add_hook(lxc_conf, LXCHOOK_STOP, copy);
291 else if (strcmp(key + 9, "post-stop") == 0)
292 return add_hook(lxc_conf, LXCHOOK_POSTSTOP, copy);
293 else if (strcmp(key + 9, "clone") == 0)
294 return add_hook(lxc_conf, LXCHOOK_CLONE, copy);
295 else if (strcmp(key + 9, "destroy") == 0)
296 return add_hook(lxc_conf, LXCHOOK_DESTROY, copy);
303 #### Do Not Use C99 Variable Length Arrays (VLA)
305 - They are made optional and there is no guarantee that future C standards
308 #### Use Standard libc Macros When Exiting
310 - libc provides `EXIT_FAILURE` and `EXIT_SUCCESS`. Use them whenever possible
311 in the child of `fork()`ed process or when exiting from a `main()` function.
315 `goto`s are an essential language construct of C and are perfect to perform
316 cleanup operations or simplify the logic of functions. However, here are the
318 - use descriptive `goto` labels.
319 For example, if you know that this label is only used as an error path you
320 should use something like `on_error` instead of `out` as label name.
321 - **only** jump downwards unless you are handling `EAGAIN` errors and want to
322 avoid `do-while` constructs.
323 - An example of a good usage of `goto` is:
325 static int set_config_idmaps(const char *key, const char *value,
326 struct lxc_conf *lxc_conf, void *data)
328 unsigned long hostid, nsid, range;
331 struct lxc_list *idmaplist = NULL;
332 struct id_map *idmap = NULL;
334 if (lxc_config_value_empty(value))
335 return lxc_clear_idmaps(lxc_conf);
337 idmaplist = malloc(sizeof(*idmaplist));
341 idmap = malloc(sizeof(*idmap));
344 memset(idmap, 0, sizeof(*idmap));
346 ret = parse_idmaps(value, &type, &nsid, &hostid, &range);
348 ERROR("Failed to parse id mappings");
352 INFO("Read uid map: type %c nsid %lu hostid %lu range %lu", type, nsid, hostid, range);
354 idmap->idtype = ID_TYPE_UID;
355 else if (type == 'g')
356 idmap->idtype = ID_TYPE_GID;
360 idmap->hostid = hostid;
362 idmap->range = range;
363 idmaplist->elem = idmap;
364 lxc_list_add_tail(&lxc_conf->id_map, idmaplist);
366 if (!lxc_conf->root_nsuid_map && idmap->idtype == ID_TYPE_UID)
367 if (idmap->nsid == 0)
368 lxc_conf->root_nsuid_map = idmap;
371 if (!lxc_conf->root_nsgid_map && idmap->idtype == ID_TYPE_GID)
372 if (idmap->nsid == 0)
373 lxc_conf->root_nsgid_map = idmap;
387 #### Use Booleans instead of integers
389 - When something can be conceptualized in a binary way use a boolean not
392 #### Cleanup Functions Must Handle The Object's Null Type And Being Passed Already Cleaned Up Objects
394 - If you implement a custom cleanup function to e.g. free a complex type
395 you declared you must ensure that the object's null type is handled and
396 treated as a NOOP. For example:
398 void lxc_free_array(void **array, lxc_free_fn element_free_fn)
401 for (p = array; p && *p; p++)
406 - Cleanup functions should also expect to be passed already cleaned up objects.
407 One way to handle this cleanly is to initialize the cleaned up variable to
408 a special value that signals the function that the element has already been
409 freed on the next call. For example, the following function cleans up file
410 descriptors and sets the already closed file descriptors to `-EBADF`. On the
411 next call it can simply check whether the file descriptor is positive and
414 static void lxc_put_attach_clone_payload(struct attach_clone_payload *p)
416 if (p->ipc_socket >= 0) {
417 shutdown(p->ipc_socket, SHUT_RDWR);
418 close(p->ipc_socket);
419 p->ipc_socket = -EBADF;
422 if (p->pty_fd >= 0) {
428 lxc_proc_put_context_info(p->init_ctx);
434 ### Cast to `(void)` When Intentionally Ignoring Return Values
436 - There are cases where you do not care about the return value of a function.
437 Please cast the return value to `(void)` when doing so.
438 - Standard library functions or functions which are known to be ignored by
439 default do not need to be cast to `(void)`. Classical candidates are
440 `close()` and `fclose()`.
443 for (i = 0; hierarchies[i]; i++) {
445 char *path = hierarchies[i]->fullcgpath;
447 ret = chowmod(path, destuid, nsgid, 0755);
451 /* failures to chown() these are inconvenient but not
452 * detrimental we leave these owned by the container launcher,
453 * so that container root can write to the files to attach. we
454 * chmod() them 664 so that container systemd can write to the
455 * files (which systemd in wily insists on doing).
458 if (hierarchies[i]->version == cgroup_super_magic) {
459 fullpath = must_make_path(path, "tasks", null);
460 (void)chowmod(fullpath, destuid, nsgid, 0664);
464 fullpath = must_make_path(path, "cgroup.procs", null);
465 (void)chowmod(fullpath, destuid, 0, 0664);
468 if (hierarchies[i]->version != cgroup2_super_magic)
471 fullpath = must_make_path(path, "cgroup.subtree_control", null);
472 (void)chowmod(fullpath, destuid, nsgid, 0664);
475 fullpath = must_make_path(path, "cgroup.threads", null);
476 (void)chowmod(fullpath, destuid, nsgid, 0664);
481 #### Use `for (;;)` instead of `while (1)` or `while (true)`
483 - Let's be honest, it is really the only sensible way to do this.
485 #### Use The Set Of Supported DCO Statements
487 - Signed-off-by: Random J Developer <random@developer.org>
488 - You did write this code or have the right to contribute it to LXC.
489 - Acked-by: Random J Developer <random@developer.org>
490 - You did read the code and think it is correct. This is usually only used by
491 maintainers or developers that have made significant contributions and can
492 vouch for the correctness of someone else's code.
493 - Reviewed-by: Random J Developer <random@developer.org>
494 - You did review the code and vouch for its correctness, i.e. you'd be
495 prepared to fix bugs it might cause. This is usually only used by
496 maintainers or developers that have made significant contributions and can
497 vouch for the correctness of someone else's code.
498 - Co-developed-by: Random J Developer <random@developer.org>
499 - The code can not be reasonably attributed to a single developer, i.e.
500 you worked on this together.
501 - Tested-by: Random J Developer <random@developer.org>
502 - You verified that the code fixes a given bug or is behaving as advertised.
503 - Reported-by: Random J Developer <random@developer.org>
504 - You found and reported the bug.
505 - Suggested-by: Random J Developer <random@developer.org>
506 - You wrote the code but someone contributed the idea. This line is usually
507 overlooked but it is a sign of good etiquette and coding ethics: if someone
508 helped you solve a problem or had a clever idea do not silently claim it by
509 slapping your Signed-off-by underneath. Be honest and add a Suggested-by.
511 #### Commit Message Outline
513 - You **must** stick to the 80chars limit especially in the title of the commit
515 - Please use English commit messages only.
516 - use meaningful commit messages.
517 - Use correct spelling and grammar.
518 If you are not a native speaker and/or feel yourself struggling with this it
519 is perfectly fine to point this out and there's no need to apologize. Usually
520 developers will be happy to pull your branch and adopt the commit message.
521 - Please always use the affected file (without the file type suffix) or module
522 as a prefix in the commit message.
523 - Examples of good commit messages are:
525 commit b87243830e3b5e95fa31a17cf1bfebe55353bf13
526 Author: Felix Abecassis <fabecassis@nvidia.com>
527 Date: Fri Feb 2 06:19:13 2018 -0800
529 hooks: change the semantic of NVIDIA_VISIBLE_DEVICES=""
531 With LXC, you can override the value of an environment variable to
532 null, but you can't unset an existing variable.
534 The NVIDIA hook was previously activated when NVIDIA_VISIBLE_DEVICES
535 was set to null. As a result, it was not possible to disable the hook
536 by overriding the environment variable in the configuration.
538 The hook can now be disabled by setting NVIDIA_VISIBLE_DEVICES to
539 null or to the new special value "void".
541 Signed-off-by: Felix Abecassis <fabecassis@nvidia.com>
544 commit d6337a5f9dc7311af168aa3d586fdf239f5a10d3
545 Author: Christian Brauner <christian.brauner@ubuntu.com>
546 Date: Wed Jan 31 16:25:11 2018 +0100
548 cgroups: get controllers on the unified hierarchy
550 Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
553 #### Use `_exit()` To Terminate `fork()`ed Child Processes
555 - When `fork()`ing off a child process use `_exit()` to terminate it instead of
556 `exit()`. The `exit()` function is not thread-safe and thus not suited for
557 the shared library which must ensure that it is thread-safe.
559 #### Keep Arrays of `struct`s Aligned Horizontally When Initializing
561 - Arrays of `struct`s are:
569 struct foo_struct new_instance[] = {
575 - Leave a single space after the opening `{` and before closing `}` of the
576 largest member of the last column.
577 - Always leave a single space between the largest member of the current column
578 and the member in the next column.
586 static const struct signame signames[] = {
619 { SIGSTKFLT, "STKFLT" },
634 { SIGVTALRM, "VTALRM" },
640 { SIGWINCH, "WINCH" },
658 { SIGUNUSED, "UNUSED" },
666 #### Use `strlcpy()` instead of `strncpy()`
668 When copying strings always use `strlcpy()` instead of `strncpy()`. The
669 advantage of `strlcpy()` is that it will always append a `\0` byte to the
672 Unless you have a valid reason to accept truncation you must check whether
673 truncation has occurred, treat it as an error, and handle the error
676 #### Use `strlcat()` instead of `strncat()`
678 When concatenating strings always use `strlcat()` instead of `strncat()`. The
679 advantage of `strlcat()` is that it will always append a `\0` byte to the
682 Unless you have a valid reason to accept truncation you must check whether
683 truncation has occurred, treat it as an error, and handle the error