[PATCH RFC v3 0/3] checkpatch: add verbose mode

[PATCH RFC v3 0/3] checkpatch: add verbose mode

[Dwaipayan Ray]

Add a new verbose mode to checkpatch. The verbose test 
descriptions are read from the checkpatch documentation
file at `Documentation/dev-tools/checkpatch.rst`.

The verbose mode is optional and can be enabled by the
flag -v or --verbose.

The documentation file is only parsed by checkpatch.pl
if the verbose mode is enabled. The verbose mode is also
suppressed when the --terse option is specified.

Changes in v3:
- Simplify documentation file parsing in checkpatch
- Document a total of 33 message types for checkpatch

Changes in v2:
- Use .rst Field Lists to specify the type descriptions.
- Add a few more type descriptions to documentation.


Dwaipayan Ray (3):
  checkpatch: add verbose mode
  docs: add documentation for checkpatch
  docs: add documentation for checkpatch

 Documentation/dev-tools/checkpatch.rst | 494 +++++++++++++++++++++++++
 Documentation/dev-tools/index.rst      |   1 +
 scripts/checkpatch.pl                  |  55 ++-
 3 files changed, 549 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/dev-tools/checkpatch.rst

-- 
2.30.0

[Linux-kernel-mentees] [PATCH RFC v3 0/3] checkpatch: add verbose mode

[Dwaipayan Ray]

Add a new verbose mode to checkpatch. The verbose test 
descriptions are read from the checkpatch documentation
file at `Documentation/dev-tools/checkpatch.rst`.

The verbose mode is optional and can be enabled by the
flag -v or --verbose.

The documentation file is only parsed by checkpatch.pl
if the verbose mode is enabled. The verbose mode is also
suppressed when the --terse option is specified.

Changes in v3:
- Simplify documentation file parsing in checkpatch
- Document a total of 33 message types for checkpatch

Changes in v2:
- Use .rst Field Lists to specify the type descriptions.
- Add a few more type descriptions to documentation.


Dwaipayan Ray (3):
  checkpatch: add verbose mode
  docs: add documentation for checkpatch
  docs: add documentation for checkpatch

 Documentation/dev-tools/checkpatch.rst | 494 +++++++++++++++++++++++++
 Documentation/dev-tools/index.rst      |   1 +
 scripts/checkpatch.pl                  |  55 ++-
 3 files changed, 549 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/dev-tools/checkpatch.rst

-- 
2.30.0

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

[PATCH RFC v3 1/3] checkpatch: add verbose mode

[Dwaipayan Ray]

Add a new verbose mode to checkpatch.pl to emit additional verbose
test descriptions. The verbose mode is optional and can be enabled
by the flag -v or --verbose.

The test descriptions are parsed from the checkpatch documentation
file at `Documentation/dev-tools/checkpatch.rst`. The test
descriptions in the docs are kept in a fixed format using rst field
lists, an example of which is as follows:

:LINE_SPACING:
  Vertical space is wasted given the limited number of lines an
  editor window can display when multiple blank lines are used.

: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.
  ref: `Documentation/process/submitting-patches.rst`

The verbose descriptions are not shown when the --terse option
is enabled.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 scripts/checkpatch.pl | 55 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9a549b009d2f..062545e68405 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -23,6 +23,8 @@ my $V = '0.32';
 use Getopt::Long qw(:config no_auto_abbrev);
 
 my $quiet = 0;
+my $verbose = 0;
+my %verbose_messages = ();
 my $tree = 1;
 my $chk_signoff = 1;
 my $chk_patch = 1;
@@ -61,6 +63,7 @@ my $spelling_file = "$D/spelling.txt";
 my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
+my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst";
 my $typedefsfile;
 my $color = "auto";
 my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
@@ -78,6 +81,7 @@ Version: $V
 
 Options:
   -q, --quiet                quiet
+  -v, --verbose              verbose mode
   --no-tree                  run without a kernel tree
   --no-signoff               do not check for 'Signed-off-by' line
   --patch                    treat FILE as patchfile (default)
@@ -198,6 +202,46 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+sub load_docs {
+	open(my $docs, '<', "$docsfile")
+	    or warn "$P: Can't read the documentation file $docsfile $!\n";
+
+	my $type = '';
+	my $desc = '';
+	my $in_desc = 0;
+
+	while (<$docs>) {
+		chomp;
+		my $line = $_;
+		$line =~ s/\s+$//;
+
+		if ($line =~ /^\:(.+)\:$/) {
+			if ($desc ne '') {
+				$verbose_messages{$type} = trim($desc);
+			}
+			$type = $1;
+			$desc = '';
+			$in_desc = 1;
+		} elsif ($in_desc) {
+			if ($line =~ /^(?:\s{2,}|$)/) {
+				$line =~ s/^\s{2}//;
+				$desc .= $line;
+				$desc .= "\n";
+			} else {
+				$verbose_messages{$type} = trim($desc);
+				$type = '';
+				$desc = '';
+				$in_desc = 0;
+			}
+		}
+	}
+
+	if ($desc ne '') {
+		$verbose_messages{$type} = trim($desc);
+	}
+	close($docs);
+}
+
 # Perl's Getopt::Long allows options to take optional arguments after a space.
 # Prevent --color by itself from consuming other arguments
 foreach (@ARGV) {
@@ -208,6 +252,7 @@ foreach (@ARGV) {
 
 GetOptions(
 	'q|quiet+'	=> \$quiet,
+	'v|verbose!'	=> \$verbose,
 	'tree!'		=> \$tree,
 	'signoff!'	=> \$chk_signoff,
 	'patch!'	=> \$chk_patch,
@@ -249,6 +294,8 @@ help(0) if ($help);
 
 list_types(0) if ($list_types);
 
+load_docs() if ($verbose && !$terse);
+
 $fix = 1 if ($fix_inplace);
 $check_orig = $check;
 
@@ -2209,7 +2256,13 @@ sub report {
 		splice(@lines, 1, 1);
 		$output = join("\n", @lines);
 	}
-	$output = (split('\n', $output))[0] . "\n" if ($terse);
+
+	if ($terse) {
+		$output = (split('\n', $output))[0] . "\n";
+	} elsif ($verbose &&
+		 exists $verbose_messages{$type}) {
+		$output .= $verbose_messages{$type} . "\n\n";
+	}
 
 	push(our @report, $output);
 
-- 
2.30.0

[Linux-kernel-mentees] [PATCH RFC v3 1/3] checkpatch: add verbose mode

[Dwaipayan Ray]

Add a new verbose mode to checkpatch.pl to emit additional verbose
test descriptions. The verbose mode is optional and can be enabled
by the flag -v or --verbose.

The test descriptions are parsed from the checkpatch documentation
file at `Documentation/dev-tools/checkpatch.rst`. The test
descriptions in the docs are kept in a fixed format using rst field
lists, an example of which is as follows:

:LINE_SPACING:
  Vertical space is wasted given the limited number of lines an
  editor window can display when multiple blank lines are used.

: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.
  ref: `Documentation/process/submitting-patches.rst`

The verbose descriptions are not shown when the --terse option
is enabled.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 scripts/checkpatch.pl | 55 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 54 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9a549b009d2f..062545e68405 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -23,6 +23,8 @@ my $V = '0.32';
 use Getopt::Long qw(:config no_auto_abbrev);
 
 my $quiet = 0;
+my $verbose = 0;
+my %verbose_messages = ();
 my $tree = 1;
 my $chk_signoff = 1;
 my $chk_patch = 1;
@@ -61,6 +63,7 @@ my $spelling_file = "$D/spelling.txt";
 my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $conststructsfile = "$D/const_structs.checkpatch";
+my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst";
 my $typedefsfile;
 my $color = "auto";
 my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE
@@ -78,6 +81,7 @@ Version: $V
 
 Options:
   -q, --quiet                quiet
+  -v, --verbose              verbose mode
   --no-tree                  run without a kernel tree
   --no-signoff               do not check for 'Signed-off-by' line
   --patch                    treat FILE as patchfile (default)
@@ -198,6 +202,46 @@ if (-f $conf) {
 	unshift(@ARGV, @conf_args) if @conf_args;
 }
 
+sub load_docs {
+	open(my $docs, '<', "$docsfile")
+	    or warn "$P: Can't read the documentation file $docsfile $!\n";
+
+	my $type = '';
+	my $desc = '';
+	my $in_desc = 0;
+
+	while (<$docs>) {
+		chomp;
+		my $line = $_;
+		$line =~ s/\s+$//;
+
+		if ($line =~ /^\:(.+)\:$/) {
+			if ($desc ne '') {
+				$verbose_messages{$type} = trim($desc);
+			}
+			$type = $1;
+			$desc = '';
+			$in_desc = 1;
+		} elsif ($in_desc) {
+			if ($line =~ /^(?:\s{2,}|$)/) {
+				$line =~ s/^\s{2}//;
+				$desc .= $line;
+				$desc .= "\n";
+			} else {
+				$verbose_messages{$type} = trim($desc);
+				$type = '';
+				$desc = '';
+				$in_desc = 0;
+			}
+		}
+	}
+
+	if ($desc ne '') {
+		$verbose_messages{$type} = trim($desc);
+	}
+	close($docs);
+}
+
 # Perl's Getopt::Long allows options to take optional arguments after a space.
 # Prevent --color by itself from consuming other arguments
 foreach (@ARGV) {
@@ -208,6 +252,7 @@ foreach (@ARGV) {
 
 GetOptions(
 	'q|quiet+'	=> \$quiet,
+	'v|verbose!'	=> \$verbose,
 	'tree!'		=> \$tree,
 	'signoff!'	=> \$chk_signoff,
 	'patch!'	=> \$chk_patch,
@@ -249,6 +294,8 @@ help(0) if ($help);
 
 list_types(0) if ($list_types);
 
+load_docs() if ($verbose && !$terse);
+
 $fix = 1 if ($fix_inplace);
 $check_orig = $check;
 
@@ -2209,7 +2256,13 @@ sub report {
 		splice(@lines, 1, 1);
 		$output = join("\n", @lines);
 	}
-	$output = (split('\n', $output))[0] . "\n" if ($terse);
+
+	if ($terse) {
+		$output = (split('\n', $output))[0] . "\n";
+	} elsif ($verbose &&
+		 exists $verbose_messages{$type}) {
+		$output .= $verbose_messages{$type} . "\n\n";
+	}
 
 	push(our @report, $output);
 
-- 
2.30.0

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

[PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

Add documentation for kernel script checkpatch.pl.
This documentation is also parsed by checkpatch to
enable a verbose mode.

The message types in checkpatch are documented with rst
field lists. A total of 33 checkpatch type descriptions
are added.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 494 +++++++++++++++++++++++++
 1 file changed, 494 insertions(+)
 create mode 100644 Documentation/dev-tools/checkpatch.rst

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
new file mode 100644
index 000000000000..8e6ff1e27353
--- /dev/null
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -0,0 +1,494 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+==========
+Checkpatch
+==========
+
+This document describes the kernel script checkpatch.pl.
+
+.. Table of Contents
+
+	=== 1 Introduction
+	=== 2 Options
+	=== 3 Message Levels
+	=== 4 Type Descriptions
+
+1 Introduction
+--------------
+
+Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style
+violations in patches and optionally corrects them.  Checkpatch can also be run on
+file contexts and without the kernel tree.
+
+Checkpatch is not always right. Your judgement takes precedence over checkpatch
+messages.  If your code looks better with the violations, then its probably
+best left alone.
+
+
+2 Options
+---------
+
+This section will describe the options checkpatch can be run with.
+
+Usage::
+
+  ./scripts/checkpatch.pl [OPTION]... [FILE]...
+
+Available options:
+
+ - -q,  --quiet
+
+   Enable quiet mode.
+
+ - -v,  --verbose
+   Enable verbose mode.  Additional verbose test descriptions are output
+   so as to provide information on why that particular message is shown.
+
+ - --no-tree
+
+   Run checkpatch without the kernel tree.
+
+ - --no-signoff
+
+   Disable the 'Signed-off-by' line check.  The sign-off is a simple line at
+   the end of the explanation for the patch, which certifies that you wrote it
+   or otherwise have the right to pass it on as an open-source patch.
+
+   Example::
+
+	 Signed-off-by: Random J Developer <random@developer.example.org>
+
+   Setting this flag effectively stops a message for a missing signed-off-by line
+   in a patch context.
+
+ - --patch
+
+   Treat FILE as a patch.  This is the default option and need not be
+   explicitly specified.
+
+ - --emacs
+
+   Set output to emacs compile window format.  This allows emacs users to jump
+   from the error in the compile window directly to the offending line in the patch.
+
+ - --terse
+
+   Output only one line per report.
+
+ - --showfile
+
+   Show the diffed file position instead of the input file position.
+
+ - -g,  --git
+
+   Treat FILE as a single commit or a git revision range.
+
+   Single commit with:
+
+   - <rev>
+   - <rev>^
+   - <rev>~n
+
+   Multiple commits with:
+
+   - <rev1>..<rev2>
+   - <rev1>...<rev2>
+   - <rev>-<count>
+
+ - -f,  --file
+
+   Treat FILE as a regular source file.  This option must be used when running
+   checkpatch on source files in the kernel.
+
+ - --subjective,  --strict
+
+   Enable stricter tests in checkpatch.  By default the tests emitted as CHECK
+   do not activate by default.  Use this flag to activate the CHECK tests.
+
+ - --list-types
+
+   Every message emitted by checkpatch has an associated TYPE.  Add this flag to
+   display all the types in checkpatch.
+
+   Note that when this flag is active, checkpatch does not read the input FILE, and
+   no message is emitted.  Only a list of types in checkpatch is output.
+
+ - --types TYPE(,TYPE2...)
+
+   Only display messages with the given types.
+
+   Example::
+
+     ./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,NO_AUTHOR_SIGN_OFF
+
+ - --ignore TYPE(,TYPE2...)
+
+   Checkpatch will not emit messages for the specified types.
+
+   Example::
+
+     ./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,NO_AUTHOR_SIGN_OFF
+
+ - --show-types
+
+   By default checkpatch doesn't display the type associated with the messages.
+   Set this flag to show the message type in the output.
+
+ - --max-line-length=n
+
+   Set the max line length (default 100).  If a line exceeds the specified length,
+   a LONG_LINE message is emitted.
+
+
+   The message level is different for patch and file contexts.  For patches, a WARNING is
+   emitted.  While a milder CHECK is emitted for files.  So for file contexts, the --strict
+   flag must also be enabled.
+
+ - --min-conf-desc-length=n
+
+   Set the Kconfig entry minimum description length, if shorter, warn.
+
+ - --tab-size=n
+
+   Set the number of spaces for tab (default 8).
+
+ - --root=PATH
+
+   PATH to the kernel tree root.
+
+   This option must be specified when invoking checkpatch from outside
+   the kernel root.
+
+ - --no-summary
+
+   Suppress the per file summary.
+
+ - --mailback
+
+   Only produce a report in case of Warnings or Errors.  Milder Checks are
+   excluded from this.
+
+ - --summary-file
+
+   Include the filename in summary.
+
+ - --debug KEY=[0|1]
+
+   Turn on/off debugging of KEY, where KEY is one of 'values', 'possible',
+   'type', and 'attr' (default is all off).
+
+ - --fix
+
+   This is an EXPERIMENTAL feature.  If correctable errors exists, a file
+   <inputfile>.EXPERIMENTAL-checkpatch-fixes is created which has the
+   automatically fixable errors corrected.
+
+ - --fix-inplace
+
+   EXPERIMENTAL - Similar to --fix but the input file is overwritten with fixes.
+
+   DO NOT USE this flag unless you are absolutely sure and you have a backup in place.
+
+ - --ignore-perl-version
+
+   Override checking of perl version.  Runtime errors maybe encountered after
+   enabling this flag if the perl version does not meet the minimum specified.
+
+ - --codespell
+
+   Use the codespell dictionary for checking spelling errors.
+
+ - --codespellfile
+
+   Use the specified codespell file.  Default is '/usr/share/codespell/dictionary.txt'.
+
+ - --typedefsfile
+
+   Read additional types from this file.
+
+ - --color[=WHEN]
+
+   Use colors 'always', 'never', or only when output is a terminal ('auto').
+   Default is 'auto'.
+
+ - --kconfig-prefix=WORD
+
+   Use WORD as a prefix for Kconfig symbols (default is `CONFIG_`).
+
+ - -h, --help, --version
+
+   Display the help text.
+
+3 Message Levels
+----------------
+
+Messages in checkpatch are divided into three levels. The levels of messages in
+checkpatch denote the severity of the error. They are:
+
+ - ERROR
+
+   This is the most strict level.  Messages of type ERROR must be taken
+   seriously as they denote things that are very likely to be wrong.
+
+ - WARNING
+
+   This is the next stricter level.  Messages of type WARNING requires a
+   more careful review.  But it is milder than an ERROR.
+
+ - CHECK
+
+   This is the mildest level.  These are things which may require some thought.
+
+4 Type Descriptions
+-------------------
+
+This section contains a description of all the message types in checkpatch.
+
+.. Types in this section are also parsed by checkpatch.
+.. Please keep the types sorted alphabetically.
+
+:ALLOC_ARRAY_ARGS:
+  The first argument for kcalloc or kmalloc_array should be the
+  number of elements.  sizeof() as the first argument is generally
+  wrong.
+
+:ALLOC_SIZEOF_STRUCT:
+  The allocation style is bad.  In general for family of
+  allocation functions using sizeof() to get memory size,
+  constructs like::
+
+    p = alloc(sizeof(struct foo), ...)
+
+  should be::
+
+    p = alloc(sizeof(*p), ...)
+
+:ALLOC_WITH_MULTIPLY:
+  Prefer kmalloc_array/kcalloc over kmalloc/kzalloc with a
+  sizeof multiply.
+  Ref: `Documentation/core-api/memory-allocation.rst`
+
+:ARCH_DEFINES:
+  Architecture specific defines should be avoided wherever
+  possible.
+
+:ARCH_INCLUDE_LINUX:
+  Whenever asm/file.h is included and linux/file.h exists, a
+  conversion can be made when linux/file.h includes asm/file.h.
+  However this is not always the case (See signal.h).
+  This message type is emitted only for includes from arch/.
+
+:ARRAY_SIZE:
+  The ARRAY_SIZE(foo) macro should be preferred over
+  sizeof(foo)/sizeof(foo[0]) for finding number of elements in an
+  array.
+
+  The macro is defined in include/linux/kernel.h::
+
+    #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+
+:ASSIGNMENT_CONTINUATIONS:
+  Assignment operators should not be written at the start of a
+  line but should follow the operand at the previous line.
+
+:ASSIGN_IN_IF:
+  Do not use assignments in if condition.
+  Example::
+
+    if ((foo = bar(...)) < BAZ) {
+
+  should be written as::
+
+    foo = bar(...);
+    if (foo < BAZ) {
+
+:AVOID_BUG:
+  BUG() or BUG_ON() should be avoided totally.
+  Use WARN() and WARN_ON() instead, and handle the "impossible"
+  error condition as gracefully as possible.
+  Ref: `Documentation/process/deprecated.rst`
+
+:AVOID_EXTERNS:
+  Function prototypes don't need to be declared extern in .h
+  files.  It's assumed by the compiler and is unnecessary.
+
+:AVOID_L_PREFIX:
+  Local symbol names that are prefixed with `.L` should be avoided,
+  as this has special meaning for the assembler; a symbol entry will
+  not be emitted into the symbol table.  This can prevent `objtool`
+  from generating correct unwind info.
+
+  Symbols with STB_LOCAL binding may still be used, and `.L` prefixed
+  local symbol names are still generally usable within a function,
+  but `.L` prefixed local symbol names should not be used to denote
+  the beginning or end of code regions via
+  `SYM_CODE_START_LOCAL`/`SYM_CODE_END`
+
+:BAD_SIGN_OFF:
+  The signed-off-by line does not fall in line with the standards
+  specified by the community.
+  Ref: `Documentation/process/submitting-patches.rst`
+
+:BAD_STABLE_ADDRESS_STYLE:
+  The email format for stable is incorrect.
+  Some valid options for stable address are::
+
+    1. stable@vger.kernel.org
+    2. stable@kernel.org
+
+  For adding version info, the following comment style should be used::
+
+    stable@vger.kernel.org # version info
+
+:BIT_MACRO:
+  Defines like: 1 << <digit> could be BIT(digit).
+  The BIT() macro is defined in include/linux/bitops.h::
+
+    #define BIT(nr)         (1UL << (nr))
+
+:BLOCK_COMMENT_STYLE:
+  The comment style is incorrect.  The preferred style for multi-
+  line comments is::
+
+    /*
+     * This is the preferred style
+     * for multi line comments.
+     */
+
+  The networking comment style is a bit different, with the first line
+  not empty like the former::
+
+    /* This is the preferred comment style
+     * for files in net/ and drivers/net/
+     */
+
+  Ref: `Documentation/process/coding-style.rst`
+
+:BOOL_COMPARISON:
+  Comparisons of A to true and false are better written
+  as A and !A.
+  Ref: `https://lore.kernel.org/lkml/1365563834.27174.12.camel@joe-AO722/`
+
+:BRACES:
+  The placement of braces is stylistically incorrect.
+  The preferred way is to put the opening brace last on the line,
+  and put the closing brace first::
+
+    if (x is true) {
+      we do y
+    }
+
+  This applies for all non-functional blocks.
+  However, there is one special case, namely functions: they have the
+  opening brace at the beginning of the next line, thus::
+
+    int function(int x)
+    {
+      body of function
+    }
+
+  Ref: `Documentation/process/coding-style.rst Section 3`
+
+:BRACKET_SPACE:
+  Whitespace before opening bracket '[' is prohibited.
+  There are some exceptions:
+
+  1. With a type on the left::
+
+     ;int [] a;
+
+  2. At the beginning of a line for slice initialisers::
+
+     [0...10] = 5,
+
+  3. Inside a curly brace::
+
+     = { [0...10] = 5 }
+
+:C99_COMMENTS:
+  C99 style single line comments (//) should not be used.
+  Prefer the block comment style instead.
+  Ref: `Documentation/process/coding-style.rst Section 8`
+
+:CAMELCASE:
+  Avoid CamelCase Identifiers.  snake_case can be an
+  alternative.
+
+:CODE_INDENT:
+  Code indent should use tabs instead of spaces.
+  Outside of comments, documentation and Kconfig,
+  spaces are never used for indentation.
+  Ref: `Documentation/process/coding-style.rst Section 1`
+
+:COMMIT_COMMENT_SYMBOL:
+  Commit log lines starting with a '#' are ignored by git as
+  comments.  To solve this problem addition of a single space
+  infront of the log line is enough.
+
+:COMMIT_MESSAGE:
+  The patch is missing a commit description.  A brief
+  description of the changes made by the patch should be added.
+  Ref: `Documentation/process/submitting-patches.rst`
+
+:COMPARISON_TO_NULL:
+  Comparisons to NULL in the form (foo == NULL) or (foo != NULL)
+  are better written as (!foo) and (foo).
+
+:COMPLEX_MACRO:
+  Macros with complex values should be enclosed within parentheses.
+  Consider::
+
+    #define SOME_MACRO IS_ENABLED(CONFIG_XX) ? 1 : 0
+
+  This can be better written as::
+
+    #define SOME_MACRO (IS_ENABLED(CONFIG_XX) ? 1 : 0)
+
+:CONCATENATED_STRING:
+  Concatenated elements should have a space in between.
+  Example::
+
+    printk(KERN_INFO"bar");
+
+  should be::
+
+    printk(KERN_INFO "bar");
+
+:CONFIG_DESCRIPTION:
+  Kconfig symbols should have a help text which fully describes
+  it.
+
+:CONSIDER_KSTRTO:
+  The simple_strtol(), simple_strtoll(), simple_strtoul(), and
+  simple_strtoull() functions explicitly ignore overflows, which
+  may lead to unexpected results in callers.  The respective kstrtol(),
+  kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the
+  correct replacements.
+  Ref: `Documentation/process/deprecated.rst`
+
+:CONSTANT_COMPARISON:
+  Comparisons with a constant or upper case identifier on the left
+  side of the test should be avoided.
+
+:LINE_SPACING:
+  Vertical space is wasted given the limited number of lines an
+  editor window can display when multiple blank lines are used.
+  Ref: `Documentation/process/coding-style.rst Section 3.1`
+
+: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.
+  ref: `Documentation/process/submitting-patches.rst`
+
+:NO_AUTHOR_SIGN_OFF:
+  The author of the patch has not signed off the patch.  It is
+  required that a simple sign off line should be present at the
+  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.
+
+:TRAILING_WHITESPACE:
+  Trailing whitespace should always be removed.
+  Some editors highlight the trailing whitespace and cause visual
+  distractions when editing files.
-- 
2.30.0

[Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

Add documentation for kernel script checkpatch.pl.
This documentation is also parsed by checkpatch to
enable a verbose mode.

The message types in checkpatch are documented with rst
field lists. A total of 33 checkpatch type descriptions
are added.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/checkpatch.rst | 494 +++++++++++++++++++++++++
 1 file changed, 494 insertions(+)
 create mode 100644 Documentation/dev-tools/checkpatch.rst

diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
new file mode 100644
index 000000000000..8e6ff1e27353
--- /dev/null
+++ b/Documentation/dev-tools/checkpatch.rst
@@ -0,0 +1,494 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+==========
+Checkpatch
+==========
+
+This document describes the kernel script checkpatch.pl.
+
+.. Table of Contents
+
+	=== 1 Introduction
+	=== 2 Options
+	=== 3 Message Levels
+	=== 4 Type Descriptions
+
+1 Introduction
+--------------
+
+Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style
+violations in patches and optionally corrects them.  Checkpatch can also be run on
+file contexts and without the kernel tree.
+
+Checkpatch is not always right. Your judgement takes precedence over checkpatch
+messages.  If your code looks better with the violations, then its probably
+best left alone.
+
+
+2 Options
+---------
+
+This section will describe the options checkpatch can be run with.
+
+Usage::
+
+  ./scripts/checkpatch.pl [OPTION]... [FILE]...
+
+Available options:
+
+ - -q,  --quiet
+
+   Enable quiet mode.
+
+ - -v,  --verbose
+   Enable verbose mode.  Additional verbose test descriptions are output
+   so as to provide information on why that particular message is shown.
+
+ - --no-tree
+
+   Run checkpatch without the kernel tree.
+
+ - --no-signoff
+
+   Disable the 'Signed-off-by' line check.  The sign-off is a simple line at
+   the end of the explanation for the patch, which certifies that you wrote it
+   or otherwise have the right to pass it on as an open-source patch.
+
+   Example::
+
+	 Signed-off-by: Random J Developer <random@developer.example.org>
+
+   Setting this flag effectively stops a message for a missing signed-off-by line
+   in a patch context.
+
+ - --patch
+
+   Treat FILE as a patch.  This is the default option and need not be
+   explicitly specified.
+
+ - --emacs
+
+   Set output to emacs compile window format.  This allows emacs users to jump
+   from the error in the compile window directly to the offending line in the patch.
+
+ - --terse
+
+   Output only one line per report.
+
+ - --showfile
+
+   Show the diffed file position instead of the input file position.
+
+ - -g,  --git
+
+   Treat FILE as a single commit or a git revision range.
+
+   Single commit with:
+
+   - <rev>
+   - <rev>^
+   - <rev>~n
+
+   Multiple commits with:
+
+   - <rev1>..<rev2>
+   - <rev1>...<rev2>
+   - <rev>-<count>
+
+ - -f,  --file
+
+   Treat FILE as a regular source file.  This option must be used when running
+   checkpatch on source files in the kernel.
+
+ - --subjective,  --strict
+
+   Enable stricter tests in checkpatch.  By default the tests emitted as CHECK
+   do not activate by default.  Use this flag to activate the CHECK tests.
+
+ - --list-types
+
+   Every message emitted by checkpatch has an associated TYPE.  Add this flag to
+   display all the types in checkpatch.
+
+   Note that when this flag is active, checkpatch does not read the input FILE, and
+   no message is emitted.  Only a list of types in checkpatch is output.
+
+ - --types TYPE(,TYPE2...)
+
+   Only display messages with the given types.
+
+   Example::
+
+     ./scripts/checkpatch.pl mypatch.patch --types EMAIL_SUBJECT,NO_AUTHOR_SIGN_OFF
+
+ - --ignore TYPE(,TYPE2...)
+
+   Checkpatch will not emit messages for the specified types.
+
+   Example::
+
+     ./scripts/checkpatch.pl mypatch.patch --ignore EMAIL_SUBJECT,NO_AUTHOR_SIGN_OFF
+
+ - --show-types
+
+   By default checkpatch doesn't display the type associated with the messages.
+   Set this flag to show the message type in the output.
+
+ - --max-line-length=n
+
+   Set the max line length (default 100).  If a line exceeds the specified length,
+   a LONG_LINE message is emitted.
+
+
+   The message level is different for patch and file contexts.  For patches, a WARNING is
+   emitted.  While a milder CHECK is emitted for files.  So for file contexts, the --strict
+   flag must also be enabled.
+
+ - --min-conf-desc-length=n
+
+   Set the Kconfig entry minimum description length, if shorter, warn.
+
+ - --tab-size=n
+
+   Set the number of spaces for tab (default 8).
+
+ - --root=PATH
+
+   PATH to the kernel tree root.
+
+   This option must be specified when invoking checkpatch from outside
+   the kernel root.
+
+ - --no-summary
+
+   Suppress the per file summary.
+
+ - --mailback
+
+   Only produce a report in case of Warnings or Errors.  Milder Checks are
+   excluded from this.
+
+ - --summary-file
+
+   Include the filename in summary.
+
+ - --debug KEY=[0|1]
+
+   Turn on/off debugging of KEY, where KEY is one of 'values', 'possible',
+   'type', and 'attr' (default is all off).
+
+ - --fix
+
+   This is an EXPERIMENTAL feature.  If correctable errors exists, a file
+   <inputfile>.EXPERIMENTAL-checkpatch-fixes is created which has the
+   automatically fixable errors corrected.
+
+ - --fix-inplace
+
+   EXPERIMENTAL - Similar to --fix but the input file is overwritten with fixes.
+
+   DO NOT USE this flag unless you are absolutely sure and you have a backup in place.
+
+ - --ignore-perl-version
+
+   Override checking of perl version.  Runtime errors maybe encountered after
+   enabling this flag if the perl version does not meet the minimum specified.
+
+ - --codespell
+
+   Use the codespell dictionary for checking spelling errors.
+
+ - --codespellfile
+
+   Use the specified codespell file.  Default is '/usr/share/codespell/dictionary.txt'.
+
+ - --typedefsfile
+
+   Read additional types from this file.
+
+ - --color[=WHEN]
+
+   Use colors 'always', 'never', or only when output is a terminal ('auto').
+   Default is 'auto'.
+
+ - --kconfig-prefix=WORD
+
+   Use WORD as a prefix for Kconfig symbols (default is `CONFIG_`).
+
+ - -h, --help, --version
+
+   Display the help text.
+
+3 Message Levels
+----------------
+
+Messages in checkpatch are divided into three levels. The levels of messages in
+checkpatch denote the severity of the error. They are:
+
+ - ERROR
+
+   This is the most strict level.  Messages of type ERROR must be taken
+   seriously as they denote things that are very likely to be wrong.
+
+ - WARNING
+
+   This is the next stricter level.  Messages of type WARNING requires a
+   more careful review.  But it is milder than an ERROR.
+
+ - CHECK
+
+   This is the mildest level.  These are things which may require some thought.
+
+4 Type Descriptions
+-------------------
+
+This section contains a description of all the message types in checkpatch.
+
+.. Types in this section are also parsed by checkpatch.
+.. Please keep the types sorted alphabetically.
+
+:ALLOC_ARRAY_ARGS:
+  The first argument for kcalloc or kmalloc_array should be the
+  number of elements.  sizeof() as the first argument is generally
+  wrong.
+
+:ALLOC_SIZEOF_STRUCT:
+  The allocation style is bad.  In general for family of
+  allocation functions using sizeof() to get memory size,
+  constructs like::
+
+    p = alloc(sizeof(struct foo), ...)
+
+  should be::
+
+    p = alloc(sizeof(*p), ...)
+
+:ALLOC_WITH_MULTIPLY:
+  Prefer kmalloc_array/kcalloc over kmalloc/kzalloc with a
+  sizeof multiply.
+  Ref: `Documentation/core-api/memory-allocation.rst`
+
+:ARCH_DEFINES:
+  Architecture specific defines should be avoided wherever
+  possible.
+
+:ARCH_INCLUDE_LINUX:
+  Whenever asm/file.h is included and linux/file.h exists, a
+  conversion can be made when linux/file.h includes asm/file.h.
+  However this is not always the case (See signal.h).
+  This message type is emitted only for includes from arch/.
+
+:ARRAY_SIZE:
+  The ARRAY_SIZE(foo) macro should be preferred over
+  sizeof(foo)/sizeof(foo[0]) for finding number of elements in an
+  array.
+
+  The macro is defined in include/linux/kernel.h::
+
+    #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+
+:ASSIGNMENT_CONTINUATIONS:
+  Assignment operators should not be written at the start of a
+  line but should follow the operand at the previous line.
+
+:ASSIGN_IN_IF:
+  Do not use assignments in if condition.
+  Example::
+
+    if ((foo = bar(...)) < BAZ) {
+
+  should be written as::
+
+    foo = bar(...);
+    if (foo < BAZ) {
+
+:AVOID_BUG:
+  BUG() or BUG_ON() should be avoided totally.
+  Use WARN() and WARN_ON() instead, and handle the "impossible"
+  error condition as gracefully as possible.
+  Ref: `Documentation/process/deprecated.rst`
+
+:AVOID_EXTERNS:
+  Function prototypes don't need to be declared extern in .h
+  files.  It's assumed by the compiler and is unnecessary.
+
+:AVOID_L_PREFIX:
+  Local symbol names that are prefixed with `.L` should be avoided,
+  as this has special meaning for the assembler; a symbol entry will
+  not be emitted into the symbol table.  This can prevent `objtool`
+  from generating correct unwind info.
+
+  Symbols with STB_LOCAL binding may still be used, and `.L` prefixed
+  local symbol names are still generally usable within a function,
+  but `.L` prefixed local symbol names should not be used to denote
+  the beginning or end of code regions via
+  `SYM_CODE_START_LOCAL`/`SYM_CODE_END`
+
+:BAD_SIGN_OFF:
+  The signed-off-by line does not fall in line with the standards
+  specified by the community.
+  Ref: `Documentation/process/submitting-patches.rst`
+
+:BAD_STABLE_ADDRESS_STYLE:
+  The email format for stable is incorrect.
+  Some valid options for stable address are::
+
+    1. stable@vger.kernel.org
+    2. stable@kernel.org
+
+  For adding version info, the following comment style should be used::
+
+    stable@vger.kernel.org # version info
+
+:BIT_MACRO:
+  Defines like: 1 << <digit> could be BIT(digit).
+  The BIT() macro is defined in include/linux/bitops.h::
+
+    #define BIT(nr)         (1UL << (nr))
+
+:BLOCK_COMMENT_STYLE:
+  The comment style is incorrect.  The preferred style for multi-
+  line comments is::
+
+    /*
+     * This is the preferred style
+     * for multi line comments.
+     */
+
+  The networking comment style is a bit different, with the first line
+  not empty like the former::
+
+    /* This is the preferred comment style
+     * for files in net/ and drivers/net/
+     */
+
+  Ref: `Documentation/process/coding-style.rst`
+
+:BOOL_COMPARISON:
+  Comparisons of A to true and false are better written
+  as A and !A.
+  Ref: `https://lore.kernel.org/lkml/1365563834.27174.12.camel@joe-AO722/`
+
+:BRACES:
+  The placement of braces is stylistically incorrect.
+  The preferred way is to put the opening brace last on the line,
+  and put the closing brace first::
+
+    if (x is true) {
+      we do y
+    }
+
+  This applies for all non-functional blocks.
+  However, there is one special case, namely functions: they have the
+  opening brace at the beginning of the next line, thus::
+
+    int function(int x)
+    {
+      body of function
+    }
+
+  Ref: `Documentation/process/coding-style.rst Section 3`
+
+:BRACKET_SPACE:
+  Whitespace before opening bracket '[' is prohibited.
+  There are some exceptions:
+
+  1. With a type on the left::
+
+     ;int [] a;
+
+  2. At the beginning of a line for slice initialisers::
+
+     [0...10] = 5,
+
+  3. Inside a curly brace::
+
+     = { [0...10] = 5 }
+
+:C99_COMMENTS:
+  C99 style single line comments (//) should not be used.
+  Prefer the block comment style instead.
+  Ref: `Documentation/process/coding-style.rst Section 8`
+
+:CAMELCASE:
+  Avoid CamelCase Identifiers.  snake_case can be an
+  alternative.
+
+:CODE_INDENT:
+  Code indent should use tabs instead of spaces.
+  Outside of comments, documentation and Kconfig,
+  spaces are never used for indentation.
+  Ref: `Documentation/process/coding-style.rst Section 1`
+
+:COMMIT_COMMENT_SYMBOL:
+  Commit log lines starting with a '#' are ignored by git as
+  comments.  To solve this problem addition of a single space
+  infront of the log line is enough.
+
+:COMMIT_MESSAGE:
+  The patch is missing a commit description.  A brief
+  description of the changes made by the patch should be added.
+  Ref: `Documentation/process/submitting-patches.rst`
+
+:COMPARISON_TO_NULL:
+  Comparisons to NULL in the form (foo == NULL) or (foo != NULL)
+  are better written as (!foo) and (foo).
+
+:COMPLEX_MACRO:
+  Macros with complex values should be enclosed within parentheses.
+  Consider::
+
+    #define SOME_MACRO IS_ENABLED(CONFIG_XX) ? 1 : 0
+
+  This can be better written as::
+
+    #define SOME_MACRO (IS_ENABLED(CONFIG_XX) ? 1 : 0)
+
+:CONCATENATED_STRING:
+  Concatenated elements should have a space in between.
+  Example::
+
+    printk(KERN_INFO"bar");
+
+  should be::
+
+    printk(KERN_INFO "bar");
+
+:CONFIG_DESCRIPTION:
+  Kconfig symbols should have a help text which fully describes
+  it.
+
+:CONSIDER_KSTRTO:
+  The simple_strtol(), simple_strtoll(), simple_strtoul(), and
+  simple_strtoull() functions explicitly ignore overflows, which
+  may lead to unexpected results in callers.  The respective kstrtol(),
+  kstrtoll(), kstrtoul(), and kstrtoull() functions tend to be the
+  correct replacements.
+  Ref: `Documentation/process/deprecated.rst`
+
+:CONSTANT_COMPARISON:
+  Comparisons with a constant or upper case identifier on the left
+  side of the test should be avoided.
+
+:LINE_SPACING:
+  Vertical space is wasted given the limited number of lines an
+  editor window can display when multiple blank lines are used.
+  Ref: `Documentation/process/coding-style.rst Section 3.1`
+
+: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.
+  ref: `Documentation/process/submitting-patches.rst`
+
+:NO_AUTHOR_SIGN_OFF:
+  The author of the patch has not signed off the patch.  It is
+  required that a simple sign off line should be present at the
+  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.
+
+:TRAILING_WHITESPACE:
+  Trailing whitespace should always be removed.
+  Some editors highlight the trailing whitespace and cause visual
+  distractions when editing files.
-- 
2.30.0

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

[PATCH RFC v3 3/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

Link the checkpatch documentation to the dev-tools index
for sphinx.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/index.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 1b1cf4f5c9d9..43d28998118b 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -14,6 +14,7 @@ whole; patches welcome!
 .. toctree::
    :maxdepth: 2
 
+   checkpatch
    coccinelle
    sparse
    kcov
-- 
2.30.0

[Linux-kernel-mentees] [PATCH RFC v3 3/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

Link the checkpatch documentation to the dev-tools index
for sphinx.

Signed-off-by: Dwaipayan Ray <dwaipayanray1@gmail.com>
---
 Documentation/dev-tools/index.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 1b1cf4f5c9d9..43d28998118b 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -14,6 +14,7 @@ whole; patches welcome!
 .. toctree::
    :maxdepth: 2
 
+   checkpatch
    coccinelle
    sparse
    kcov
-- 
2.30.0

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

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Matthew Wilcox]

On Sat, Feb 13, 2021 at 06:45:12PM +0530, Dwaipayan Ray wrote:
> +Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style

It's quite amusing that this patch contains lines > 80 columns.

Also patches 2 & 3 can be combined into a single patch.

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Matthew Wilcox]

On Sat, Feb 13, 2021 at 06:45:12PM +0530, Dwaipayan Ray wrote:
> +Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style

It's quite amusing that this patch contains lines > 80 columns.

Also patches 2 & 3 can be combined into a single patch.

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

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Joe Perches]

On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> Add documentation for kernel script checkpatch.pl.
> This documentation is also parsed by checkpatch to
> enable a verbose mode.
> 
> The message types in checkpatch are documented with rst
> field lists. A total of 33 checkpatch type descriptions
> are added.

Alphabetic ordering isn't that great for these entries.
Please group them by use:

whitespace/code layout style:
SPACING, TRAILING_WHITESPACE, LINE_SPACING

commit message defects:
BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL, COMMIT_MESSAGE

Allocation style:
group: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY

> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
[]
> +4 Type Descriptions
> +-------------------
> +
> +This section contains a description of all the message types in checkpatch.
> +
> +.. Types in this section are also parsed by checkpatch.
> +.. Please keep the types sorted alphabetically.
> +
> +:ALLOC_ARRAY_ARGS:
> +  The first argument for kcalloc or kmalloc_array should be the
> +  number of elements.  sizeof() as the first argument is generally
> +  wrong.

If you look at the generated .html file, the output format  is poor.

It would probably be better to use
**<TYPE>**
for each of these blocks instead of
:<TYPE>:

and update the script appropriately.

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Joe Perches]

On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> Add documentation for kernel script checkpatch.pl.
> This documentation is also parsed by checkpatch to
> enable a verbose mode.
> 
> The message types in checkpatch are documented with rst
> field lists. A total of 33 checkpatch type descriptions
> are added.

Alphabetic ordering isn't that great for these entries.
Please group them by use:

whitespace/code layout style:
SPACING, TRAILING_WHITESPACE, LINE_SPACING

commit message defects:
BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL, COMMIT_MESSAGE

Allocation style:
group: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY

> diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
[]
> +4 Type Descriptions
> +-------------------
> +
> +This section contains a description of all the message types in checkpatch.
> +
> +.. Types in this section are also parsed by checkpatch.
> +.. Please keep the types sorted alphabetically.
> +
> +:ALLOC_ARRAY_ARGS:
> +  The first argument for kcalloc or kmalloc_array should be the
> +  number of elements.  sizeof() as the first argument is generally
> +  wrong.

If you look at the generated .html file, the output format  is poor.

It would probably be better to use
**<TYPE>**
for each of these blocks instead of
:<TYPE>:

and update the script appropriately.

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

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > Add documentation for kernel script checkpatch.pl.
> > This documentation is also parsed by checkpatch to
> > enable a verbose mode.
> >
> > The message types in checkpatch are documented with rst
> > field lists. A total of 33 checkpatch type descriptions
> > are added.
>
> Alphabetic ordering isn't that great for these entries.
> Please group them by use:
>
> whitespace/code layout style:
> SPACING, TRAILING_WHITESPACE, LINE_SPACING
>
> commit message defects:
> BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL, COMMIT_MESSAGE
>
> Allocation style:
> group: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
>
> > diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> []
> > +4 Type Descriptions
> > +-------------------
> > +
> > +This section contains a description of all the message types in checkpatch.
> > +
> > +.. Types in this section are also parsed by checkpatch.
> > +.. Please keep the types sorted alphabetically.
> > +
> > +:ALLOC_ARRAY_ARGS:
> > +  The first argument for kcalloc or kmalloc_array should be the
> > +  number of elements.  sizeof() as the first argument is generally
> > +  wrong.
>
> If you look at the generated .html file, the output format  is poor.
>
> It would probably be better to use
> **<TYPE>**
> for each of these blocks instead of
> :<TYPE>:
>
> and update the script appropriately.

Thanks, I will do these.

Also someone pointed out in the list that some lines in the patch contained > 80
columns. Checkpatch doesn't generate any warning for that. Is it something that
could be added to checkpatch or was it decided against at some point?

Thanks,
Dwaipayan.

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > Add documentation for kernel script checkpatch.pl.
> > This documentation is also parsed by checkpatch to
> > enable a verbose mode.
> >
> > The message types in checkpatch are documented with rst
> > field lists. A total of 33 checkpatch type descriptions
> > are added.
>
> Alphabetic ordering isn't that great for these entries.
> Please group them by use:
>
> whitespace/code layout style:
> SPACING, TRAILING_WHITESPACE, LINE_SPACING
>
> commit message defects:
> BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL, COMMIT_MESSAGE
>
> Allocation style:
> group: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
>
> > diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> []
> > +4 Type Descriptions
> > +-------------------
> > +
> > +This section contains a description of all the message types in checkpatch.
> > +
> > +.. Types in this section are also parsed by checkpatch.
> > +.. Please keep the types sorted alphabetically.
> > +
> > +:ALLOC_ARRAY_ARGS:
> > +  The first argument for kcalloc or kmalloc_array should be the
> > +  number of elements.  sizeof() as the first argument is generally
> > +  wrong.
>
> If you look at the generated .html file, the output format  is poor.
>
> It would probably be better to use
> **<TYPE>**
> for each of these blocks instead of
> :<TYPE>:
>
> and update the script appropriately.

Thanks, I will do these.

Also someone pointed out in the list that some lines in the patch contained > 80
columns. Checkpatch doesn't generate any warning for that. Is it something that
could be added to checkpatch or was it decided against at some point?

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

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Joe Perches]

On Mon, 2021-02-15 at 21:20 +0530, Dwaipayan Ray wrote:

> Also someone pointed out in the list that some lines in the patch contained > 80
> columns. Checkpatch doesn't generate any warning for that. Is it something that
> could be added to checkpatch or was it decided against at some point?

decided against.
checkpatch isn't useful for .rst files.

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Joe Perches]

On Mon, 2021-02-15 at 21:20 +0530, Dwaipayan Ray wrote:

> Also someone pointed out in the list that some lines in the patch contained > 80
> columns. Checkpatch doesn't generate any warning for that. Is it something that
> could be added to checkpatch or was it decided against at some point?

decided against.
checkpatch isn't useful for .rst files.


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

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Joe Perches]

On Sun, 2021-02-14 at 12:15 +0000, Matthew Wilcox wrote:
> On Sat, Feb 13, 2021 at 06:45:12PM +0530, Dwaipayan Ray wrote:
> > +Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style
> 
> It's quite amusing that this patch contains lines > 80 columns.

Then you could amuse yourself further by looking at the existing
line lengths of .rst files.

$ git ls-files -- '*.rst' | \
  xargs cat | \
  awk '{print length($0);}' | \
  sort -n | \
  uniq -c | \
  tail -20
      2 226
      1 230
      1 233
      1 234
     48 246
      1 253
      2 257
      1 263
      1 270
      1 275
      1 276
      1 293
      1 294
      1 308
      2 324
      1 359
      1 360
      5 369
      1 370
      2 409

Other than testing whether or not an SPDX license line exists,
checkpatch doesn't inspect .rst files.

There are better tools for that.

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Joe Perches]

On Sun, 2021-02-14 at 12:15 +0000, Matthew Wilcox wrote:
> On Sat, Feb 13, 2021 at 06:45:12PM +0530, Dwaipayan Ray wrote:
> > +Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style
> 
> It's quite amusing that this patch contains lines > 80 columns.

Then you could amuse yourself further by looking at the existing
line lengths of .rst files.

$ git ls-files -- '*.rst' | \
  xargs cat | \
  awk '{print length($0);}' | \
  sort -n | \
  uniq -c | \
  tail -20
      2 226
      1 230
      1 233
      1 234
     48 246
      1 253
      2 257
      1 263
      1 270
      1 275
      1 276
      1 293
      1 294
      1 308
      2 324
      1 359
      1 360
      5 369
      1 370
      2 409

Other than testing whether or not an SPDX license line exists,
checkpatch doesn't inspect .rst files.

There are better tools for that.


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

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Matthew Wilcox]

On Mon, Feb 15, 2021 at 10:11:03AM -0800, Joe Perches wrote:
> On Sun, 2021-02-14 at 12:15 +0000, Matthew Wilcox wrote:
> > On Sat, Feb 13, 2021 at 06:45:12PM +0530, Dwaipayan Ray wrote:
> > > +Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style
> > 
> > It's quite amusing that this patch contains lines > 80 columns.
> 
> Then you could amuse yourself further by looking at the existing
> line lengths of .rst files.

There are good reasons for rst lines to be longer than 80 columns,
but there weren't any in this case.

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Matthew Wilcox]

On Mon, Feb 15, 2021 at 10:11:03AM -0800, Joe Perches wrote:
> On Sun, 2021-02-14 at 12:15 +0000, Matthew Wilcox wrote:
> > On Sat, Feb 13, 2021 at 06:45:12PM +0530, Dwaipayan Ray wrote:
> > > +Checkpatch (scripts/checkpatch.pl) is a perl script which checks for trivial style
> > 
> > It's quite amusing that this patch contains lines > 80 columns.
> 
> Then you could amuse yourself further by looking at the existing
> line lengths of .rst files.

There are good reasons for rst lines to be longer than 80 columns,
but there weren't any in this case.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > Add documentation for kernel script checkpatch.pl.
> > This documentation is also parsed by checkpatch to
> > enable a verbose mode.
> >
> > The message types in checkpatch are documented with rst
> > field lists. A total of 33 checkpatch type descriptions
> > are added.
>
> Alphabetic ordering isn't that great for these entries.
> Please group them by use:
>
> whitespace/code layout style:
> SPACING, TRAILING_WHITESPACE, LINE_SPACING
>
> commit message defects:
> BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL, COMMIT_MESSAGE
>
> Allocation style:
> group: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
>
> > diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> []
> > +4 Type Descriptions
> > +-------------------
> > +
> > +This section contains a description of all the message types in checkpatch.
> > +
> > +.. Types in this section are also parsed by checkpatch.
> > +.. Please keep the types sorted alphabetically.
> > +
> > +:ALLOC_ARRAY_ARGS:
> > +  The first argument for kcalloc or kmalloc_array should be the
> > +  number of elements.  sizeof() as the first argument is generally
> > +  wrong.
>
> If you look at the generated .html file, the output format  is poor.
>
> It would probably be better to use
> **<TYPE>**
> for each of these blocks instead of
> :<TYPE>:
>
> and update the script appropriately.
>

Hi,
Could I get some comment on this grouping for types:

Allocation Style: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY

API Usage: ARCH_DEFINES, ARCH_INCLUDE_LINUX, ARRAY_SIZE, AVOID_BUG,
                    AVOID_EXTERNS, AVOID_L_PREFIX, BIT_MACRO, CONSIDER_KSTRTO

Comment Style: BLOCK_COMMENT_STYLE, C99_COMMENTS

Commit Message: BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL,
                              COMMIT_MESSAGE, MISSING_SIGN_OFF,
NO_AUTHOR_SIGN_OFF

Comparison Style: ASSIGN_IN_IF, BOOL_COMPARISON, COMPARISON_TO_NULL,
                               CONSTANT_COMPARISON

Spacing & Brackets: ASSIGNMENT_CONTINUATIONS, BRACES, BRACKET_SPACE,
                                  CODE_INDENT, CONCATENATED_STRING,
LINE_SPACING,
                                  TRAILING_WHITESPACE

Others: CAMELCASE, CONFIG_DESCRIPTION

This is what I have done till now. Any suggestions would be nice and if it looks
okay I would like to send the v4 in.

Thanks & Regards,
Dwaipayan.

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
>
> On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > Add documentation for kernel script checkpatch.pl.
> > This documentation is also parsed by checkpatch to
> > enable a verbose mode.
> >
> > The message types in checkpatch are documented with rst
> > field lists. A total of 33 checkpatch type descriptions
> > are added.
>
> Alphabetic ordering isn't that great for these entries.
> Please group them by use:
>
> whitespace/code layout style:
> SPACING, TRAILING_WHITESPACE, LINE_SPACING
>
> commit message defects:
> BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL, COMMIT_MESSAGE
>
> Allocation style:
> group: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
>
> > diff --git a/Documentation/dev-tools/checkpatch.rst b/Documentation/dev-tools/checkpatch.rst
> []
> > +4 Type Descriptions
> > +-------------------
> > +
> > +This section contains a description of all the message types in checkpatch.
> > +
> > +.. Types in this section are also parsed by checkpatch.
> > +.. Please keep the types sorted alphabetically.
> > +
> > +:ALLOC_ARRAY_ARGS:
> > +  The first argument for kcalloc or kmalloc_array should be the
> > +  number of elements.  sizeof() as the first argument is generally
> > +  wrong.
>
> If you look at the generated .html file, the output format  is poor.
>
> It would probably be better to use
> **<TYPE>**
> for each of these blocks instead of
> :<TYPE>:
>
> and update the script appropriately.
>

Hi,
Could I get some comment on this grouping for types:

Allocation Style: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY

API Usage: ARCH_DEFINES, ARCH_INCLUDE_LINUX, ARRAY_SIZE, AVOID_BUG,
                    AVOID_EXTERNS, AVOID_L_PREFIX, BIT_MACRO, CONSIDER_KSTRTO

Comment Style: BLOCK_COMMENT_STYLE, C99_COMMENTS

Commit Message: BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL,
                              COMMIT_MESSAGE, MISSING_SIGN_OFF,
NO_AUTHOR_SIGN_OFF

Comparison Style: ASSIGN_IN_IF, BOOL_COMPARISON, COMPARISON_TO_NULL,
                               CONSTANT_COMPARISON

Spacing & Brackets: ASSIGNMENT_CONTINUATIONS, BRACES, BRACKET_SPACE,
                                  CODE_INDENT, CONCATENATED_STRING,
LINE_SPACING,
                                  TRAILING_WHITESPACE

Others: CAMELCASE, CONFIG_DESCRIPTION

This is what I have done till now. Any suggestions would be nice and if it looks
okay I would like to send the v4 in.

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

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Joe Perches]

On Tue, 2021-02-16 at 19:48 +0530, Dwaipayan Ray wrote:
> On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
> > On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > > Add documentation for kernel script checkpatch.pl.
> > > This documentation is also parsed by checkpatch to
> > > enable a verbose mode.
> > > 
> > > The message types in checkpatch are documented with rst
> > > field lists. A total of 33 checkpatch type descriptions
> > > are added.
> > 
> > Alphabetic ordering isn't that great for these entries.
> > Please group them by use:
> > 
> > whitespace/code layout style:
> > SPACING, TRAILING_WHITESPACE, LINE_SPACING
[]
> Could I get some comment on this grouping for types:
> 
> Allocation Style: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
> 
> API Usage: ARCH_DEFINES, ARCH_INCLUDE_LINUX, ARRAY_SIZE, AVOID_BUG,
>                     AVOID_EXTERNS, AVOID_L_PREFIX, BIT_MACRO, CONSIDER_KSTRTO
> 
> Comment Style: BLOCK_COMMENT_STYLE, C99_COMMENTS
> 
> Commit Message: BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL,
>                               COMMIT_MESSAGE, MISSING_SIGN_OFF,
> NO_AUTHOR_SIGN_OFF
> 
> Comparison Style: ASSIGN_IN_IF, BOOL_COMPARISON, COMPARISON_TO_NULL,
>                                CONSTANT_COMPARISON
> 
> Spacing & Brackets: ASSIGNMENT_CONTINUATIONS, BRACES, BRACKET_SPACE,
>                                   CODE_INDENT, CONCATENATED_STRING,
> LINE_SPACING,
>                                   TRAILING_WHITESPACE
> 
> Others: CAMELCASE, CONFIG_DESCRIPTION
> 
> This is what I have done till now. Any suggestions would be nice and if it looks
> okay I would like to send the v4 in.

Looks OK.

Please make sure you at least include SPACING in the spacing & brackets
descriptions.

It also seems like ref links to Documentation/process/coding-style.rst
<section> (3.1 in the SPACING case) should be used more frequently.

It'd be 'nice' to somehow use sortable tables with some grouping
attribute for these groups, but I have no idea if that's feasible with
.rst restrutured text files.

Perhaps simplify the checkpatch code a bit for the --terse and --verbose
output.

Maybe something like:
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 869d80397f9f..07566cb3b3f8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl

@@ -292,15 +292,16 @@ GetOptions(
 
 help(0) if ($help);
 
+die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
+die "$P: --verbose canot be used with --terse\n" if ($verbose && $terse);
+
 list_types(0) if ($list_types);

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Joe Perches]

On Tue, 2021-02-16 at 19:48 +0530, Dwaipayan Ray wrote:
> On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
> > On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > > Add documentation for kernel script checkpatch.pl.
> > > This documentation is also parsed by checkpatch to
> > > enable a verbose mode.
> > > 
> > > The message types in checkpatch are documented with rst
> > > field lists. A total of 33 checkpatch type descriptions
> > > are added.
> > 
> > Alphabetic ordering isn't that great for these entries.
> > Please group them by use:
> > 
> > whitespace/code layout style:
> > SPACING, TRAILING_WHITESPACE, LINE_SPACING
[]
> Could I get some comment on this grouping for types:
> 
> Allocation Style: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
> 
> API Usage: ARCH_DEFINES, ARCH_INCLUDE_LINUX, ARRAY_SIZE, AVOID_BUG,
>                     AVOID_EXTERNS, AVOID_L_PREFIX, BIT_MACRO, CONSIDER_KSTRTO
> 
> Comment Style: BLOCK_COMMENT_STYLE, C99_COMMENTS
> 
> Commit Message: BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL,
>                               COMMIT_MESSAGE, MISSING_SIGN_OFF,
> NO_AUTHOR_SIGN_OFF
> 
> Comparison Style: ASSIGN_IN_IF, BOOL_COMPARISON, COMPARISON_TO_NULL,
>                                CONSTANT_COMPARISON
> 
> Spacing & Brackets: ASSIGNMENT_CONTINUATIONS, BRACES, BRACKET_SPACE,
>                                   CODE_INDENT, CONCATENATED_STRING,
> LINE_SPACING,
>                                   TRAILING_WHITESPACE
> 
> Others: CAMELCASE, CONFIG_DESCRIPTION
> 
> This is what I have done till now. Any suggestions would be nice and if it looks
> okay I would like to send the v4 in.

Looks OK.

Please make sure you at least include SPACING in the spacing & brackets
descriptions.

It also seems like ref links to Documentation/process/coding-style.rst
<section> (3.1 in the SPACING case) should be used more frequently.

It'd be 'nice' to somehow use sortable tables with some grouping
attribute for these groups, but I have no idea if that's feasible with
.rst restrutured text files.

Perhaps simplify the checkpatch code a bit for the --terse and --verbose
output.

Maybe something like:
---
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 869d80397f9f..07566cb3b3f8 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl

@@ -292,15 +292,16 @@ GetOptions(
 
 help(0) if ($help);
 
+die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
+die "$P: --verbose canot be used with --terse\n" if ($verbose && $terse);
+
 list_types(0) if ($list_types);


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

Re: [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

On Wed, Feb 17, 2021 at 4:07 PM Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2021-02-16 at 19:48 +0530, Dwaipayan Ray wrote:
> > On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
> > > On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > > > Add documentation for kernel script checkpatch.pl.
> > > > This documentation is also parsed by checkpatch to
> > > > enable a verbose mode.
> > > >
> > > > The message types in checkpatch are documented with rst
> > > > field lists. A total of 33 checkpatch type descriptions
> > > > are added.
> > >
> > > Alphabetic ordering isn't that great for these entries.
> > > Please group them by use:
> > >
> > > whitespace/code layout style:
> > > SPACING, TRAILING_WHITESPACE, LINE_SPACING
> []
> > Could I get some comment on this grouping for types:
> >
> > Allocation Style: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
> >
> > API Usage: ARCH_DEFINES, ARCH_INCLUDE_LINUX, ARRAY_SIZE, AVOID_BUG,
> >                     AVOID_EXTERNS, AVOID_L_PREFIX, BIT_MACRO, CONSIDER_KSTRTO
> >
> > Comment Style: BLOCK_COMMENT_STYLE, C99_COMMENTS
> >
> > Commit Message: BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL,
> >                               COMMIT_MESSAGE, MISSING_SIGN_OFF,
> > NO_AUTHOR_SIGN_OFF
> >
> > Comparison Style: ASSIGN_IN_IF, BOOL_COMPARISON, COMPARISON_TO_NULL,
> >                                CONSTANT_COMPARISON
> >
> > Spacing & Brackets: ASSIGNMENT_CONTINUATIONS, BRACES, BRACKET_SPACE,
> >                                   CODE_INDENT, CONCATENATED_STRING,
> > LINE_SPACING,
> >                                   TRAILING_WHITESPACE
> >
> > Others: CAMELCASE, CONFIG_DESCRIPTION
> >
> > This is what I have done till now. Any suggestions would be nice and if it looks
> > okay I would like to send the v4 in.
>
> Looks OK.
>
> Please make sure you at least include SPACING in the spacing & brackets
> descriptions.
>
> It also seems like ref links to Documentation/process/coding-style.rst
> <section> (3.1 in the SPACING case) should be used more frequently.
>
> It'd be 'nice' to somehow use sortable tables with some grouping
> attribute for these groups, but I have no idea if that's feasible with
> .rst restrutured text files.
>

I too didn't find anything like that. Seems like static tables are the most
that's feasible with sphynx. But here it might not look well.

> Perhaps simplify the checkpatch code a bit for the --terse and --verbose
> output.
>
> Maybe something like:
> ---
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 869d80397f9f..07566cb3b3f8 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
>
> @@ -292,15 +292,16 @@ GetOptions(
>
>  help(0) if ($help);
>
> +die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
> +die "$P: --verbose canot be used with --terse\n" if ($verbose && $terse);
> +
>  list_types(0) if ($list_types);
>

Sure will do this. Sending you the updated patch in a while.

Thanks!
--Dwaipayan

Re: [Linux-kernel-mentees] [PATCH RFC v3 2/3] docs: add documentation for checkpatch

[Dwaipayan Ray]

On Wed, Feb 17, 2021 at 4:07 PM Joe Perches <joe@perches.com> wrote:
>
> On Tue, 2021-02-16 at 19:48 +0530, Dwaipayan Ray wrote:
> > On Sun, Feb 14, 2021 at 10:27 PM Joe Perches <joe@perches.com> wrote:
> > > On Sat, 2021-02-13 at 18:45 +0530, Dwaipayan Ray wrote:
> > > > Add documentation for kernel script checkpatch.pl.
> > > > This documentation is also parsed by checkpatch to
> > > > enable a verbose mode.
> > > >
> > > > The message types in checkpatch are documented with rst
> > > > field lists. A total of 33 checkpatch type descriptions
> > > > are added.
> > >
> > > Alphabetic ordering isn't that great for these entries.
> > > Please group them by use:
> > >
> > > whitespace/code layout style:
> > > SPACING, TRAILING_WHITESPACE, LINE_SPACING
> []
> > Could I get some comment on this grouping for types:
> >
> > Allocation Style: ALLOC_ARRAY_ARGS, ALLOC_SIZEOF_STRUCT, ALLOC_WITH_MULTIPLY
> >
> > API Usage: ARCH_DEFINES, ARCH_INCLUDE_LINUX, ARRAY_SIZE, AVOID_BUG,
> >                     AVOID_EXTERNS, AVOID_L_PREFIX, BIT_MACRO, CONSIDER_KSTRTO
> >
> > Comment Style: BLOCK_COMMENT_STYLE, C99_COMMENTS
> >
> > Commit Message: BAD_SIGN_OFF, BAD_STABLE_ADDRESS_STYLE, COMMIT_COMMENT_SYMBOL,
> >                               COMMIT_MESSAGE, MISSING_SIGN_OFF,
> > NO_AUTHOR_SIGN_OFF
> >
> > Comparison Style: ASSIGN_IN_IF, BOOL_COMPARISON, COMPARISON_TO_NULL,
> >                                CONSTANT_COMPARISON
> >
> > Spacing & Brackets: ASSIGNMENT_CONTINUATIONS, BRACES, BRACKET_SPACE,
> >                                   CODE_INDENT, CONCATENATED_STRING,
> > LINE_SPACING,
> >                                   TRAILING_WHITESPACE
> >
> > Others: CAMELCASE, CONFIG_DESCRIPTION
> >
> > This is what I have done till now. Any suggestions would be nice and if it looks
> > okay I would like to send the v4 in.
>
> Looks OK.
>
> Please make sure you at least include SPACING in the spacing & brackets
> descriptions.
>
> It also seems like ref links to Documentation/process/coding-style.rst
> <section> (3.1 in the SPACING case) should be used more frequently.
>
> It'd be 'nice' to somehow use sortable tables with some grouping
> attribute for these groups, but I have no idea if that's feasible with
> .rst restrutured text files.
>

I too didn't find anything like that. Seems like static tables are the most
that's feasible with sphynx. But here it might not look well.

> Perhaps simplify the checkpatch code a bit for the --terse and --verbose
> output.
>
> Maybe something like:
> ---
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 869d80397f9f..07566cb3b3f8 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
>
> @@ -292,15 +292,16 @@ GetOptions(
>
>  help(0) if ($help);
>
> +die "$P: --git cannot be used with --file or --fix\n" if ($git && ($file || $fix));
> +die "$P: --verbose canot be used with --terse\n" if ($verbose && $terse);
> +
>  list_types(0) if ($list_types);
>

Sure will do this. Sending you the updated patch in a while.

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