Re: [PATCH v2] docs: document all error message types in checkpatch

On Sat, Mar 20, 2021 at 8:14 PM Lukas Bulwahn [off-list ref] wrote:

On Thu, Mar 18, 2021 at 8:03 PM Dwaipayan Ray [off-list ref] wrote:

quoted

All the error message types now have a verbose description.

Also there are two new groups of message types:

- Macros, Attributes and Symbols
- Functions and Variables

Rearrange the message types to fit these new groups as needed.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---

Changes in v2:
- Replace 4.10 kernel doc links by latest

All good, see some further comments below.

I am generally wondering if having kernel.org Links is right, though.

Can you not use "doc:" or "ref:" in the documentation, so that sphinx
would check those references and create those links automatically?

E.g., in a few releases, documentation might be reorganized and by
referring to latest, all documentation of previous releases would then
contain broken links because pages were reorganized in latest.

Actually we used ref: links in the initial version. But that posed a problem.
Since this was being parsed by checkpatch, an user would see output
in the form:
ref: `Documentation/process/coding-style.rst`

And the current version would show:

See: https://www.kernel.org/doc/html/latest/process/coding-style.html

This is more intuitive for the person who runs checkpatch in verbose
mode. But at the same time we have to take the risk that if at some point
links are reorganized, we would have to change them too. I think that is
a fair tradeoff, unless we find something better.

quoted

+  **LOCKDEP**
+    lockdep_no_validate class is reserved for device->mutex.

I do not understand this one.

I too didn't find a mention of it anywhere other than checkpatch.
On the list what I got was this patch:
https://lore.kernel.org/lkml/1268959062.9440.467.camel@laptop/ (local)

They added the novalidate class for lockdep conversions in device->mutex.
I can add this perhaps.

quoted

   **LINE_SPACING**
     Vertical space is wasted given the limited number of lines an
     editor window can display when multiple blank lines are used.
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces

+  **OPEN_BRACE**
+    The opening brace should be following the function definitions on the
+    next line.  For any non functional block it should be on the same line

s/non functional/non-functional/

But is non-functional block really a good wording?

I am not sure of a particular replacement. We want to mention every block except
a function. They have used the same wording here:
https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces

Do you have any suggestions?

quoted

+  **CORRUPTED_PATCH**
+    The patch seems to be corrupted or lines are wrapped.
+    Please regenerate the patch file before sending to the maintainer.
+

s/sending to/sending it to/

will correct this. thanks.

quoted

+  **DOS_LINE_ENDINGS**
+    For DOS-formatted patches, there are extra ^M symbols at the end of
+    the line.  These should be removed.
+
+  **EXECUTE_PERMISSIONS**
+    There is no reason for source files to be executable.  The executable
+    bit can be removed safely.
+
+  **NON_OCTAL_PERMISSIONS**
+    Permission bits should use 4 digit octal permissions (like 0700 or 0444).
+    Avoid using any other base like decimal.
+
+  **NOT_UNIFIED_DIFF**
+    The patchfile does not appear to be in unified-diff format.  Please
+    regenerate the patch before sending to the maintainer.
+

s/patch file/patch

quoted

+  **PRINTF_0XDECIMAL**
+    Prefixing Ox with decimal output is defective and should be corrected.
+

s/O/0/ ?

oh sure my bad. They look almost similar

quoted

+  **TRAILING_STATEMENTS**
+    Trailing statements (for example after any conditional) should be
+    on the next line.
+    Like::
+
+      if (x == y) break;
+
+    should be::
+
+      if (x == y)
+              break;
--
2.30.0

Thanks!
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees