[lxc-devel] [lxc/master] CODING_STYLE: add coding style file
brauner on Github
lxc-bot at linuxcontainers.org
Wed Feb 14 13:06:03 UTC 2018
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180214/b9e1e026/attachment.bin>
-------------- next part --------------
From b706b2eee7be2cabab1499d0cbb6e0a0a7ff37a1 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 14 Feb 2018 13:04:48 +0100
Subject: [PATCH 1/2] CONTRIBUTING: update
Signed-off-by: Christian Brauner <christian.brauner at 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 at 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 at 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 <random at developer.org>
+ Reviewed-by: Random J Developer <random at 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 <random_1 at developer.org>
+ Co-developed-by: Random J Developer 2 <random_1 at developer.org>
From 6a6dc702d045ae0f5fa4d8272e4f143e7b202d7a Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Wed, 14 Feb 2018 13:04:59 +0100
Subject: [PATCH 2/2] CODING_STYLE: add
Signed-off-by: Christian Brauner <christian.brauner at 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 <random at developer.org>
+ - You did write this code or have the right to contribute it to LXC.
+- Acked-by: Random J Developer <random at 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 <random at 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 <random at 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 <random at developer.org>
+ - You verified that the code fixes a given bug or is behaving as advertised.
+- Reported-by: Random J Developer <random at developer.org>
+ - You found and reported the bug.
+- Suggested-by: Random J Developer <random at 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 <fabecassis at 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 <fabecassis at nvidia.com>
+
+
+ commit d6337a5f9dc7311af168aa3d586fdf239f5a10d3
+ Author: Christian Brauner <christian.brauner at ubuntu.com>
+ Date: Wed Jan 31 16:25:11 2018 +0100
+
+ cgroups: get controllers on the unified hierarchy
+
+ Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
+
+ ```
More information about the lxc-devel
mailing list