Re: [PATCH] docs: checkpatch: add some new checkpatch documentation messages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Utkarsh,

On Tue, Aug 02, 2022 at 11:25:28AM +0530, Utkarsh Verma wrote:
> Added and documented the following new message types:
> - JIFFIES_COMPARISON
> - LONG_UDELAY
> - MSLEEP
> - INDENTED_LABEL
> - IF_0
> - IF_1
> - MISORDERED_TYPE
> - UNNECESSARY_BREAK
> - UNNECESSARY_ELSE
> - UNNECESSARY_INT
> - UNSPECIFIED_INT
> 

Use imperative mood for patch description instead.

> +  **MISORDERED_TYPE**
> +    According to section “6.7.2 Type Specifiers” in C90 standard, “type
> +    specifiers may occur in any order.” This means that "signed long long
> +    int" is same as "long long int signed". But to avoid confusion and make
> +    the code easier to read, the declaration type should use the following
> +    format::
> +
> +      [[un]signed] [short|int|long|long long]
> +
> +    Below is the list of standard integer types. Each row lists all the
> +    different ways of specifying a particular type delimited by commas.
> +    Note: Also include all the permutations of a particular type
> +    on the left column delimited by comma. For example, the permutations
> +    for "signed long int" are "signed int long", "long signed int",
> +    "long int signed", "int signed long", and "int long signed".
> +
> +    +--------------------------------------------------+--------------------+
> +    |                       Types                      |   Recommended Way  |
> +    +=======================================================================+
> +    | char                                             | char               |
> +    +-----------------------------------------------------------------------+
> +    | signed char                                      | signed char        |
> +    +-----------------------------------------------------------------------+
> +    | unsigned char                                    | unsigned char      |
> +    +-----------------------------------------------------------------------+
> +    | signed, int, signed int                          | int                |
> +    +-----------------------------------------------------------------------+
> +    | unsigned, unsigned int                           | unsigned int       |
> +    +-----------------------------------------------------------------------+
> +    | short, signed short, short int, signed short int | short              |
> +    +-----------------------------------------------------------------------+
> +    | unsigned short, unsigned short int               | unsigned short     |
> +    +-----------------------------------------------------------------------+
> +    | long, signed long, long int, signed long int     | long               |
> +    +-----------------------------------------------------------------------+
> +    | unsigned long, unsigned long int                 | unsigned long      |
> +    +-----------------------------------------------------------------------|
> +    | long long, signed long long, long long int,      | long long          |
> +    | signed long long int                             |                    |
> +    +-----------------------------------------------------------------------+
> +    | unsigned long long, unsigned long long int       | unsigned long long |
> +    +-----------------------------------------------------------------------+
> +

The table above triggers htmldocs error, so I have to apply the fixup:

---- >8 ----

>From 4fcc0df4ffcf1190330c12db5352cae03f8620fb Mon Sep 17 00:00:00 2001
From: Bagas Sanjaya <bagasdotme@xxxxxxxxx>
Date: Wed, 3 Aug 2022 09:48:43 +0700
Subject: [PATCH] Documentation: checkpatch: MISORDERED_TYPE table intersection
 fixup

Sphinx reported error and warnings pointed at MISORDERED_TYPE table:

Documentation/dev-tools/checkpatch.rst:1393: (SEVERE/4) Unexpected section title or transition.
Documentation/dev-tools/checkpatch.rst:1393: WARNING: Block quote ends without a blank line; unexpected unindent.
Documentation/dev-tools/checkpatch.rst:1393: WARNING: Unexpected section title or transition.

Fix these above by marking cell intersections with plus (+) sign.

Link: https://lore.kernel.org/linux-doc/202208030829.xj2bvI7P-lkp@xxxxxxxxx/
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Signed-off-by: Bagas Sanjaya <bagasdotme@xxxxxxxxx>
---
 Documentation/dev-tools/checkpatch.rst | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
index 78abcadb522824..a9d27913e6c46f 100644
--- a/Documentation/dev-tools/checkpatch.rst
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -1371,30 +1371,30 @@ Others
 
     +--------------------------------------------------+--------------------+
     |                       Types                      |   Recommended Way  |
-    +=======================================================================+
+    +==================================================+====================+
     | char                                             | char               |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | signed char                                      | signed char        |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | unsigned char                                    | unsigned char      |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | signed, int, signed int                          | int                |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | unsigned, unsigned int                           | unsigned int       |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | short, signed short, short int, signed short int | short              |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | unsigned short, unsigned short int               | unsigned short     |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | long, signed long, long int, signed long int     | long               |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | unsigned long, unsigned long int                 | unsigned long      |
-    +-----------------------------------------------------------------------|
+    +--------------------------------------------------+--------------------+
     | long long, signed long long, long long int,      | long long          |
     | signed long long int                             |                    |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
     | unsigned long long, unsigned long long int       | unsigned long long |
-    +-----------------------------------------------------------------------+
+    +--------------------------------------------------+--------------------+
 
   **NOT_UNIFIED_DIFF**
     The patch file does not appear to be in unified-diff format.  Please
---- >8 ----

> +    Also, do not change the code if there is only a single return statement
> +    inside if-else block, like::
> +
> +      if (a > b)
> +              return a;
> +      else
> +              return b;
> +
> +    now if the else statement is removed::
> +
> +      if (a > b)
> +              return a;
> +      return b;
> +
> +    there is no considerable increase in the readability and one can argue
> +    that the first form is more readable because of the indentation. So
> +    do not remove the else statement in case of a single return statement
> +    inside the if-else block.
> +

So the first indentation above is more readable for it's clear that
b is returned if the condition is false (in this case, a < b), right?

> +    See: https://lore.kernel.org/lkml/20140925032215.GK7996@xxxxxxxxxxxxxxxxxx/
> +
> +  **UNNECESSARY_INT**
> +    On Sun, 2018-08-05 at 08:52 -0700, Linus Torvalds wrote:
> +    > "long unsigned int" isn't _technically_ wrong. But we normally
> +    > call that type "unsigned long".
> +

IMO, the mail quote above can be deleted.

Thanks.

-- 
An old man doll... just what I always wanted! - Clara



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux