All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] docs: Add more message type documentations for checkpatch
@ 2021-05-15 13:23 Dwaipayan Ray
  2021-05-17 10:04 ` Lukas Bulwahn
  2021-05-20 20:13 ` Jonathan Corbet
  0 siblings, 2 replies; 5+ messages in thread
From: Dwaipayan Ray @ 2021-05-15 13:23 UTC (permalink / raw)
  To: corbet, mchehab; +Cc: linux-kernel, lukas.bulwahn, linux-doc, Dwaipayan Ray

- Document a couple of more checkpatch message types.
- Add a blank line before all `See:` lines to improve the
  rst output.
- Create a new subsection `Permissions` and move a few types
  to it.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 170 ++++++++++++++++++++++++-
 1 file changed, 163 insertions(+), 7 deletions(-)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index 51fed1bd72ec..e409f27f48b6 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -246,6 +246,7 @@ Allocation style
     The first argument for kcalloc or kmalloc_array should be the
     number of elements.  sizeof() as the first argument is generally
     wrong.
+
     See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html
 
   **ALLOC_SIZEOF_STRUCT**
@@ -264,6 +265,7 @@ Allocation style
   **ALLOC_WITH_MULTIPLY**
     Prefer kmalloc_array/kcalloc over kmalloc/kzalloc with a
     sizeof multiply.
+
     See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html
 
 
@@ -284,6 +286,7 @@ API usage
     BUG() or BUG_ON() should be avoided totally.
     Use WARN() and WARN_ON() instead, and handle the "impossible"
     error condition as gracefully as possible.
+
     See: https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on
 
   **CONSIDER_KSTRTO**
@@ -292,12 +295,23 @@ API usage
     may lead to unexpected results in callers.  The respective kstrtol(),
     kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the
     correct replacements.
+
     See: https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull
 
+  **IN_ATOMIC**
+    in_atomic() is not for driver use so any such use is reported as an ERROR.
+    Also in_atomic() is often used to determine if we may sleep, but it is not
+    reliable in this use model therefore its use is strongly discouraged.
+
+    However, in_atomic() is ok for core kernel use.
+
+    See: https://lore.kernel.org/lkml/20080320201723.b87b3732.akpm@linux-foundation.org/
+
   **LOCKDEP**
     The lockdep_no_validate class was added as a temporary measure to
     prevent warnings on conversion of device->sem to device->mutex.
     It should not be used for any other purpose.
+
     See: https://lore.kernel.org/lkml/1268959062.9440.467.camel@laptop/
 
   **MALFORMED_INCLUDE**
@@ -308,11 +322,18 @@ API usage
   **USE_LOCKDEP**
     lockdep_assert_held() annotations should be preferred over
     assertions based on spin_is_locked()
+
     See: https://www.kernel.org/doc/html/latest/locking/lockdep-design.html#annotations
 
   **UAPI_INCLUDE**
     No #include statements in include/uapi should use a uapi/ path.
 
+  **USLEEP_RANGE**
+    usleep_range() should be preferred over udelay(). The proper way of
+    using usleep_range() is mentioned in the kernel docs.
+
+    See: https://www.kernel.org/doc/html/latest/timers/timers-howto.html#delays-information-on-the-various-kernel-delay-sleep-mechanisms
+
 
 Comment style
 -------------
@@ -338,6 +359,7 @@ Comment style
   **C99_COMMENTS**
     C99 style single line comments (//) should not be used.
     Prefer the block comment style instead.
+
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting
 
 
@@ -347,6 +369,7 @@ Commit message
   **BAD_SIGN_OFF**
     The signed-off-by line does not fall in line with the standards
     specified by the community.
+
     See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#developer-s-certificate-of-origin-1-1
 
   **BAD_STABLE_ADDRESS_STYLE**
@@ -368,12 +391,26 @@ Commit message
   **COMMIT_MESSAGE**
     The patch is missing a commit description.  A brief
     description of the changes made by the patch should be added.
+
     See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
 
+  **FROM_SIGN_OFF_MISMATCH**
+    The author's email does not match with that in the Signed-off-by:
+    line(s). This can be sometimes caused due to an improperly configured
+    email client.
+
+    This message is emitted due to any of the following reasons::
+
+      - The email names do not match.
+      - The email addresses do not match.
+      - The email subaddresses do not match.
+      - The email comments do not match.
+
   **MISSING_SIGN_OFF**
     The patch is missing a Signed-off-by line.  A signed-off-by
     line should be added according to Developer's certificate of
     Origin.
+
     See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
 
   **NO_AUTHOR_SIGN_OFF**
@@ -382,6 +419,7 @@ Commit message
     end of explanation of the patch to denote that the author has
     written it or otherwise has the rights to pass it on as an open
     source patch.
+
     See: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin
 
   **DIFF_IN_COMMIT_MSG**
@@ -389,6 +427,7 @@ Commit message
     This causes problems when one tries to apply a file containing both
     the changelog and the diff because patch(1) tries to apply the diff
     which it found in the changelog.
+
     See: https://lore.kernel.org/lkml/20150611134006.9df79a893e3636019ad2759e@linux-foundation.org/
 
   **GERRIT_CHANGE_ID**
@@ -431,6 +470,7 @@ Comparison style
   **BOOL_COMPARISON**
     Comparisons of A to true and false are better written
     as A and !A.
+
     See: https://lore.kernel.org/lkml/1365563834.27174.12.camel@joe-AO722/
 
   **COMPARISON_TO_NULL**
@@ -492,6 +532,7 @@ Macros, Attributes and Symbols
     The kernel does *not* use the ``__DATE__`` and ``__TIME__`` macros,
     and enables warnings if they are used as they can lead to
     non-deterministic builds.
+
     See: https://www.kernel.org/doc/html/latest/kbuild/reproducible-builds.html#timestamps
 
   **DEFINE_ARCH_HAS**
@@ -502,6 +543,7 @@ Macros, Attributes and Symbols
     want architectures able to override them with optimized ones, we
     should either use weak functions (appropriate for some cases), or
     the symbol that protects them should be the same symbol we use.
+
     See: https://lore.kernel.org/lkml/CA+55aFycQ9XJvEOsiM3txHL5bjUc8CeKWJNR_H+MiicaddB42Q@mail.gmail.com/
 
   **INIT_ATTRIBUTE**
@@ -528,6 +570,20 @@ Macros, Attributes and Symbols
               ...
       }
 
+  **MISPLACED_INIT**
+    It is possible to use section markers on variables in a way
+    which gcc doesn't understand (or at least not the way the
+    developer intended)::
+
+      static struct __initdata samsung_pll_clock exynos4_plls[nr_plls] = {
+
+    does not put exynos4_plls in the .initdata section. The __initdata
+    marker can be virtually anywhere on the line, except right after
+    "struct". The preferred location is before the "=" sign if there is
+    one, or before the trailing ";" otherwise.
+
+    See: https://lore.kernel.org/lkml/1377655732.3619.19.camel@joe-AO722/
+
   **MULTISTATEMENT_MACRO_USE_DO_WHILE**
     Macros with multiple statements should be enclosed in a
     do - while block.  Same should also be the case for macros
@@ -541,6 +597,10 @@ Macros, Attributes and Symbols
 
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#macros-enums-and-rtl
 
+  **PREFER_FALLTHROUGH**
+    Use the `fallthrough;` pseudo keyword instead of
+    `/* fallthrough */` like comments.
+
   **WEAK_DECLARATION**
     Using weak declarations like __attribute__((weak)) or __weak
     can have unintended link defects.  Avoid using them.
@@ -551,6 +611,7 @@ Functions and Variables
 
   **CAMELCASE**
     Avoid CamelCase Identifiers.
+
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#naming
 
   **FUNCTION_WITHOUT_ARGS**
@@ -583,6 +644,27 @@ Functions and Variables
       return bar;
 
 
+Permissions
+-----------
+
+  **EXECUTE_PERMISSIONS**
+    There is no reason for source files to be executable.  The executable
+    bit can be removed safely.
+
+  **EXPORTED_WORLD_WRITABLE**
+    Exporting world writable sysfs/debugfs files is usually a bad thing.
+    When done arbitrarily they can introduce serious security bugs.
+    In the past, some of the debugfs vulnerabilities would seemingly allow
+    any local user to write arbitrary values into device registers - a
+    situation from which little good can be expected to emerge.
+
+    See: https://lore.kernel.org/linux-arm-kernel/cover.1296818921.git.segoon@openwall.com/
+
+  **NON_OCTAL_PERMISSIONS**
+    Permission bits should use 4 digit octal permissions (like 0700 or 0444).
+    Avoid using any other base like decimal.
+
+
 Spacing and Brackets
 --------------------
 
@@ -616,7 +698,7 @@ Spacing and Brackets
 
     1. With a type on the left::
 
-        ;int [] a;
+        int [] a;
 
     2. At the beginning of a line for slice initialisers::
 
@@ -630,6 +712,7 @@ Spacing and Brackets
     Code indent should use tabs instead of spaces.
     Outside of comments, documentation and Kconfig,
     spaces are never used for indentation.
+
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#indentation
 
   **CONCATENATED_STRING**
@@ -644,17 +727,20 @@ Spacing and Brackets
 
   **ELSE_AFTER_BRACE**
     `else {` should follow the closing block `}` on the same line.
+
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces
 
   **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
     as the last construct.
+
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#placing-braces-and-spaces
 
   **POINTER_LOCATION**
@@ -671,6 +757,7 @@ Spacing and Brackets
 
   **SPACING**
     Whitespace style used in the kernel sources is described in kernel docs.
+
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces
 
   **SWITCH_CASE_INDENT_LEVEL**
@@ -700,8 +787,40 @@ Spacing and Brackets
     Trailing whitespace should always be removed.
     Some editors highlight the trailing whitespace and cause visual
     distractions when editing files.
+
     See: https://www.kernel.org/doc/html/latest/process/coding-style.html#spaces
 
+  **UNNECESSARY_PARENTHESES**
+    Parentheses are not required in the following cases::
+
+      1. Function pointer uses::
+
+          (foo->bar)();
+
+        could be::
+
+          foo->bar();
+
+      2. Comparisons in if::
+
+          if ((foo->bar) && (foo->baz))
+          if ((foo == bar))
+
+        could be::
+
+          if (foo->bar && foo->baz)
+          if (foo == bar)
+
+      3. addressof/dereference single Lvalues::
+
+          &(foo->bar)
+          *(foo->bar)
+
+        could be::
+
+          &foo->bar
+          *foo->bar
+
   **WHILE_AFTER_BRACE**
     while should follow the closing bracket on the same line::
 
@@ -727,13 +846,40 @@ Others
     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.
+  **FSF_MAILING_ADDRESS**
+    Kernel maintainers reject new instances of the GPL boilerplate paragraph
+    directing people to write to the FSF for a copy of the GPL, since the
+    FSF has moved in the past and may do so again.
+    So do not write paragraphs about writing to the Free Software Foundation's
+    mailing address.
 
-  **NON_OCTAL_PERMISSIONS**
-    Permission bits should use 4 digit octal permissions (like 0700 or 0444).
-    Avoid using any other base like decimal.
+    See: https://lore.kernel.org/lkml/20131006222342.GT19510@leaf/
+
+  **LONG_LINE**
+    The line has exceeded the specified maximum length. Consider refactoring
+    it.
+    To use a different maximum line length, the --max-line-length=n option
+    may be added while invoking checkpatch.
+
+    See: https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
+
+  **LONG_LINE_STRING**
+    A string starts before but extends beyond the maximum line length.
+    To use a different maximum line length, the --max-line-length=n option
+    may be added while invoking checkpatch.
+
+    See: https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
+
+  **LONG_LINE_COMMENT**
+    A comment starts before but extends beyond the maximum line length.
+    To use a different maximum line length, the --max-line-length=n option
+    may be added while invoking checkpatch.
+
+    See: https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings
+
+  **MEMSET**
+    The memset use appears to be incorrect.  This may be caused due to
+    badly ordered parameters.  Please recheck the usage.
 
   **NOT_UNIFIED_DIFF**
     The patch file does not appear to be in unified-diff format.  Please
@@ -742,6 +888,13 @@ Others
   **PRINTF_0XDECIMAL**
     Prefixing 0x with decimal output is defective and should be corrected.
 
+  **SPDX_LICENSE_TAG**
+    The source file is missing or has an improper SPDX identifier tag.
+    The Linux kernel requires the precise SPDX identifier in all source files,
+    and it is thoroughly documented in the kernel docs.
+
+    See: https://www.kernel.org/doc/html/latest/process/license-rules.html
+
   **TRAILING_STATEMENTS**
     Trailing statements (for example after any conditional) should be
     on the next line.
@@ -753,3 +906,6 @@ Others
 
       if (x == y)
               break;
+
+  **TYPO_SPELLING**
+    Some words may have been misspelled.  Consider reviewing them.
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] docs: Add more message type documentations for checkpatch
  2021-05-15 13:23 [PATCH] docs: Add more message type documentations for checkpatch Dwaipayan Ray
@ 2021-05-17 10:04 ` Lukas Bulwahn
  2021-05-20 20:13 ` Jonathan Corbet
  1 sibling, 0 replies; 5+ messages in thread
From: Lukas Bulwahn @ 2021-05-17 10:04 UTC (permalink / raw)
  To: Dwaipayan Ray
  Cc: Jonathan Corbet, Mauro Carvalho Chehab,
	Linux Kernel Mailing List, open list:DOCUMENTATION

On Sat, May 15, 2021 at 3:24 PM Dwaipayan Ray <dwaipayanray1@gmail.com> wrote:
>
> - Document a couple of more checkpatch message types.
> - Add a blank line before all `See:` lines to improve the
>   rst output.
> - Create a new subsection `Permissions` and move a few types
>   to it.
>
> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>

Dwaipayan, thanks for all the good work on this documentation:

Acked-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>

Lukas

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] docs: Add more message type documentations for checkpatch
  2021-05-15 13:23 [PATCH] docs: Add more message type documentations for checkpatch Dwaipayan Ray
  2021-05-17 10:04 ` Lukas Bulwahn
@ 2021-05-20 20:13 ` Jonathan Corbet
  2021-05-20 20:33   ` Dwaipayan Ray
  1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Corbet @ 2021-05-20 20:13 UTC (permalink / raw)
  To: Dwaipayan Ray, mchehab
  Cc: linux-kernel, lukas.bulwahn, linux-doc, Dwaipayan Ray

Dwaipayan Ray <dwaipayanray1@gmail.com> writes:

> - Document a couple of more checkpatch message types.
> - Add a blank line before all `See:` lines to improve the
>   rst output.
> - Create a new subsection `Permissions` and move a few types
>   to it.
>
> Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> ---
>  Documentation/dev-tools/checkpatch.rst | 170 ++++++++++++++++++++++++-
>  1 file changed, 163 insertions(+), 7 deletions(-)

So this seems good, but I just notice something that has evidently
escaped me until now...

> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> index 51fed1bd72ec..e409f27f48b6 100644
> --- a/Documentation/dev-tools/checkpatch.rst
> +++ b/Documentation/dev-tools/checkpatch.rst
> @@ -246,6 +246,7 @@ Allocation style
>      The first argument for kcalloc or kmalloc_array should be the
>      number of elements.  sizeof() as the first argument is generally
>      wrong.
> +
>      See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html
>  
>    **ALLOC_SIZEOF_STRUCT**
> @@ -264,6 +265,7 @@ Allocation style
>    **ALLOC_WITH_MULTIPLY**
>      Prefer kmalloc_array/kcalloc over kmalloc/kzalloc with a
>      sizeof multiply.
> +
>      See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html
>  
>  
> @@ -284,6 +286,7 @@ API usage
>      BUG() or BUG_ON() should be avoided totally.
>      Use WARN() and WARN_ON() instead, and handle the "impossible"
>      error condition as gracefully as possible.
> +
>      See: https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on
>  
>    **CONSIDER_KSTRTO**
> @@ -292,12 +295,23 @@ API usage
>      may lead to unexpected results in callers.  The respective kstrtol(),
>      kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the
>      correct replacements.
> +
>      See: https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull
>  
> +  **IN_ATOMIC**
> +    in_atomic() is not for driver use so any such use is reported as an ERROR.
> +    Also in_atomic() is often used to determine if we may sleep, but it is not
> +    reliable in this use model therefore its use is strongly discouraged.
> +
> +    However, in_atomic() is ok for core kernel use.
> +
> +    See: https://lore.kernel.org/lkml/20080320201723.b87b3732.akpm@linux-foundation.org/
> +
>    **LOCKDEP**
>      The lockdep_no_validate class was added as a temporary measure to
>      prevent warnings on conversion of device->sem to device->mutex.
>      It should not be used for any other purpose.
> +
>      See: https://lore.kernel.org/lkml/1268959062.9440.467.camel@laptop/
>  
>    **MALFORMED_INCLUDE**
> @@ -308,11 +322,18 @@ API usage
>    **USE_LOCKDEP**
>      lockdep_assert_held() annotations should be preferred over
>      assertions based on spin_is_locked()
> +
>      See: https://www.kernel.org/doc/html/latest/locking/lockdep-design.html#annotations
>  
>    **UAPI_INCLUDE**
>      No #include statements in include/uapi should use a uapi/ path.
>  
> +  **USLEEP_RANGE**
> +    usleep_range() should be preferred over udelay(). The proper way of
> +    using usleep_range() is mentioned in the kernel docs.
> +
> +    See: https://www.kernel.org/doc/html/latest/timers/timers-howto.html#delays-information-on-the-various-kernel-delay-sleep-mechanisms

We really shouldn't be linking to outside sites - even kernel.org - when
referring to the kernel docs themselves.  Just say
"Documentation/timers/timers-howto" and let the build system handle the
links.

There's a lot of these in this file, alas...

I've applied this patch since it makes things better overall, but I
would really like to see all those URLs go away if possible.

Thanks,

jon

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] docs: Add more message type documentations for checkpatch
  2021-05-20 20:13 ` Jonathan Corbet
@ 2021-05-20 20:33   ` Dwaipayan Ray
  2021-05-20 20:45     ` Jonathan Corbet
  0 siblings, 1 reply; 5+ messages in thread
From: Dwaipayan Ray @ 2021-05-20 20:33 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Mauro Carvalho Chehab, linux-kernel, Lukas Bulwahn,
	open list:DOCUMENTATION, Joe Perches

(Adding Joe for comments)

On Fri, May 21, 2021 at 1:43 AM Jonathan Corbet <corbet@lwn.net> wrote:
>
> Dwaipayan Ray <dwaipayanray1@gmail.com> writes:
>
> > - Document a couple of more checkpatch message types.
> > - Add a blank line before all `See:` lines to improve the
> >   rst output.
> > - Create a new subsection `Permissions` and move a few types
> >   to it.
> >
> > Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
> > ---
> >  Documentation/dev-tools/checkpatch.rst | 170 ++++++++++++++++++++++++-
> >  1 file changed, 163 insertions(+), 7 deletions(-)
>
> So this seems good, but I just notice something that has evidently
> escaped me until now...
>
> > diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> > index 51fed1bd72ec..e409f27f48b6 100644
> > --- a/Documentation/dev-tools/checkpatch.rst
> > +++ b/Documentation/dev-tools/checkpatch.rst
> > @@ -246,6 +246,7 @@ Allocation style
> >      The first argument for kcalloc or kmalloc_array should be the
> >      number of elements.  sizeof() as the first argument is generally
> >      wrong.
> > +
> >      See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html
> >
> >    **ALLOC_SIZEOF_STRUCT**
> > @@ -264,6 +265,7 @@ Allocation style
> >    **ALLOC_WITH_MULTIPLY**
> >      Prefer kmalloc_array/kcalloc over kmalloc/kzalloc with a
> >      sizeof multiply.
> > +
> >      See: https://www.kernel.org/doc/html/latest/core-api/memory-allocation.html
> >
> >
> > @@ -284,6 +286,7 @@ API usage
> >      BUG() or BUG_ON() should be avoided totally.
> >      Use WARN() and WARN_ON() instead, and handle the "impossible"
> >      error condition as gracefully as possible.
> > +
> >      See: https://www.kernel.org/doc/html/latest/process/deprecated.html#bug-and-bug-on
> >
> >    **CONSIDER_KSTRTO**
> > @@ -292,12 +295,23 @@ API usage
> >      may lead to unexpected results in callers.  The respective kstrtol(),
> >      kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the
> >      correct replacements.
> > +
> >      See: https://www.kernel.org/doc/html/latest/process/deprecated.html#simple-strtol-simple-strtoll-simple-strtoul-simple-strtoull
> >
> > +  **IN_ATOMIC**
> > +    in_atomic() is not for driver use so any such use is reported as an ERROR.
> > +    Also in_atomic() is often used to determine if we may sleep, but it is not
> > +    reliable in this use model therefore its use is strongly discouraged.
> > +
> > +    However, in_atomic() is ok for core kernel use.
> > +
> > +    See: https://lore.kernel.org/lkml/20080320201723.b87b3732.akpm@linux-foundation.org/
> > +
> >    **LOCKDEP**
> >      The lockdep_no_validate class was added as a temporary measure to
> >      prevent warnings on conversion of device->sem to device->mutex.
> >      It should not be used for any other purpose.
> > +
> >      See: https://lore.kernel.org/lkml/1268959062.9440.467.camel@laptop/
> >
> >    **MALFORMED_INCLUDE**
> > @@ -308,11 +322,18 @@ API usage
> >    **USE_LOCKDEP**
> >      lockdep_assert_held() annotations should be preferred over
> >      assertions based on spin_is_locked()
> > +
> >      See: https://www.kernel.org/doc/html/latest/locking/lockdep-design.html#annotations
> >
> >    **UAPI_INCLUDE**
> >      No #include statements in include/uapi should use a uapi/ path.
> >
> > +  **USLEEP_RANGE**
> > +    usleep_range() should be preferred over udelay(). The proper way of
> > +    using usleep_range() is mentioned in the kernel docs.
> > +
> > +    See: https://www.kernel.org/doc/html/latest/timers/timers-howto.html#delays-information-on-the-various-kernel-delay-sleep-mechanisms
>
> We really shouldn't be linking to outside sites - even kernel.org - when
> referring to the kernel docs themselves.  Just say
> "Documentation/timers/timers-howto" and let the build system handle the
> links.
>
> There's a lot of these in this file, alas...
>
> I've applied this patch since it makes things better overall, but I
> would really like to see all those URLs go away if possible.
>

Thanks Jonathan.

Yes it might make things better for the documentation, but again
since we are using these descriptions in checkpatch's --verbose mode,
we shall lose the ability to show the links there which I think is currently
a good addition for the end user.

And I don't think there will be a way to generate these links in checkpatch
without sphinx's build system....

Are there any alternatives?

Also Joe, any comments here?

Thanks,
Dwaipayan.

> Thanks,
>
> jon

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] docs: Add more message type documentations for checkpatch
  2021-05-20 20:33   ` Dwaipayan Ray
@ 2021-05-20 20:45     ` Jonathan Corbet
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Corbet @ 2021-05-20 20:45 UTC (permalink / raw)
  To: Dwaipayan Ray
  Cc: Mauro Carvalho Chehab, linux-kernel, Lukas Bulwahn,
	open list:DOCUMENTATION, Joe Perches

Dwaipayan Ray <dwaipayanray1@gmail.com> writes:

> (Adding Joe for comments)
>
>> We really shouldn't be linking to outside sites - even kernel.org - when
>> referring to the kernel docs themselves.  Just say
>> "Documentation/timers/timers-howto" and let the build system handle the
>> links.
>>
>> There's a lot of these in this file, alas...
>>
>> I've applied this patch since it makes things better overall, but I
>> would really like to see all those URLs go away if possible.
>>
>
> Thanks Jonathan.
>
> Yes it might make things better for the documentation, but again
> since we are using these descriptions in checkpatch's --verbose mode,
> we shall lose the ability to show the links there which I think is currently
> a good addition for the end user.
>
> And I don't think there will be a way to generate these links in checkpatch
> without sphinx's build system....

Ah, OK, I wasn't thinking about that aspect of things; that does change
the situation a bit.

It wouldn't be that hard to do an equivalent of the automarkup magic in
checkpatch to create the relevant links, but I'm not so set on this as
to try to require that.  The links can be as they are, sorry for the
noise.

Thanks,

jon

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-05-20 20:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-15 13:23 [PATCH] docs: Add more message type documentations for checkpatch Dwaipayan Ray
2021-05-17 10:04 ` Lukas Bulwahn
2021-05-20 20:13 ` Jonathan Corbet
2021-05-20 20:33   ` Dwaipayan Ray
2021-05-20 20:45     ` Jonathan Corbet

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.