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.
24 #### Only use `/* */` Style Comments
26 - Any comments that are added must use `/* */`.
27 - All comments should start on the same line as the opening `/*`.
28 - Single-line comments should simply be placed between `/* */`. For example:
30 /* Define pivot_root() if missing from the C library */
32 - Multi-line comments should end with the closing `*/` on a separate line. For
35 /* At this point the old-root is mounted on top of our new-root
36 * To unmounted it we must not be chdir()ed into it, so escape back
41 #### Try To Wrap At 80chars
43 - This is not strictly enforced. It is perfectly valid to sometimes
44 overflow this limit if it helps clarity. Nonetheless, try to stick to it
45 and use common sense to decide when not to.
49 - Error messages must start with a capital letter and must **not** end with a
51 - They should be descriptive, without being needlessly long. It is best to just
52 use already existing error messages as examples.
53 - Examples of acceptable error messages are:
55 SYSERROR("Failed to create directory \"%s\"", path);
56 WARN("\"/dev\" directory does not exist. Proceeding without autodev being set up");
59 #### Return Error Codes
61 - When writing a function that can fail in a non-binary way try to return
62 meaningful negative error codes (e.g. `return -EINVAL;`).
64 #### All Unexported Functions Must Be Declared `static`
66 - Functions which are only used in the current file and are not exported
67 within the codebase need to be declared with the `static` attribute.
69 #### All Exported Functions Must Be Declared `extern` In A Header File
71 - Functions which are used in different files in the library should be declared
72 in a suitable `*.c` file and exposed in a suitable `*.h` file. When defining
73 the function in the `*.c` file the function signature should not be preceded
74 by the `extern` keyword. When declaring the function signature in the `*.h`
75 file it must be preceded by the `extern` keyword. For example:
77 /* Valid function definition in a *.c file */
78 ssize_t lxc_write_nointr(int fd, const void* buf, size_t count)
82 ret = write(fd, buf, count);
83 if (ret < 0 && errno == EINTR)
88 /* Valid function declaration in a *.h file */
89 extern ssize_t lxc_write_nointr(int fd, const void* buf, size_t count);
92 #### All Names Must Be In lower_case
94 - All functions and variable names must use lower case.
96 #### Declaring Variables
98 - variables should be declared at the top of the function or at the beginning
99 of a new scope but **never** in the middle of a scope
100 1. uninitialized variables
101 - put base types before complex types
102 - put standard types defined by libc before types defined by LXC
103 - put multiple declarations of the same type on the same line
104 2. initialized variables
105 - put base types before complex types
106 - put standard types defined by libc before types defined by LXC
107 - put multiple declarations of the same type on the same line
108 - Examples of good declarations can be seen in the following function:
110 int lxc_clear_procs(struct lxc_conf *c, const char *key)
112 struct lxc_list *it, *next;
114 const char *k = NULL;
116 if (strcmp(key, "lxc.proc") == 0)
118 else if (strncmp(key, "lxc.proc.", sizeof("lxc.proc.") - 1) == 0)
119 k = key + sizeof("lxc.proc.") - 1;
123 lxc_list_for_each_safe(it, &c->procs, next) {
124 struct lxc_proc *proc = it->elem;
126 if (!all && strcmp(proc->filename, k) != 0)
129 free(proc->filename);
139 #### Single-line `if` blocks should not be enclosed in `{}`
141 - This also affects `if-else` ladders if and only if all constituting
143 single-line conditions. If there is at least one non-single-line
144 condition `{}` must be used.
147 /* no brackets needed */
151 /* The else branch has more than one-line and so needs {}. This entails that
152 * the if branch also needs to have {}.
154 if ( errno == EROFS ) {
155 WARN("Warning: Read Only file system while creating %s", path);
157 SYSERROR("Error creating %s", path);
162 for (i = 0; list[i]; i++)
163 if (strcmp(list[i], entry) == 0)
168 WARN("Failed to set FD_CLOEXEC flag on slave fd %d of "
169 "pty device \"%s\": %s", pty_info->slave,
170 pty_info->name, strerror(errno));
174 for (i = 0; i < sizeof(limit_opt)/sizeof(limit_opt[0]); ++i) {
175 if (strcmp(res, limit_opt[i].name) == 0)
176 return limit_opt[i].value;
180 #### Functions Not Returning Booleans Must Assigned Return Value Before Performing Checks
182 - When checking whether a function not returning booleans was successful or not
183 the returned value must be assigned before it is checked (`str{n}cmp()`
184 functions being one notable exception). For example:
186 /* assign value to "ret" first */
187 ret = mount(sourcepath, cgpath, "cgroup", remount_flags, NULL);
188 /* check whether function was successful */
190 SYSERROR("Failed to remount \"%s\" ro", cgpath);
195 Functions returning booleans can be checked directly. For example:
197 extern bool lxc_string_in_array(const char *needle, const char **haystack);
199 /* check right away */
200 if (lxc_string_in_array("ns", (const char **)h->subsystems))
204 #### Do Not Use C99 Variable Length Arrays (VLA)
206 - They are made optional and there is no guarantee that future C standards
209 #### Use Standard libc Macros When Exiting
211 - libc provides `EXIT_FAILURE` and `EXIT_SUCCESS`. Use them whenever possible
212 in the child of `fork()`ed process or when exiting from a `main()` function.
216 `goto`s are an essential language construct of C and are perfect to perform
217 cleanup operations or simplify the logic of functions. However, here are the
219 - use descriptive `goto` labels.
220 For example, if you know that this label is only used as an error path you
221 should use something like `on_error` instead of `out` as label name.
222 - **only** jump downwards unless you are handling `EAGAIN` errors and want to
223 avoid `do-while` constructs.
224 - An example of a good usage of `goto` is:
226 static int set_config_idmaps(const char *key, const char *value,
227 struct lxc_conf *lxc_conf, void *data)
229 unsigned long hostid, nsid, range;
232 struct lxc_list *idmaplist = NULL;
233 struct id_map *idmap = NULL;
235 if (lxc_config_value_empty(value))
236 return lxc_clear_idmaps(lxc_conf);
238 idmaplist = malloc(sizeof(*idmaplist));
242 idmap = malloc(sizeof(*idmap));
245 memset(idmap, 0, sizeof(*idmap));
247 ret = parse_idmaps(value, &type, &nsid, &hostid, &range);
249 ERROR("Failed to parse id mappings");
253 INFO("Read uid map: type %c nsid %lu hostid %lu range %lu", type, nsid, hostid, range);
255 idmap->idtype = ID_TYPE_UID;
256 else if (type == 'g')
257 idmap->idtype = ID_TYPE_GID;
261 idmap->hostid = hostid;
263 idmap->range = range;
264 idmaplist->elem = idmap;
265 lxc_list_add_tail(&lxc_conf->id_map, idmaplist);
267 if (!lxc_conf->root_nsuid_map && idmap->idtype == ID_TYPE_UID)
268 if (idmap->nsid == 0)
269 lxc_conf->root_nsuid_map = idmap;
272 if (!lxc_conf->root_nsgid_map && idmap->idtype == ID_TYPE_GID)
273 if (idmap->nsid == 0)
274 lxc_conf->root_nsgid_map = idmap;
288 #### Use Booleans instead of integers
290 - When something can be conceptualized in a binary way use a boolean not
293 #### Cleanup Functions Must Handle The Object's Null Type And Being Passed Already Cleaned Up Objects
295 - If you implement a custom cleanup function to e.g. free a complex type
296 you declared you must ensure that the object's null type is handled and
297 treated as a NOOP. For example:
299 void lxc_free_array(void **array, lxc_free_fn element_free_fn)
302 for (p = array; p && *p; p++)
307 - Cleanup functions should also expect to be passed already cleaned up objects.
308 One way to handle this cleanly is to initialize the cleaned up variable to
309 a special value that signals the function that the element has already been
310 freed on the next call. For example, the following function cleans up file
311 descriptors and sets the already closed file descriptors to `-EBADF`. On the
312 next call it can simply check whether the file descriptor is positive and
315 static void lxc_put_attach_clone_payload(struct attach_clone_payload *p)
317 if (p->ipc_socket >= 0) {
318 shutdown(p->ipc_socket, SHUT_RDWR);
319 close(p->ipc_socket);
320 p->ipc_socket = -EBADF;
323 if (p->pty_fd >= 0) {
329 lxc_proc_put_context_info(p->init_ctx);
335 ### Cast to `(void)` When Intentionally Ignoring Return Values
337 - There are cases where you do not care about the return value of a function.
338 Please cast the return value to `(void)` when doing so.
339 - Standard library functions or functions which are known to be ignored by
340 default do not need to be cast to `(void)`. Classical candidates are
341 `close()` and `fclose()`.
344 for (i = 0; hierarchies[i]; i++) {
346 char *path = hierarchies[i]->fullcgpath;
348 ret = chowmod(path, destuid, nsgid, 0755);
352 /* failures to chown() these are inconvenient but not
353 * detrimental we leave these owned by the container launcher,
354 * so that container root can write to the files to attach. we
355 * chmod() them 664 so that container systemd can write to the
356 * files (which systemd in wily insists on doing).
359 if (hierarchies[i]->version == cgroup_super_magic) {
360 fullpath = must_make_path(path, "tasks", null);
361 (void)chowmod(fullpath, destuid, nsgid, 0664);
365 fullpath = must_make_path(path, "cgroup.procs", null);
366 (void)chowmod(fullpath, destuid, 0, 0664);
369 if (hierarchies[i]->version != cgroup2_super_magic)
372 fullpath = must_make_path(path, "cgroup.subtree_control", null);
373 (void)chowmod(fullpath, destuid, nsgid, 0664);
376 fullpath = must_make_path(path, "cgroup.threads", null);
377 (void)chowmod(fullpath, destuid, nsgid, 0664);
382 #### Use `_exit()` in `fork()`ed Processes
384 - This has multiple reasons but the gist is:
385 - `exit()` is not thread-safe
386 - `exit()` in libc runs exit handlers which might interfer with the parents
389 #### Use `for (;;)` instead of `while (1)` or `while (true)`
391 - Let's be honest, it is really the only sensible way to do this.
393 #### Use The Set Of Supported DCO Statements
395 - Signed-off-by: Random J Developer <random@developer.org>
396 - You did write this code or have the right to contribute it to LXC.
397 - Acked-by: Random J Developer <random@developer.org>
398 - You did read the code and think it is correct. This is usually only used by
399 maintainers or developers that have made significant contributions and can
400 vouch for the correctness of someone else's code.
401 - Reviewed-by: Random J Developer <random@developer.org>
402 - You did review the code and vouch for its correctness, i.e. you'd be
403 prepared to fix bugs it might cause. This is usually only used by
404 maintainers or developers that have made significant contributions and can
405 vouch for the correctness of someone else's code.
406 - Co-developed-by: Random J Developer <random@developer.org>
407 - The code can not be reasonably attributed to a single developer, i.e.
408 you worked on this together.
409 - Tested-by: Random J Developer <random@developer.org>
410 - You verified that the code fixes a given bug or is behaving as advertised.
411 - Reported-by: Random J Developer <random@developer.org>
412 - You found and reported the bug.
413 - Suggested-by: Random J Developer <random@developer.org>
414 - You wrote the code but someone contributed the idea. This line is usually
415 overlooked but it is a sign of good etiquette and coding ethics: if someone
416 helped you solve a problem or had a clever idea do not silently claim it by
417 slapping your Signed-off-by underneath. Be honest and add a Suggested-by.
419 #### Commit Message Outline
421 - You **must** stick to the 80chars limit especially in the title of the commit
423 - Please use English commit messages only.
424 - use meaningful commit messages.
425 - Use correct spelling and grammar.
426 If you are not a native speaker and/or feel yourself struggling with this it
427 is perfectly fine to point this out and there's no need to apologize. Usually
428 developers will be happy to pull your branch and adopt the commit message.
429 - Please always use the affected file (without the file type suffix) or module
430 as a prefix in the commit message.
431 - Examples of good commit messages are:
433 commit b87243830e3b5e95fa31a17cf1bfebe55353bf13
434 Author: Felix Abecassis <fabecassis@nvidia.com>
435 Date: Fri Feb 2 06:19:13 2018 -0800
437 hooks: change the semantic of NVIDIA_VISIBLE_DEVICES=""
439 With LXC, you can override the value of an environment variable to
440 null, but you can't unset an existing variable.
442 The NVIDIA hook was previously activated when NVIDIA_VISIBLE_DEVICES
443 was set to null. As a result, it was not possible to disable the hook
444 by overriding the environment variable in the configuration.
446 The hook can now be disabled by setting NVIDIA_VISIBLE_DEVICES to
447 null or to the new special value "void".
449 Signed-off-by: Felix Abecassis <fabecassis@nvidia.com>
452 commit d6337a5f9dc7311af168aa3d586fdf239f5a10d3
453 Author: Christian Brauner <christian.brauner@ubuntu.com>
454 Date: Wed Jan 31 16:25:11 2018 +0100
456 cgroups: get controllers on the unified hierarchy
458 Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>