This allows to use a one-character expression inside the 'if'
statement and multiple spaces before the line continuation character.
Fixes false positive in case like this:
#define MACRO(ARG) \
if (a) { \
do_work(ARG); \
}
Fixes: 16770c6d9179 ("checkpatch: support macro continuation")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Aaron Conole <aconole@redhat.com>
OVS has defines for loops like 'BITMAP_FOR_EACH_1' or
'ULLONG_FOR_EACH_1', but the regexp in checkpatch doesn't match with
numbers and skips these loops while checking.
This patch adds numbers into regexp and adds some FER_EACH loops to
the unit tests.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
Unlike manual splitting, 'splitlines' correctly handles different
line endings. Without this change script fails to check files with
'\r\n' endings treating the whole patch as a header.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Using python `sys.exit(-1)` on Windows produces mixed results.
Let's take the following results from different shells:
CMD
>python -c "import sys; sys.exit(-1)" & echo %errorlevel%
1
MSYS
$ python -c "import sys; sys.exit(-1)" && echo $?
0
WSL
$ python -c "import sys; sys.exit(-1)"; echo $?
255
this results in the following tests to fail:
checkpatch
10: checkpatch - sign-offs FAILED (checkpatch.at:32)
11: checkpatch - parenthesized constructs FAILED (checkpatch.at:32)
12: checkpatch - parenthesized constructs - for FAILED (checkpatch.at:32)
13: checkpatch - comments FAILED (checkpatch.at:32)
because of:
./checkpatch.at:32: exit code was 0, expected 255
This patch introduces a positive constant for the default exit code (1)
similar to other OVS utilities.
Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
' -(' matches a single character in the range between ' ' (index 32)
and '(' (index 40). This leads to the false positive:
WARNING: Line lacks whitespace around operator
#445 FILE: ovsdb/monitor.c:573:
if (--mcs->n_refs == 0) {
Need to escape '-' to have a right behaviour.
This patch additionally escapes all other '-' chars in the similar
regexes and makes them be one per line to ease the review in case of
future changes.
Basic unit tests added.
CC: Joe Stringer <joe@ovn.org>
Fixes: 0d7b16daea50 ("checkpatch: Check for infix operator whitespace.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Current checkpatch rules matches only OVS 'FOR_EACH' loops.
This change will apply same style checks for DPDK iterators
like 'RTE_ETH_FOREACH_MATCHING_DEV () {}'.
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
It appears that Python silently treats invalid escape sequences in
strings as literals, e.g. "\." is the same as "\\.". Newer versions of
checkpatch complain, and it does seem reasonable to me to fix these.
Acked-by: Numan Siddique <nusiddiq@redhat.com>
Tested-by: Numan Siddique <nusiddiq@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
A typographical error in a prompt for missing python enchant library is
identified and fixed.
Signed-off-by: Bala Sankaran <bsankara@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
On my machine it takes almost a second for enchant to read its dictionary.
This time is wasted when spell checking is not enabled. This commit makes
checkpatch read the dictionary only when it will be used.
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Somehow some such patches snuck through. checkpatch caught them (and the
committer missed that) but this makes it even more explicit.
Reviewed-by: Yifeng Sun <pkusunyifeng@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Added a test to identify a macro and skip
printing errors if the condition or loop
is part of a macro.
Additional tests are added to checkpatch
testsuite that cover conditionals and
loop constructs.
Signed-off-by: Bala Sankaran <bsankara@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
This also makes a start at a testsuite for checkpatch.
CC: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@bytheb.org>
void functions do not need to have a return statement, because
such statements are redundant. Warn the user of such instances.
An interim line check is added to allow gathering additional
context for each line that is being processed.
Signed-off-by: Bala Sankaran <bsankara@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
The separator line always starts with three dashes on a line, optionally
followed by either white-space, OR a single space and a filename. The
regex would previously match on any three dashes in a row. This means
that a patch (such as [1]) would trigger the parser state machine to
advance beyond the signed-off checks.
Now, bound the check only to use what git-mailinfo would use as a
separator.
--- <filename>
---<sp>
1: https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348625.html
Fixes: c599d5ccf316 ("checkpatch.py: A simple script for finding patch issues")
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Formatted patches can contain a heirarchy of sign-offs. This is true when
merging patches from different projects (eg. backports to the datapath
directory from the linux net project).
This means that a submitted backport will contain multiple signed-off
tags, and not all should be considered.
This commit updates checkpatch to only consider those signoff lines which
start at the beginning of a line. So the following:
Signed-off-by: Foo Bar <foo@bar.com>
should not trigger.
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
This allows scripts which only want to process error messages to silence
the normal 'warm and fuzzy' status messages from checkpatch.
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Until now checkpatch warnings have not said how long a too-long line is
or what word might be misspelled. This commit makes the messages more
explicit.
To do this the 'print' functions needed to know the line that was in error.
One way to do that was to also pass the line in question to the 'print'
function. I decided instead to just allow the 'print' function to be
missing and to instead issue these warnings from the 'check' function. I
don't know whether this design raises any red flags with anyone.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
The .match() method only matches at the beginning of a string but the
blacklists here need to match anywhere in a string.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
Acked-by: Numan Siddique <nusiddiq@redhat.com>
When a new rst document is added under Documentation, check if the
new file is added to the proper index.rst and to the automake.mk.
Signed-off-by: Flavio Leitner <fbl@sysclose.org>
Signed-off-by: Ben Pfaff <blp@ovn.org>
The regular expression here would flag any slash that wasn't adjacent to
an asterisk as missing whitespace. This fixes the problem.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Tested-by: Aaron Conole <aconole@redhat.com>
Grow a new opt-in feature to check comments for possible spelling
mistakes. Uses the 'enchant' library to provide a default link to
aspell/ispell as the backend.
Additionally, a custom set of kewords is included inline to match what
would be possibly encountered in 'the wild'. The list is fairly
comprehensive at this point.
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
For the infix operator whitespace checks, some of these operators are
used within comments. In those cases, it probably doesn't make sense
to warn about whitespacing.
There may be other checks that could use this kind of filter, but
that can wait for a future commit (and someone ambitious enough to
test each case).
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
checkpatch would sometimes confuse comment markers for operators. This
fixes the problem.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
"xxx" is often used to indicate items that the developer wanted to look
at again before committing. Flag those as a warning.
Signed-off-by: Justin Pettit <jpettit@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
Lines should be counted for each file separately.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
The coding style states that BSD-style brace placement should be used,
and even single statements should be enclosed. Add checks to checkpatch
for this, particularly for 'else' statements.
Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
The style guide states that lines should not end with '?' or ':'. Check
for this and report an error.
Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
Most of the prerequisite checks so far matched on filenames that ended
in some character followed by 'c' or 'h', rather than a filename that
ends in '.c' or '.h'. Fix this.
Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
The 'Expressions' section of the coding style specifies that one space
should be on either side of infix binary and ternary operators. This
adds a check to checkpatch.py for most of these.
The regex won't match if there are speech marks on the line, because
the style should not apply to the contents of strings.
This check is left at warning level because there isn't a good way to
determine whether a line is within a multiline comment or string, so it
will occasionally flag such lines which contain hyphenated words.
Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
It's better to see real commits instead of 'HEAD~n'.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Russell Bryant <russell@ovn.org>
Currently to check more than one patch or file it's required
to invoke script for each file separately.
Fix that by iterating over all the passed filenames.
Note: If '-f' option passed, all the files treated as usual files.
Without '-f' all the files treated as patch files.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Russell Bryant <russell@ovn.org>
Currently, result status printed only for patch files.
It'll be nice to have results for other checking types.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Russell Bryant <russell@ovn.org>
Local Gerrit Change-Ids are not welcome in common repository.
Inspired by checkpatch.pl from Linux Kernel.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Acked-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Russell Bryant <russell@ovn.org>
Suggest the author to use the OVS wrapper of the assert function.
Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
There are three paths for running the core checkpatch path: From a file,
from stdin, or reading from git output. Currently, the file version of
this calls the "email" library's decode routine which translates the
stream into a bytes array, which we later call decode() to turn it back
into a regular string. This works on python2 and python3, but the other
paths don't work in python3 due to the following error:
$ utilities/checkpatch.py -1
== Checking HEAD~0 ==
Traceback (most recent call last):
File "utilities/checkpatch.py", line 491, in <module>
if ovs_checkpatch_parse(patch, revision):
File "utilities/checkpatch.py", line 324, in ovs_checkpatch_parse
for line in text.decode().split('\n'):
AttributeError: 'str' object has no attribute 'decode'
Rather than performing this extra encode/decode, strip these out from
this path so that the stdin and git variants of checkpatch can work in
python3.
Signed-off-by: Joe Stringer <joe@ovn.org>
Acked-by: Ben Pfaff <blp@ovn.org>
'range(n_patches, 0, -1)' generates list starting from 'n_patches'
and not including zero. This leads to checking of N most recent
commits starting from the second one.
New version will generate right list starting from 'n_patches - 1'
and including zero. So, the most recent commit (HEAD~0) will be
checked and desired behavior will be achieved.
Also, 'reversed' looks better than 'range(n_patches - 1, -1, -1)'
Fixes: a1fccabce2cb ("checkpatch: Support checking recent commits in the current repo.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Autotest .at files often have lines with samples of expected output from
various programs, which fairly often includes leading tabs, so this warning
causes false positives there.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Reviewed-by: Aaron Conole <aconole@redhat.com>
Comments are more freeform than code, so this patch tries to ignore many
checks on comment lines. It assumes that any line that begins with "/*"
or "* " is a comment line. (Without a following space, "*" might be
something like "*x = 1;".)
Suggested-by: Joe Stringer <joe@ovn.org>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
A lot of checkpatch warnings are only enabled for particular kinds of
files, e.g. C warnings only apply to C source and header files. The -f
option didn't pass the file name to the code that determines what kinds
of warnings to report, so only generic warnings were actually reported.
This fixes that problem.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
The switch statement and our FOR_EACH macro iteration constructs have the
same rules as if, for, and while.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
