-#### General Notes
+LXC Coding Style Guide
+======================
+
+In general the LXC project follows the Linux kernel coding style. However,
+there are a few differences. They are outlined in this document.
+
+The Linux kernel coding style guide can be found within the kernel tree:
+
+ Documentation/process/coding-style.rst
+
+It can be accessed online too:
+
+https://www.kernel.org/doc/html/latest/process/coding-style.html
+
+## 1) General Notes
- The coding style guide refers to new code. But legacy code can be cleaned up
and we are happy to take those patches.
style and add their Signed-off-by line to it. This is especially helpful to
make it easier for first-time contributors and to prevent having pull
requests being stuck in the merge queue because of minor details.
+- We currently do not provide automatic coding style checks but if a suitable
+ tool is found we are happy to integrate it into our test suite. It is
+ possible and recommended to use the `clang-format` binary to check your code.
+ The following options are an approximation of the coding style used here.
+ Simply create a file called `.clang-format` in your home directory with the
+ following options:
+ ```sh
+ cat << EOF > "${HOME}"/.clang-format
+ AlignEscapedNewlines: Left
+ BreakBeforeBraces: Attach
+ AlwaysBreakBeforeMultilineStrings: false
+ BreakBeforeBinaryOperators: None
+ MaxEmptyLinesToKeep: 1
+ PenaltyBreakBeforeFirstCallParameter: 1000000
+ BinPackArguments: true
+ BinPackParameters: true
+ AllowAllParametersOfDeclarationOnNextLine: false
+ AlignAfterOpenBracket: true
+ SpacesInSquareBrackets: false
+ SpacesInCStyleCastParentheses: false
+ SpaceInEmptyParentheses: false
+ SpaceBeforeParens: ControlStatements
+ SpaceAfterCStyleCast: false
+ SortIncludes: true
+ PenaltyReturnTypeOnItsOwnLine: 10000
+ PenaltyExcessCharacter: 10
+ Language: Cpp
+ ForEachMacros: ['lxc_list_for_each', 'lxc_list_for_each_safe']
+ AllowShortLoopsOnASingleLine: false
+ AllowShortIfStatementsOnASingleLine: false
+ AllowShortFunctionsOnASingleLine: None
+ AllowShortCaseLabelsOnASingleLine: false
+ AllowShortBlocksOnASingleLine: false
+ BasedOnStyle: LLVM
+ TabWidth: 8
+ IndentWidth: 8
+ UseTab: Always
+ BreakBeforeBraces: Linux
+ AllowShortIfStatementsOnASingleLine: false
+ IndentCaseLabels: false
+ EOF
+ ```
+ However, it will not handle all cases correctly. For example, most `struct`
+ initializations will not be correct. In such cases please refer to the coding
+ style here.
-#### Only Use Tabs
+## 2) Only Use Tabs
- LXC uses tabs.
-#### Only use `/* */` Style Comments
+## 3) Only use `/* */` Style Comments
- Any comments that are added must use `/* */`.
-- All comments should start on the same line as the opening `/*`.
+- Single-line comments should start on the same line as the opening `/*`.
- Single-line comments should simply be placed between `/* */`. For example:
- ```
+ ```C
/* Define pivot_root() if missing from the C library */
```
-- Multi-line comments should end with the closing `*/` on a separate line. For
+- Mutli-line comment should start on the next line following the opening
+ `/*`and should end with the closing `*/` on a separate line. For
example:
- ```
- /* At this point the old-root is mounted on top of our new-root
+ ```C
+ /*
+ * At this point the old-root is mounted on top of our new-root
* To unmounted it we must not be chdir()ed into it, so escape back
* to old-root.
*/
```
-#### Try To Wrap At 80chars
+## 4) Try To Wrap At 80chars
- This is not strictly enforced. It is perfectly valid to sometimes
overflow this limit if it helps clarity. Nonetheless, try to stick to it
and use common sense to decide when not to.
-#### Error Messages
+## 5) Error Messages
- Error messages must start with a capital letter and must **not** end with a
punctuation sign.
- They should be descriptive, without being needlessly long. It is best to just
use already existing error messages as examples.
+- The commit message itself is not subject to rule 4), i.e. it should not be
+ wrapped at 80chars. This is to make it easy to grep for it.
- Examples of acceptable error messages are:
- ```
+ ```C
SYSERROR("Failed to create directory \"%s\"", path);
WARN("\"/dev\" directory does not exist. Proceeding without autodev being set up");
```
-#### Return Error Codes
+## 6) Set `errno`
-- When writing a function that can fail in a non-binary way try to return
- meaningful negative error codes (e.g. `return -EINVAL;`).
+- Functions that can fail in a non-binary way should return `-1` and set
+ `errno` to a meaningful error code.
+ As a convenience LXC provides the `minus_one_set_errno` macro:
+ ```C
+ static int set_config_net_l2proxy(const char *key, const char *value,
+ struct lxc_conf *lxc_conf, void *data)
+ {
+ struct lxc_netdev *netdev = data;
+ unsigned int val = 0;
+ int ret;
-#### All Unexported Functions Must Be Declared `static`
+ if (lxc_config_value_empty(value))
+ return clr_config_net_l2proxy(key, lxc_conf, data);
-- Functions which are only used in the current file and are not exported
- within the codebase need to be declared with the `static` attribute.
+ if (!netdev)
+ return minus_one_set_errno(EINVAL);
-#### All Exported Functions Must Be Declared `extern` In A Header File
+ ret = lxc_safe_uint(value, &val);
+ if (ret < 0)
+ return minus_one_set_errno(-ret);
+
+ switch (val) {
+ case 0:
+ netdev->l2proxy = false;
+ return 0;
+ case 1:
+ netdev->l2proxy = true;
+ return 0;
+ }
-- Functions which are used in different files in the library should be declared
- in a suitable `*.c` file and exposed in a suitable `*.h` file. When defining
- the function in the `*.c` file the function signature should not be preceded
- by the `extern` keyword. When declaring the function signature in the `*.h`
- file it must be preceded by the `extern` keyword. For example:
- ```
- /* Valid function definition in a *.c file */
- ssize_t lxc_write_nointr(int fd, const void* buf, size_t count)
- {
- ssize_t ret;
- again:
- ret = write(fd, buf, count);
- if (ret < 0 && errno == EINTR)
- goto again;
- return ret;
+ return minus_one_set_errno(EINVAL);
}
-
- /* Valid function declaration in a *.h file */
- extern ssize_t lxc_write_nointr(int fd, const void* buf, size_t count);
```
-#### All Names Must Be In lower_case
+## 7) All Unexported Functions Must Be Declared `static`
+
+- Functions which are only used in the current file and are not exported
+ within the codebase need to be declared with the `static` attribute.
+
+## 8) All Exported Functions Must Be Declared `extern` In A Header File
-- All functions and variable names must use lower case.
+- Functions declared in header files (`*.h`) should use the `extern` keyword.
+- Functions declared in source files (`*.c`) should not use the `extern` keyword.
-#### Declaring Variables
+## 9) Declaring Variables
- variables should be declared at the top of the function or at the beginning
- of a new scope but **never** in the middle of a scope
-1. uninitialized variables
- - put base types before complex types
- - put standard types defined by libc before types defined by LXC
- - put multiple declarations of the same type on the same line
+ of a new scope but **never** in the middle of a scope. They should be ordered
+ in the following way:
+1. automatically freed variables
+ - This specifically references variables cleaned up via the `cleanup`
+ attribute as supported by `gcc` and `clang`.
2. initialized variables
- - put base types before complex types
- - put standard types defined by libc before types defined by LXC
- - put multiple declarations of the same type on the same line
+3. uninitialized variables
+General rules are:
+- put base types before complex types
+- put standard types defined by libc before types defined by LXC
+- put multiple declarations of the same type on the same line
- Examples of good declarations can be seen in the following function:
- ```
+ ```C
int lxc_clear_procs(struct lxc_conf *c, const char *key)
{
struct lxc_list *it, *next;
}
```
-#### Single-line `if` blocks should not be enclosed in `{}`
-
-- This also affects `if-else` ladders if and only if all constituting
- conditions are
- single-line conditions. If there is at least one non-single-line
- condition `{}` must be used.
-- For example:
- ```
- /* no brackets needed */
- if (size > INT_MAX)
- return -EFBIG;
-
- /* The else branch has more than one-line and so needs {}. This entails that
- * the if branch also needs to have {}.
- */
- if ( errno == EROFS ) {
- WARN("Warning: Read Only file system while creating %s", path);
- } else {
- SYSERROR("Error creating %s", path);
- return -1;
- }
-
- /* also fine */
- for (i = 0; list[i]; i++)
- if (strcmp(list[i], entry) == 0)
- return true;
-
- /* also fine */
- if (ret < 0)
- WARN("Failed to set FD_CLOEXEC flag on slave fd %d of "
- "pty device \"%s\": %s", pty_info->slave,
- pty_info->name, strerror(errno));
-
- /* also fine */
- if (ret == 0)
- for (i = 0; i < sizeof(limit_opt)/sizeof(limit_opt[0]); ++i) {
- if (strcmp(res, limit_opt[i].name) == 0)
- return limit_opt[i].value;
- }
- ```
-
-#### Functions Not Returning Booleans Must Assign Return Value Before Performing Checks
+## 10) Functions Not Returning Booleans Must Assign Return Value Before Performing Checks
- When checking whether a function not returning booleans was successful or not
the returned value must be assigned before it is checked (`str{n}cmp()`
functions being one notable exception). For example:
- ```
+ ```C
/* assign value to "ret" first */
ret = mount(sourcepath, cgpath, "cgroup", remount_flags, NULL);
/* check whether function was successful */
}
```
Functions returning booleans can be checked directly. For example:
- ```
+ ```C
extern bool lxc_string_in_array(const char *needle, const char **haystack);
/* check right away */
continue;
```
-#### Non-Boolean Functions That Behave Like Boolean Functions Must Explicitly Check Against A Value
+## 11) Non-Boolean Functions That Behave Like Boolean Functions Must Explicitly Check Against A Value
- This rule mainly exists for `str{n}cmp()` type functions. In most cases they
are used like a boolean function to check whether a string matches or not.
treated as such since they don't really behave like boolean functions. So
`if (!str{n}cmp())` and `if (str{n}cmp())` checks must not be used. Good
examples are found in the following functions:
- ```
+ ```C
static int set_config_hooks(const char *key, const char *value,
struct lxc_conf *lxc_conf, void *data)
}
```
-#### Do Not Use C99 Variable Length Arrays (VLA)
+## 12) Do Not Use C99 Variable Length Arrays (VLA)
- They are made optional and there is no guarantee that future C standards
will support them.
-#### Use Standard libc Macros When Exiting
+## 13) Use Standard libc Macros When Exiting
- libc provides `EXIT_FAILURE` and `EXIT_SUCCESS`. Use them whenever possible
in the child of `fork()`ed process or when exiting from a `main()` function.
-#### Use `goto`s
+## 14) Use `goto`s
`goto`s are an essential language construct of C and are perfect to perform
cleanup operations or simplify the logic of functions. However, here are the
- **only** jump downwards unless you are handling `EAGAIN` errors and want to
avoid `do-while` constructs.
- An example of a good usage of `goto` is:
- ```
+ ```C
static int set_config_idmaps(const char *key, const char *value,
struct lxc_conf *lxc_conf, void *data)
{
}
```
-#### Use Booleans instead of integers
+## 15) Use Booleans instead of integers
- When something can be conceptualized in a binary way use a boolean not
an integer.
-#### Cleanup Functions Must Handle The Object's Null Type And Being Passed Already Cleaned Up Objects
+## 16) Cleanup Functions Must Handle The Object's Null Type And Being Passed Already Cleaned Up Objects
- If you implement a custom cleanup function to e.g. free a complex type
you declared you must ensure that the object's null type is handled and
treated as a NOOP. For example:
- ```
+ ```C
void lxc_free_array(void **array, lxc_free_fn element_free_fn)
{
void **p;
descriptors and sets the already closed file descriptors to `-EBADF`. On the
next call it can simply check whether the file descriptor is positive and
move on if it isn't:
- ```
+ ```C
static void lxc_put_attach_clone_payload(struct attach_clone_payload *p)
{
if (p->ipc_socket >= 0) {
}
```
-### Cast to `(void)` When Intentionally Ignoring Return Values
+## 17) Cast to `(void)` When Intentionally Ignoring Return Values
- There are cases where you do not care about the return value of a function.
Please cast the return value to `(void)` when doing so.
default do not need to be cast to `(void)`. Classical candidates are
`close()` and `fclose()`.
- A good example is:
- ```
+ ```C
for (i = 0; hierarchies[i]; i++) {
char *fullpath;
char *path = hierarchies[i]->fullcgpath;
}
```
-#### Use `for (;;)` instead of `while (1)` or `while (true)`
+## 18) Use `for (;;)` instead of `while (1)` or `while (true)`
- Let's be honest, it is really the only sensible way to do this.
-#### Use The Set Of Supported DCO Statements
+## 19) Use The Set Of Supported DCO Statements
- Signed-off-by: Random J Developer <random@developer.org>
- You did write this code or have the right to contribute it to LXC.
helped you solve a problem or had a clever idea do not silently claim it by
slapping your Signed-off-by underneath. Be honest and add a Suggested-by.
-#### Commit Message Outline
+## 20) Commit Message Outline
- You **must** stick to the 80chars limit especially in the title of the commit
message.
- Please always use the affected file (without the file type suffix) or module
as a prefix in the commit message.
- Examples of good commit messages are:
- ```
+ ```Diff
commit b87243830e3b5e95fa31a17cf1bfebe55353bf13
Author: Felix Abecassis <fabecassis@nvidia.com>
Date: Fri Feb 2 06:19:13 2018 -0800
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
```
-#### Use `_exit()` To Terminate `fork()`ed Child Processes
+## 21) Use `_exit()` To Terminate `fork()`ed Child Processes
- When `fork()`ing off a child process use `_exit()` to terminate it instead of
`exit()`. The `exit()` function is not thread-safe and thus not suited for
the shared library which must ensure that it is thread-safe.
+
+## 22) Keep Arrays of `struct`s Aligned Horizontally When Initializing
+
+- Arrays of `struct`s are:
+ ```C
+ struct foo_struct {
+ int n;
+ int m;
+ int p;
+ };
+
+ struct foo_struct new_instance[] = {
+ { 1, 2, 3 },
+ { 4, 5, 6 },
+ { 7, 8, 9 },
+ };
+ ```
+- Leave a single space after the opening `{` and before closing `}` of the
+ largest member of the last column.
+- Always leave a single space between the largest member of the current column
+ and the member in the next column.
+- A good example is
+ ```C
+ struct signame {
+ int num;
+ const char *name;
+ };
+
+ static const struct signame signames[] = {
+ { SIGHUP, "HUP" },
+ { SIGINT, "INT" },
+ { SIGQUIT, "QUIT" },
+ { SIGILL, "ILL" },
+ { SIGABRT, "ABRT" },
+ { SIGFPE, "FPE" },
+ { SIGKILL, "KILL" },
+ { SIGSEGV, "SEGV" },
+ { SIGPIPE, "PIPE" },
+ { SIGALRM, "ALRM" },
+ { SIGTERM, "TERM" },
+ { SIGUSR1, "USR1" },
+ { SIGUSR2, "USR2" },
+ { SIGCHLD, "CHLD" },
+ { SIGCONT, "CONT" },
+ { SIGSTOP, "STOP" },
+ { SIGTSTP, "TSTP" },
+ { SIGTTIN, "TTIN" },
+ { SIGTTOU, "TTOU" },
+ #ifdef SIGTRAP
+ { SIGTRAP, "TRAP" },
+ #endif
+ #ifdef SIGIOT
+ { SIGIOT, "IOT" },
+ #endif
+ #ifdef SIGEMT
+ { SIGEMT, "EMT" },
+ #endif
+ #ifdef SIGBUS
+ { SIGBUS, "BUS" },
+ #endif
+ #ifdef SIGSTKFLT
+ { SIGSTKFLT, "STKFLT" },
+ #endif
+ #ifdef SIGCLD
+ { SIGCLD, "CLD" },
+ #endif
+ #ifdef SIGURG
+ { SIGURG, "URG" },
+ #endif
+ #ifdef SIGXCPU
+ { SIGXCPU, "XCPU" },
+ #endif
+ #ifdef SIGXFSZ
+ { SIGXFSZ, "XFSZ" },
+ #endif
+ #ifdef SIGVTALRM
+ { SIGVTALRM, "VTALRM" },
+ #endif
+ #ifdef SIGPROF
+ { SIGPROF, "PROF" },
+ #endif
+ #ifdef SIGWINCH
+ { SIGWINCH, "WINCH" },
+ #endif
+ #ifdef SIGIO
+ { SIGIO, "IO" },
+ #endif
+ #ifdef SIGPOLL
+ { SIGPOLL, "POLL" },
+ #endif
+ #ifdef SIGINFO
+ { SIGINFO, "INFO" },
+ #endif
+ #ifdef SIGLOST
+ { SIGLOST, "LOST" },
+ #endif
+ #ifdef SIGPWR
+ { SIGPWR, "PWR" },
+ #endif
+ #ifdef SIGUNUSED
+ { SIGUNUSED, "UNUSED" },
+ #endif
+ #ifdef SIGSYS
+ { SIGSYS, "SYS" },
+ #endif
+ };
+ ```
+
+## 23) Use `strlcpy()` instead of `strncpy()`
+
+When copying strings always use `strlcpy()` instead of `strncpy()`. The
+advantage of `strlcpy()` is that it will always append a `\0` byte to the
+string.
+
+Unless you have a valid reason to accept truncation you must check whether
+truncation has occurred, treat it as an error, and handle the error
+appropriately.
+
+## 24) Use `strlcat()` instead of `strncat()`
+
+When concatenating strings always use `strlcat()` instead of `strncat()`. The
+advantage of `strlcat()` is that it will always append a `\0` byte to the
+string.
+
+Unless you have a valid reason to accept truncation you must check whether
+truncation has occurred, treat it as an error, and handle the error
+appropriately.
+
+## 25) Use `__fallthrough__` in switch statements
+
+If LXC detects that the compiler is new enough it will tell it to check
+`switch` statements for non-documented fallthroughs. Please always place
+a `__fallthrough__` after a `case` which falls through the next one.
+
+```c
+int lxc_attach_run_command(void *payload)
+{
+ int ret = -1;
+ lxc_attach_command_t *cmd = payload;
+
+ ret = execvp(cmd->program, cmd->argv);
+ if (ret < 0) {
+ switch (errno) {
+ case ENOEXEC:
+ ret = 126;
+ break;
+ case ENOTDIR:
+ __fallthrough;
+ case ENOENT:
+ ret = 127;
+ break;
+ }
+ }
+
+ SYSERROR("Failed to exec \"%s\"", cmd->program);
+ return ret;
+}
+```
+
+## 24) Never use `fgets()`
+
+LXC does not allow the use of `fgets()`. Use `getline()` or other methods
+instead.
+
+## 25) Never allocate memory on the stack
+
+This specifically forbids any usage of `alloca()` in the codebase.
+
+## 26) Use cleanup macros supported by `gcc` and `clang`
+
+LXC has switched from manually cleaning up resources to using cleanup macros
+supported by `gcc` and `clang`:
+```c
+__attribute__((__cleanup__(<my-cleanup-function-wrapper>)))
+```
+We do not allow manually cleanups anymore if there are appropriate macros.
+Currently the following macros are supported:
+```c
+/* close file descriptor */
+__do_close_prot_errno
+
+/* free allocated memory */
+__do_free __attribute__((__cleanup__(__auto_free__)))
+
+/* close FILEs */
+__do_fclose __attribute__((__cleanup__(__auto_fclose__)))
+
+/* close DIRs */
+__do_closedir __attribute__((__cleanup__(__auto_closedir__)))
+```
+For example:
+```c
+void turn_into_dependent_mounts(void)
+{
+ __do_free char *line = NULL;
+ __do_fclose FILE *f = NULL;
+ __do_close int memfd = -EBADF, mntinfo_fd = -EBADF;
+ int ret;
+ ssize_t copied;
+ size_t len = 0;
+
+ mntinfo_fd = open("/proc/self/mountinfo", O_RDONLY | O_CLOEXEC);
+ if (mntinfo_fd < 0) {
+ SYSERROR("Failed to open \"/proc/self/mountinfo\"");
+ return;
+ }
+
+ memfd = memfd_create(".lxc_mountinfo", MFD_CLOEXEC);
+ if (memfd < 0) {
+ char template[] = P_tmpdir "/.lxc_mountinfo_XXXXXX";
+
+ if (errno != ENOSYS) {
+ SYSERROR("Failed to create temporary in-memory file");
+ return;
+ }
+
+ memfd = lxc_make_tmpfile(template, true);
+ if (memfd < 0) {
+ WARN("Failed to create temporary file");
+ return;
+ }
+ }
+
+again:
+ copied = lxc_sendfile_nointr(memfd, mntinfo_fd, NULL, LXC_SENDFILE_MAX);
+ if (copied < 0) {
+ if (errno == EINTR)
+ goto again;
+
+ SYSERROR("Failed to copy \"/proc/self/mountinfo\"");
+ return;
+ }
+
+ ret = lseek(memfd, 0, SEEK_SET);
+ if (ret < 0) {
+ SYSERROR("Failed to reset file descriptor offset");
+ return;
+ }
+
+ f = fdopen(memfd, "re");
+ if (!f) {
+ SYSERROR("Failed to open copy of \"/proc/self/mountinfo\" to mark all shared. Continuing");
+ return;
+ }
+
+ /*
+ * After a successful fdopen() memfd will be closed when calling
+ * fclose(f). Calling close(memfd) afterwards is undefined.
+ */
+ move_fd(memfd);
+
+ while (getline(&line, &len, f) != -1) {
+ char *opts, *target;
+
+ target = get_field(line, 4);
+ if (!target)
+ continue;
+
+ opts = get_field(target, 2);
+ if (!opts)
+ continue;
+
+ null_endofword(opts);
+ if (!strstr(opts, "shared"))
+ continue;
+
+ null_endofword(target);
+ ret = mount(NULL, target, NULL, MS_SLAVE, NULL);
+ if (ret < 0) {
+ SYSERROR("Failed to recursively turn old root mount tree into dependent mount. Continuing...");
+ continue;
+ }
+ TRACE("Recursively turned old root mount tree into dependent mount");
+ }
+ TRACE("Turned all mount table entries into dependent mount");
+}
+```
projects / mirror_lxc.git / blobdiff