A new check for common mistakes while formatting a 'Fixes:' tag.
Acked-by: Sunil Pai G <sunil.pai.g@intel.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Recently there has been a lot of press about the "trojan source" attack,
where Unicode characters are used to obfuscate the true functionality of
code. This attack didn't effect OVS, but adding the check here will help
guard against it sneaking in later.
Signed-off-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Gaetan Rivet <grive@u256.net>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
As part of some previous checkpatch work, we discovered that checkpatch
isn't always reporting correct line numbers. As it turns out, Python's
splitlines function considers several characters to be new lines which
common text editors do not typically consider to be new lines. For
example, form feed characters, which this code base uses to cluster
functionality.
Signed-off-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Currently, there are some patches with the tags wrongly written (with
space instead of dash ) and this may prevent some automatic system or CI
to detect them correctly.
This commit adds a check in checkpatch to be sure the tag is written
correctly with dash and not with space.
The tags supported by the commit are:
Acked-by, Reported-at, Reported-by, Requested-by, Reviewed-by, Submitted-at
and Suggested-by.
It's not necessary to add "Signed-off-by" since it's already checked in
checkpatch.
Signed-off-by: Timothy Redaelli <tredaelli@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
As Frode Nordahl points out in [0], it is possible for the
python regex module to enter a case of catastrophic backtracking
which causes oscillation between states and hangs the checkpatch
script.
One suggested solution to these cases is to use an anchor[1] in
the regex, which should force the backtrack to exit early.
However, when I tested this, it didn't seem to improve anything
(since the start is already anchored, and trying to anchor the
end results in the same hang).
Instead, we explicitly check that the line ends with '\\' before
trying to match on the 'if-inside-a-macro' check. A new check
is added to catch the case in checkpatch.
0: https://mail.openvswitch.org/pipermail/ovs-dev/2021-August/386881.html
1: https://stackoverflow.com/questions/22072406/preventing-any-backtracking-in-regex-past-a-specific-pattern
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
When defining a FOR_EACH macro, checkpatch freaks out and generates a
control block whitespace error. Create an exception so that it doesn't
generate errors for this case.
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2020-August/373509.html
Reported-by: Toshiaki Makita <toshiaki.makita1@gmail.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Coding style says: "Put a space between the ``()`` used in a cast and
the expression whose type is cast: ``(void *) 0``.".
This style rule is frequently overlooked. Let's check for it.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Ian Stokes <ian.stokes@intel.com>
There is one remaining use under datapath. That change should happen
upstream in Linux first according to our usual policy.
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Alin Gabriel Serdean <aserdean@ovn.org>
This arg can be used internally by groups using gerrit for code reviews.
Acked-by: Flavio Leitner <fbl@sysclose.org>
Signed-off-by: Roi Dayan <roid@mellanox.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
This seems useful as I'm usually making a lot of typing mistakes.
Also, few commonly used words added to the extended dictionary.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
Acked-by: William Tu <u9012063@gmail.com>
Words like 'br0' are common and usually references some code or
database objects that should not be targets for spell checking.
So, it's better to skip all the words that has digits inside instead
of ones that starts with numbers.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
Acked-by: William Tu <u9012063@gmail.com>
Abbreviations of Latin expressions like 'i.e.' or 'e.g.' are common
and known by the dictionary. However, our spell checker is not able
to recognize them because it strips dots out of them. To avoid this
issue we could pass non-stripped version of the word to the dictionary
checker too.
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Acked-by: Aaron Conole <aconole@redhat.com>
Acked-by: William Tu <u9012063@gmail.com>
Python 2 reaches end-of-life on January 1, 2020, which is only
a few months away. This means that OVS needs to stop depending
on in the next release that should occur roughly that same time.
Therefore, this commit removes all support for Python 2. It
also makes Python 3 a mandatory build dependency.
Some of the interesting consequences:
- HAVE_PYTHON, HAVE_PYTHON2, and HAVE_PYTHON3 conditionals have
been removed, since we now know that Python3 is available.
- $PYTHON and $PYTHON2 are removed, and $PYTHON3 is always
available.
- Many tests for Python 2 support have been removed, and the ones
that depended on Python 3 now run unconditionally. This allowed
several macros in the testsuite to be removed, making the code
clearer. This does make some of the changes to the testsuite
files large due to indentation level changes.
- #! lines for Python now use /usr/bin/python3 instead of
/usr/bin/python.
- Packaging depends on Python 3 packages.
Acked-by: Numan Siddique <nusiddiq@redhat.com>
Tested-by: Numan Siddique <nusiddiq@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
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>
