[lxc-devel] [lxc/master] Clean up contributing and coding stlye docs

[tcharding on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 1088 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180817/16d275a7/attachment.bin>
-------------- next part --------------
From 82778d4134d22c4ee1cb5de04152e46b1b6ecebe Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 14:16:47 +1000
Subject: [PATCH 1/9] CONTRIBUTING: Update reference to kernel coding style

Kernel coding style guide filename is stale, this file has been renamed
in the kernel tree.  While this file still exists we should use the new
filename.

Update reference to kernel coding style guide to use the new file name.

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CONTRIBUTING | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index 4e8aa389e..227641346 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -14,8 +14,9 @@ Coding Style:
 
 The coding style follows the Linux kernel coding style.
 
-The coding style is defined in the 'CodingStyle' file, available in
-the directory 'Documentation' of the Linux kernel source tree.
+The Linux kernel coding style guide can be found within the kernel tree:
+
+	Documentation/process/coding-style.rst
 
 It can be accessed online too:
 

From b6fbcbbb81558b75ccb22f75209ef36c190a9595 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 14:29:15 +1000
Subject: [PATCH 2/9] CONTRIBUTING: Link to latest online kernel docs

Currently we link to a URL for v4.10 of the kernel docs.  Since we
already mention the kernel tree we should link to the _latest_ kernel
docs online instead of a fixed past version.

Link to latest online kernel docs tracking the mainline instead of past
fixed version.

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CONTRIBUTING | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index 227641346..b1a710a19 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -20,7 +20,7 @@ The Linux kernel coding style guide can be found within the kernel tree:
 
 It can be accessed online too:
 
-https://www.kernel.org/doc/html/v4.10/process/coding-style.html
+https://www.kernel.org/doc/html/latest/process/coding-style.html
 
 Submitting Modifications:
 -------------------------

From 54df5d72edcbed115273c6ce3e8f963d9386f248 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 09:19:32 +1000
Subject: [PATCH 3/9] CONTRIBUTING: Direct readers to CODING_STYLE.md

Currently the 'Coding Style' section mentions only the kernel coding
style.  We have additions on top on this outlined in CODING_STYLE.md.
We should direct readers to this document as well as the kernel docs.

Direct readers to CODING_STLYE.md in the 'Coding Style' section.

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CONTRIBUTING | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index b1a710a19..a3479bfda 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -12,7 +12,8 @@ pay attention to a few things:
 Coding Style:
 -------------
 
-The coding style follows the Linux kernel coding style.
+The LXC project generally follows the Linux kernel coding style.  However there
+are a few differences, these are outlined it CODING_STLYE.md
 
 The Linux kernel coding style guide can be found within the kernel tree:
 

From 183d7fa5ce9135be07e057ac6b000fec536285f9 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 13:46:16 +1000
Subject: [PATCH 4/9] CODING_STYLE: Mention kernel style in introduction

Currently the coding style guide does not mention that we use kernel
coding style as a base style for LXC.  We have just linked to
CODING_STLYE.md from CONTRIBUTING (which mentions use of kernel coding
style).  We can increase documentation congruence and completeness by
mentioning kernel coding style guide in the introduction to our style
guide.

Add heading and introduction to coding style guide informing readers
that we follow kernel coding style as a base before explaining our style
additions.

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CODING_STYLE.md | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/CODING_STYLE.md b/CODING_STYLE.md
index 7f2df9ed2..c81015173 100644
--- a/CODING_STYLE.md
+++ b/CODING_STYLE.md
@@ -1,3 +1,17 @@
+LXC Coding Style Guide
+======================
+
+In general the LXC project follows the Linux kernel coding style.  There are
+however are a few differences, these 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
+
 #### General Notes
 
 - The coding style guide refers to new code. But legacy code can be cleaned up

From cd15ab7a04e03c9af0c07b9d8cf0942d1ab1932a Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 09:22:40 +1000
Subject: [PATCH 5/9] CONTRIBUTING: Add 'be' to fix grammar

Fix minor grammatical issue.

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CONTRIBUTING | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CONTRIBUTING b/CONTRIBUTING
index a3479bfda..55cad4b4d 100644
--- a/CONTRIBUTING
+++ b/CONTRIBUTING
@@ -5,7 +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 Github pull requests
+    2 - the format of the submission must be Github pull requests
     3 - your work must be signed
 
 

From 39a1e1fa19914f6ae7d595c5fb8fd7eb636dcbdc Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 09:38:48 +1000
Subject: [PATCH 6/9] CODING_STLYE: Simplify explanation for use of 'extern'

Current explanation of rules around usage of 'extern' are overly
verbose.  It is not necessary to state that functions should be declared
in header files, the compiler already enforces this.  These rules are
simple, they are better described with simple statements.  An example is
not necessary for such simple rules and serves only to make the document
longer.

Use two simple statements describing the rules that govern function
declaration and the usage of the 'extern' keyword.

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CODING_STYLE.md | 22 ++--------------------
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/CODING_STYLE.md b/CODING_STYLE.md
index c81015173..7e1cbf326 100644
--- a/CODING_STYLE.md
+++ b/CODING_STYLE.md
@@ -127,26 +127,8 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
 
 #### All Exported Functions Must Be Declared `extern` In A Header File
 
-- 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:
-  ```C
-  /* 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;
-  }
-
-  /* Valid function declaration in a *.h file */
-  extern ssize_t lxc_write_nointr(int fd, const void* buf, size_t count);
-  ```
+- Functions declared in header files (`*.h`) should use the `extern` keyword.
+- Functions declared in source files (`*.c`) should not use the `extern` keyword.
 
 #### All Names Must Be In lower_case
 

From 524440e920df1fc0b1839178fc391c1bf7f90b3c Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 13:55:47 +1000
Subject: [PATCH 7/9] CODING_STLYE: Remove sections implied by 'kernel style'

We explicitly state that LXC uses coding style based on Linux kernel
style.  It is redundant to then document obvious, and well known, kernel
style rules.  Identifier names certainly fall into this category as does
usage of braces.

Remove sections implied by 'kernel style'.  Naming conventions and brace
placement conventions.

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CODING_STYLE.md | 45 ---------------------------------------------
 1 file changed, 45 deletions(-)

diff --git a/CODING_STYLE.md b/CODING_STYLE.md
index 7e1cbf326..b4c77d0c8 100644
--- a/CODING_STYLE.md
+++ b/CODING_STYLE.md
@@ -130,10 +130,6 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
 - Functions declared in header files (`*.h`) should use the `extern` keyword.
 - Functions declared in source files (`*.c`) should not use the `extern` keyword.
 
-#### 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
@@ -177,47 +173,6 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
   }
     ```
 
-#### 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:
-  ```C
-  /* 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
 
 - When checking whether a function not returning booleans was successful or not

From efcb7f361beefbdb1c24ad86c75c2ed468f29c90 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 14:07:48 +1000
Subject: [PATCH 8/9] CODING_STYLE: Fix non-uniform heading level

Heading uses only 3 level header (###) but the rest of the file uses
four (####).  We should be uniform.

Use uniform heading level in line with the rest of the file.

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CODING_STYLE.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CODING_STYLE.md b/CODING_STYLE.md
index b4c77d0c8..4fef234ed 100644
--- a/CODING_STYLE.md
+++ b/CODING_STYLE.md
@@ -382,7 +382,7 @@ rules to use them:
   }
   ```
 
-### Cast to `(void)` When Intentionally Ignoring Return Values
+#### 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.

From 7419c83f441b61061f3056f4453b8ddefccef764 Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me at tobin.cc>
Date: Fri, 17 Aug 2018 14:40:45 +1000
Subject: [PATCH 9/9] CODING_STYLE: Update section header format

Currently for section headings we use fourth level markdown heading
level (####).  We do not have levels two or three.

We can use standard incremental levels for heading adornments i.e

1) =========
2) ##
3) ###
ect.

Since this document is likely referenced by maintainers when guiding new
contributors it can save maintainer time to be able to quickly reference
a section in the coding stlye guide.  If we add numbers to each heading
(like the kernel stlye guide) then maintainers can say:

   Nice patch, please see section 3 of the coding style guide and ...

So, this patch makes two changes

- Use incremental level heading adornments
- Add a number to each section heading

Signed-off-by: Tobin C. Harding <me at tobin.cc>
---
 CODING_STYLE.md | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/CODING_STYLE.md b/CODING_STYLE.md
index 4fef234ed..80d4096d8 100644
--- a/CODING_STYLE.md
+++ b/CODING_STYLE.md
@@ -12,7 +12,7 @@ It can be accessed online too:
 
 https://www.kernel.org/doc/html/latest/process/coding-style.html
 
-#### General Notes
+## 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.
@@ -76,11 +76,11 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
   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 `/*`.
@@ -97,13 +97,13 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
    */
   ```
 
-#### 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.
@@ -115,22 +115,22 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
   WARN("\"/dev\" directory does not exist. Proceeding without autodev being set up");
   ```
 
-#### Return Error Codes
+## 6) 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`
+## 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.
 
-#### All Exported Functions Must Be Declared `extern` In A Header File
+## 8) All Exported Functions Must Be Declared `extern` In A Header File
 
 - 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
@@ -173,7 +173,7 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
   }
     ```
 
-#### 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()`
@@ -197,7 +197,7 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
           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.
@@ -251,17 +251,17 @@ https://www.kernel.org/doc/html/latest/process/coding-style.html
   }
   ```
 
-#### 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
@@ -335,12 +335,12 @@ rules to use them:
   }
   ```
 
-#### 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
@@ -382,7 +382,7 @@ rules to use them:
   }
   ```
 
-#### 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.
@@ -429,11 +429,11 @@ rules to use them:
   }
   ```
 
-#### 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 at developer.org>
   - You did write this code or have the right to contribute it to LXC.
@@ -459,7 +459,7 @@ rules to use them:
     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.
@@ -501,13 +501,13 @@ rules to use them:
       Signed-off-by: Christian Brauner <christian.brauner at 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.
 
-#### Keep Arrays of `struct`s Aligned Horizontally When Initializing 
+## 22) Keep Arrays of `struct`s Aligned Horizontally When Initializing
 
 - Arrays of `struct`s are:
   ```C
@@ -614,7 +614,7 @@ rules to use them:
   };
   ```
 
-#### Use `strlcpy()` instead of `strncpy()`
+## 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
@@ -624,7 +624,7 @@ 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.
 
-#### Use `strlcat()` instead of `strncat()`
+## 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