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

Sun Jan 24 16:53:05 UTC 2021

On Sun, Jan 24, 2021 at 10:11 PM Lukas Bulwahn <lukas.bulwahn at gmail.com> wrote:
>
> On Sun, Jan 24, 2021 at 4:19 PM Dwaipayan Ray <dwaipayanray1 at gmail.com> wrote:
> >
> > Add documentation for style checker script checkpatch.pl
> > (scripts/checkpatch.pl).
> >
> > The documentation performs a dual function. At the surface it
> > serves as a documentation for checkpatch and describes the options
> > as well as the messages emitted by checkpatch.
> >
> > The type description messages will be parsed to enable a verbose
> > mode in checkpatch.
> >
> > Signed-off-by: Dwaipayan Ray <dwaipayanray1 at gmail.com>
> > ---
> >  Documentation/tools/checkpatch.rst | 271 +++++++++++++++++++++++++++++
> >  1 file changed, 271 insertions(+)
> >  create mode 100644 Documentation/tools/checkpatch.rst
> >
> > diff --git a/Documentation/tools/checkpatch.rst b/Documentation/tools/checkpatch.rst
> > new file mode 100644
> > index 000000000000..cbf153040de1
> > --- /dev/null
> > +++ b/Documentation/tools/checkpatch.rst
> > @@ -0,0 +1,271 @@
> > +==========
> > +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.
> > +
> > +It should be noted that checkpatch may not be always right.  At times the human
> > +judgement should take preference over what checkpatch has to say.  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.  All information messages are disabled.  Only the
> > +   messages and a summary report is output.
> > +
> > + - --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 at 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...)
> > +
> > +   Strip off messages with the given 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).  On exceeding the given length, a 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 min 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.
> > +
>
> Did you copy this from the current checkpatch.pl --help?
>
> Probably it is best to then refactor checkpatch.pl to just print the
> help text by parsing from the Documentation file or have the
> Documentation just include the output from checkpatch.pl --help here.
>

A part of it is copied actually. And some of the options are more thoroughly
explained with examples like the types and the signoff ones. I think more
examples could be added later on as an entra help.

Maybe it would be better off keeping this separate as checkpatch would take
some time to parse this every time the help message is to be printed?

Apart from that what do you think of it in general?

Thank you,
Dwaipayan.

> > +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.
> > +.. CHECKPATCH_START
> > +
> > +MISSING_SIGN_OFF
> > +
> > +  The patch is missing a Signed-off-by line.  This error must be taken care of
> > +  and a Signed-off-by line should be added according to Developer's certificate of
> > +  Origin.
> > +
> > +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.
> > +
> > +  Sometimes this particular warning can also occur when both email address and name of
> > +  the author do not match the sign off line because checkpatch has no mechanism to say
> > +  if it is the same person.
> > +
> > +  Consider::
> > +
> > +    From: John Doe <john.doe at example.com>
> > +    ...
> > +    Signed-off-by: J. Doe <john at example2.com>
> > +
> > +  While these may point that both the persons are same, checkpatch cannot
> > +  understand that and in such cases this warning can be ignored.
> > +
> > +.. CHECKPATCH_END
> > --
> > 2.30.0
> >