The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2165
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From b706b2eee7be2cabab1499d0cbb6e0a0a7ff37a1 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Wed, 14 Feb 2018 13:04:48 +0100 Subject: [PATCH 1/2] CONTRIBUTING: update Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- CONTRIBUTING | 44 +++++++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/CONTRIBUTING b/CONTRIBUTING index d3c343c11..16e2b7272 100644 --- a/CONTRIBUTING +++ b/CONTRIBUTING @@ -5,8 +5,7 @@ This project accepts contributions. In order to contribute, you should pay attention to a few things: 1 - your code must follow the coding style rules - 2 - the format of the submission must be email patches or github - pull requests + 2 - the format of the submission must Github pull requests 3 - your work must be signed @@ -20,25 +19,22 @@ the directory 'Documentation' of the Linux kernel source tree. It can be accessed online too: -http://lxr.linux.no/linux+v2.6.27/Documentation/CodingStyle +https://www.kernel.org/doc/html/v4.10/process/coding-style.html Submitting Modifications: ------------------------- -The contributions should be email patches or github pull requests. -The guidelines are the same as the patch submission for the Linux kernel -except for the DCO which is defined below. The guidelines are defined in the -'SubmittingPatches' file, available in the directory 'Documentation' -of the Linux kernel source tree. - -It can be accessed online too: - -https://www.kernel.org/doc/Documentation/SubmittingPatches - -You can submit your patches to the lxc-devel@lists.linuxcontainers.org mailing -list. Use http://lists.linuxcontainers.org/listinfo/lxc-devel to subscribe -to the list. - +The contributions must be Github pull requests. +It is also possible to send contributions as email patches. But please be aware +that the review process might take significantly longer than in the case of +Github pull requests. You can submit your email patches to the +lxc-devel@lists.linuxcontainers.org mailing list. (Use +http://lists.linuxcontainers.org/listinfo/lxc-devel to subscribe to the list.) +The guidelines for submitting email patches are the same as the patch submission +for the Linux kernel except for the DCO which is defined below. The guidelines +are defined in the 'SubmittingPatches' file, available in the directory +'Documentation' of the Linux kernel source tree: +https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html Licensing for new files: ------------------------ @@ -56,13 +52,11 @@ Anything else (non-libaries) needs to be Free Software and needs to be allowed to link with LGPLv2.1+ code (if needed). LXC upstream prefers LGPLv2.1+ or GPLv2 for those. - When introducing a new file into the project, please make sure it has a copyright header making clear under which license it's being released and if it doesn't match the criteria described above, please explain your decision on the lxc-devel mailing-list when submitting your patch. - Developer Certificate of Origin: -------------------------------- @@ -111,3 +105,15 @@ You can do it by using option -s or --signoff when you commit git commit --signoff ... using your real name (sorry, no pseudonyms or anonymous contributions.) + +In addition we support the following DCOs which maintainers can use to indicate +that a patch is acceptable: + + Acked-by: Random J Developer <ran...@developer.org> + Reviewed-by: Random J Developer <ran...@developer.org> + +If you are contributing as a group who is implementing a feature together such +that it cannot be reasonably attributed to a single developer please use: + + Co-developed-by: Random J Developer 1 <rando...@developer.org> + Co-developed-by: Random J Developer 2 <rando...@developer.org> From 6a6dc702d045ae0f5fa4d8272e4f143e7b202d7a Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Wed, 14 Feb 2018 13:04:59 +0100 Subject: [PATCH 2/2] CODING_STYLE: add Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- CODING_STYLE.md | 335 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 335 insertions(+) create mode 100644 CODING_STYLE.md diff --git a/CODING_STYLE.md b/CODING_STYLE.md new file mode 100644 index 000000000..b19eeb5bd --- /dev/null +++ b/CODING_STYLE.md @@ -0,0 +1,335 @@ +#### 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. +- Just because there is still code in LXC that doesn't adhere to the coding + standards outlined here does not license not adhering to the coding style. In + other words: please stick to the coding style. + +#### Only Use Tabs + +- LXC uses tabs. + +#### Only use /* Style Comments */ + +- Any comments that are added must use /* */. + +#### Try To Stick To Wrap At 80chars + +- This is not strictly enforced. It is perfectly legal 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 + +- 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. +- Examples of acceptable error messages are: + ``` + SYSERROR("Failed to create directory \"%s\"", path); + WARN("\"/dev\" directory does not exist. Proceeding without autodev being set up"); + ``` + +#### Return Error Codes + +- When writing a function that can fail in a non-binary way try to return + meaningful negative error codes (e.g. `return -EINVAL;`). + +#### 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. + +#### All Names Must Be In lower_case + +- All functions and variable names must use lower case. + +#### 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 +- separate initialized from uninitialized variable declarations +- put multiple declarations of the same type on the same line +- standard types defined by libc should preceed types defined by LXC +- *optional but recommended*: base types should preceed complex types +- Examples of good declarations can be seen in the following function: + ``` + int lxc_clear_procs(struct lxc_conf *c, const char *key) + { + struct lxc_list *it, *next; + bool all = false; + const char *k = NULL; + + if (strcmp(key, "lxc.proc") == 0) + all = true; + else if (strncmp(key, "lxc.proc.", sizeof("lxc.proc.") - 1) == 0) + k = key + sizeof("lxc.proc.") - 1; + else + return -1; + + lxc_list_for_each_safe(it, &c->procs, next) { + struct lxc_proc *proc = it->elem; + + if (!all && strcmp(proc->filename, k) != 0) + continue; + lxc_list_del(it); + free(proc->filename); + free(proc->value); + free(proc); + free(it); + } + + return 0; + } + ``` + +#### Single-line `if` blocks should not be enclosed in `{}` + +- This also affects `if-else` ladders iff all constituting conditions are + single-line conditions. If there is at least one non-single-line + condition `{}` must be used. + +#### 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 + +- 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 + +`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 +rules to use them: +- use descriptive `goto` labels. + For example, if you know that this label is only used as an error path you + should use something like `on_error` instead of `out` as label name. +- **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: + ``` + static int set_config_idmaps(const char *key, const char *value, + struct lxc_conf *lxc_conf, void *data) + { + unsigned long hostid, nsid, range; + char type; + int ret; + struct lxc_list *idmaplist = NULL; + struct id_map *idmap = NULL; + + if (lxc_config_value_empty(value)) + return lxc_clear_idmaps(lxc_conf); + + idmaplist = malloc(sizeof(*idmaplist)); + if (!idmaplist) + goto on_error; + + idmap = malloc(sizeof(*idmap)); + if (!idmap) + goto on_error; + memset(idmap, 0, sizeof(*idmap)); + + ret = parse_idmaps(value, &type, &nsid, &hostid, &range); + if (ret < 0) { + ERROR("Failed to parse id mappings"); + goto on_error; + } + + INFO("Read uid map: type %c nsid %lu hostid %lu range %lu", type, nsid, hostid, range); + if (type == 'u') + idmap->idtype = ID_TYPE_UID; + else if (type == 'g') + idmap->idtype = ID_TYPE_GID; + else + goto on_error; + + idmap->hostid = hostid; + idmap->nsid = nsid; + idmap->range = range; + idmaplist->elem = idmap; + lxc_list_add_tail(&lxc_conf->id_map, idmaplist); + + if (!lxc_conf->root_nsuid_map && idmap->idtype == ID_TYPE_UID) + if (idmap->nsid == 0) + lxc_conf->root_nsuid_map = idmap; + + + if (!lxc_conf->root_nsgid_map && idmap->idtype == ID_TYPE_GID) + if (idmap->nsid == 0) + lxc_conf->root_nsgid_map = idmap; + + idmap = NULL; + + return 0; + + on_error: + free(idmaplist); + free(idmap); + + return -1; + } + ``` + +#### 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 + +- 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. +- A good example would be: + ``` + static void lxc_put_attach_clone_payload(struct attach_clone_payload *p) + { + if (p->ipc_socket >= 0) { + shutdown(p->ipc_socket, SHUT_RDWR); + close(p->ipc_socket); + p->ipc_socket = -EBADF; + } + + if (p->pty_fd >= 0) { + close(p->pty_fd); + p->pty_fd = -EBADF; + } + + if (p->init_ctx) { + lxc_proc_put_context_info(p->init_ctx); + p->init_ctx = NULL; + } + } + ``` + +### 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. +- Standard library functions or functions which are known to be ignored by + default do not need to be cast to `(void)`. Classical candidates are + `close()` and `fclose()`. +- A good example is: + ``` + for (i = 0; hierarchies[i]; i++) { + char *fullpath; + char *path = hierarchies[i]->fullcgpath; + + ret = chowmod(path, destuid, nsgid, 0755); + if (ret < 0) + return -1; + + /* failures to chown() these are inconvenient but not + * detrimental we leave these owned by the container launcher, + * so that container root can write to the files to attach. we + * chmod() them 664 so that container systemd can write to the + * files (which systemd in wily insists on doing). + */ + + if (hierarchies[i]->version == cgroup_super_magic) { + fullpath = must_make_path(path, "tasks", null); + (void)chowmod(fullpath, destuid, nsgid, 0664); + free(fullpath); + } + + fullpath = must_make_path(path, "cgroup.procs", null); + (void)chowmod(fullpath, destuid, 0, 0664); + free(fullpath); + + if (hierarchies[i]->version != cgroup2_super_magic) + continue; + + fullpath = must_make_path(path, "cgroup.subtree_control", null); + (void)chowmod(fullpath, destuid, nsgid, 0664); + free(fullpath); + + fullpath = must_make_path(path, "cgroup.threads", null); + (void)chowmod(fullpath, destuid, nsgid, 0664); + free(fullpath); + } + ``` + +#### Use `_exit()` in `fork()`ed Processes + +- This has multiple reasons but the gist is: + - `exit()` is not thread-safe + - `exit()` in libc runs exit handlers which might interfer with the parents + state + +#### 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 + +- Signed-off-by: Random J Developer <ran...@developer.org> + - You did write this code or have the right to contribute it to LXC. +- Acked-by: Random J Developer <ran...@developer.org> + - You did read the code and think it is correct. This is usually only used by + maintainers or developers that have made significant contributions and can + vouch for the correctness of someone else's code. +- Reviewed-by: Random J Developer <ran...@developer.org> + - You did review the code and vouch for its correctness, i.e. you'd be + prepared to fix bugs it might cause. This is usually only used by + maintainers or developers that have made significant contributions and can + vouch for the correctness of someone else's code. +- Co-developed-by: Random J Developer <ran...@developer.org> + - The code can not be reasonably attributed to a single developer, i.e. + you worked on this together. +- Tested-by: Random J Developer <ran...@developer.org> + - You verified that the code fixes a given bug or is behaving as advertised. +- Reported-by: Random J Developer <ran...@developer.org> + - You found and reported the bug. +- Suggested-by: Random J Developer <ran...@developer.org> + - You wrote the code but someone contributed the idea. This line is usually + overlooked but it is a sign of good etiquette and coding ethics: if someone + 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 + +- You **must** stick to the 80chars limit especially in the title of the commit + message. +- Please use English commit messages only. +- use meaningful commit messages. +- Use correct spelling and grammar. + If you are not a native speaker and/or feel yourself struggling with this it + is perfectly fine to point this out and there's no need to apologize. Usually + developers will be happy to pull your branch and adopt 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: + ``` + commit b87243830e3b5e95fa31a17cf1bfebe55353bf13 + Author: Felix Abecassis <fabecas...@nvidia.com> + Date: Fri Feb 2 06:19:13 2018 -0800 + + hooks: change the semantic of NVIDIA_VISIBLE_DEVICES="" + + With LXC, you can override the value of an environment variable to + null, but you can't unset an existing variable. + + The NVIDIA hook was previously activated when NVIDIA_VISIBLE_DEVICES + was set to null. As a result, it was not possible to disable the hook + by overriding the environment variable in the configuration. + + The hook can now be disabled by setting NVIDIA_VISIBLE_DEVICES to + null or to the new special value "void". + + Signed-off-by: Felix Abecassis <fabecas...@nvidia.com> + + + commit d6337a5f9dc7311af168aa3d586fdf239f5a10d3 + Author: Christian Brauner <christian.brau...@ubuntu.com> + Date: Wed Jan 31 16:25:11 2018 +0100 + + cgroups: get controllers on the unified hierarchy + + Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> + + ```
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel