Remove false alarm warning of
`braces {} are required around if/while/for/else`
for macro definitions with if/while/for/else.
Signed-off-by: Vignesh Pandian <vignesh@aerlync.com>
DIR is a POSIX type defined via the dirent.h header.
In Zephyr, we define it as `typedef void DIR`, since it is only
ever described via a pointer (much like `FILE`).
However, in checkpatch.pl, functions that return DIR* or accept
a DIR* argument are met with an error of the form below:
```
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
```
Examples that trigger this false positive are, for example
```cpp
int dirfd(DIR *dirp);
DIR *fdopendir(int fd);
```
Include `DIR` as a class of specific POSIX types that should be
matched as types rather than other tokens.
Signed-off-by: Chris Friedt <cfriedt@tenstorrent.com>
scripts/checkpatch.pl was written originally for the Linux kernel, and
its code reflects the kernel's coding style. In particular, it has
checks for unneeded braces around single-statement if/else/for/while
conditions. In Zephyr however, braces are always required, and so the
checks needed modifying to verify the opposite condition.
In order to enable the now-compatible checks, we also remove the
--ignore BRACES statement in .checkpatch.conf.
Limitations: the current code works well if there are not conditional
statements (e.g. #if, #ifdef or #endif) next to the if/else/for/while
conditions. This is rarely the case, but triggers with the Bluetooth
controller in code like this:
```
#if defined(CONFIG_BT_PERIPHERAL)
if (!lll->is_hdcd)
#endif /* CONFIG_BT_PERIPHERAL */
{
```
```
} else
#endif /* CONFIG_BT_CTLR_PRIVACY */
{
```
```
#if defined(CONFIG_BT_CTLR_DF_ADV_CTE_TX)
if (lll->cte_started) {
radio_switch_complete(phy_s, 0, phy_s, 0);
} else
#endif /* CONFIG_BT_CTLR_DF_ADV_CTE_TX */
{
```
```
#ifdef DUAL_BANK
while ((FLASH_STM32_REGS(dev)->SR1 & FLASH_SR_QW) ||
(FLASH_STM32_REGS(dev)->SR2 & FLASH_SR_QW))
#else
while (FLASH_STM32_REGS(dev)->SR1 & FLASH_SR_QW)
#endif
{
```
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
There's no need for the CODEOWNERS file anymore as figuring out
maintainership and performing reviewer assignment is done via the
MAINTAINERS file now
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
When brackets are used in macros, there may sometimes be a space in
front of them. The checkpatch script should allow this.
The change includes the example that triggered the need for this
change.
Signed-off-by: Rubin Gerritsen <rubin.gerritsen@nordicsemi.no>
This adds volatile type qualifier to the struct declaration matching
when looking for blank line after declarations.
Before this commit, this structure was not accepted by chheckpatch:
struct cfg {
struct gpio_driver_config common;
volatile struct grgpio_regs *regs;
int interrupt;
};
checkpatch.pl generated the following warning:
-:158: WARNING:LINE_SPACING: Missing a blank line after declarations
#158: FILE: drivers/gpio/gpio_grgpio2.c:27:
+ struct gpio_driver_config common;
+ volatile struct grgpio_regs *regs;
With this commit, the warning is no longer generated.
Signed-off-by: Martin Åberg <martin.aberg@gaisler.com>
Unfortunately this check as it is today is causing trouble, while
not checking too well for what it intended. Let's reduce its scope
until a better solution has been found.
Background:
This check intends to ensure coding guidelines Rules A.4 and A.5
are followed, but how it is implemented it does not work well enough.
1. These rules only apply to the kernel and some other parts of the
embedded codebase respectively, but this check is performed on the
whole tree.
2. This check works under the assumption that any attempt to set
these macros in source files is a violation of these rules, while
this is not necessary the case, as there are legitimate uses for these.
(Specially for _POSIX_C_SOURCE and _XOPEN_SOURCE)
This check also fails to detect these macros being set in cmake files,
so if users are faced with this failure they can trivially bypass it.
Having a CI check which produces too many false positives, while
at the same time being very easy to bypass is not a desirable situation
as that can result in lack of trust for this type of checks,
and an overall tendency to override these CI faults,
and overlooking actual violations of these rules by reviewers.
This check was originally added in
b021dece98
Signed-off-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
Comparisons that have constants on both side of the operator
were getting flagged incorrectly. Adjust the check so that
pure constant comparisons are not flagged, reducing false
positives.
WARNING:CONSTANT_COMPARISON: \
Comparisons should place the constant on the right side of \
the test
+BUILD_ASSERT(CONFIG_MAX_PTHREAD_COUNT == \
CONFIG_MAX_PTHREAD_MUTEX_COUNT - 1);
Signed-off-by: Christopher Friedt <cfriedt@meta.com>
Add `__unused` to the `$Attribute` family along with its
`__maybe_unused`, `__always_unused` & `__used` brothers, so
that:
```c
__unused int ret;
```
is recognized as variable declaration, and doesn't raise
`LINE_SPACING` warning in CI.
Signed-off-by: Yong Cong Sin <ycsin@meta.com>
Re-ordering the API names moved a trailing '|' causing the pattern to match
*every* #define in the input.
Signed-off-by: Keith Packard <keithp@keithp.com>
The `NEW_TYPEDEFS` rule consistently generates false
positives when implementing POSIX standard APIs.
It makes sense to disable this check for the POSIX haders
rather than requiring merge superpowers constantly. That
way, we can merge as per usual after sufficient approvals
rather than waiting for someone with merge superpowers to
override / manually merge.
Signed-off-by: Yong Cong Sin <ycsin@meta.com>
All code in the Zephyr core must use only the Zephyr C library API
according to rules A.4 and A.5. Such code is not permitted to request API
extensions from the C library via any of the API request mechanisms.
This addition to checkpatch.pl verifies that patches don't #define
any of these:
__STRICT_ANSI__
_POSIX_SOURCE
_POSIX_C_SOURCE
_XOPEN_SOURCE
_ISOC99_SOURCE
_ISOC11_SOURCE
_ATFILE_SOURCE
_GNU_SOURCE
_BSD_SOURCE
_SVID_SOURCE
_DEFAULT_SOURCE
Reference: #49922
Signed-off-by: Keith Packard <keithp@keithp.com>
The `USE_NEGATIVE_ERRNO` rule consistently generates false
positives when working with certain areas of POSIX.
It makes sense to disable this check for the POSIX API
rather than requiring merge superpowers constantly. That
way, we can merge as per usual after sufficient approvals
rather than waiting for someone with merge superpowers to
override / manually merge.
Signed-off-by: Christopher Friedt <cfriedt@meta.com>
The current docs are incorrect as comma separated paths
are not working. Providing a separate --exclude argement
per path does however work.
Signed-off-by: Håkon Øye Amundsen <haakon.amundsen@nordicsemi.no>
checkpatch identifies a label only when a terminating colon
immediately follows an identifier.
Bitfield definitions can appear to be labels so ignore any
spaces between the identifier terminating colon and any digit
that may be used to define a bitfield length.
Miscellanea:
o Improve the initial checkpatch comment
o Use the more typical '&&' instead of 'and'
o Require the initial label character to be a non-digit
(Can't use $Ident here because $Ident allows ## concatenation)
o Use $sline instead of $line to ignore comments
o Use '$sline !~ /.../' instead of '!($line =~ /.../)'
Link: https://lkml.kernel.org/r/b54d673e7cde7de5de0c9ba4dd57dd0858580ca4.camel@perches.com
Signed-off-by: Joe Perches <joe@perches.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Manikishan Ghantasala <manikishanghantasala@gmail.com>
Cc: Alex Elder <elder@ieee.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Stephanos Ioannidis <stephanos.ioannidis@nordicsemi.no>
Both CONFIG_BIG_ENDIAN and CONFIG_LITTLE_ENDIAN can be used in the code,
so reflect that in the error message in checkpatch.
Signed-off-by: Carles Cufi <carles.cufi@nordicsemi.no>
With the current clang-format rules, we have that multi-line macros that
contain empty lines (usual in driver definition macros) are formatted
with spaces only, no leading tabs. For example:
```c
#define MYDRIVER_DEFINE(i) \
struct mydriver_data data##i; \
\ /* starts with spaces */
struct mydriver_config config##i; \
...
```
This patch makes checkpatch ignore such cases, so that clang-format can
be used in tree while keeping checkpatch happy.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
Some macros, e.g., FOR_EACH_FIXED_ARG, require a separator argument that
needs to be in parentheses, e.g., (||). This should not trigger a
checkpatch warning.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
Checkpatch was checking for macro concats (##), however
we often use UTIL_CAT to allow for multiple macro expansion passes
to succeed.
Signed-off-by: Tom Burdick <thomas.burdick@intel.com>
checkpatch requires a Makefile to be present at the top-level directory.
Remove this requirement.
Signed-off-by: Gerard Marull-Paretas <gerard.marull@nordicsemi.no>
This squashes and applies the Linux diffs to scripts/checkpatch.pl
between Linux commit 16fbf79b0f83bc75 ("Linux 5.6-rc7") and
9123e3a74ec7b93 ("Linux 5.9-rc1") except for commits identified below.
The last 1000 commits to Zephyr master were compared for checkpatch
output differences between the previous Zephyr version and this
version. One new diagnostic about function declarations with an empty
parameter-list was introduced (FUNCTION_WITHOUT_ARGS). The text of an
existing diagnostic was changed (DT_SPLIT_BINDING_PATCH). The text of
LONG_LINE diagnostics was enhanced to provide the actual line length.
Linux commit dfa05c28ca7ffc0a ("checkpatch: remove email address
comment from email address comparisons") was removed because
differences in the scripts resulted in false signed-off-by check
diagnostics when a full name included characters not in Basic Latin,
due to changes in how the author name was extracted. Earlier upstream
changes not integrated into Zephyr may be required.
Linux commit b95692f8b3000166 ("checkpatch: prefer fallthrough; over
fallthrough comments") was removed because Zephyr doesn't support the
upstream pseudo keyword.
Linux commit bdc48fa11e46f867 ("checkpatch/coding-style: deprecate
80-column warning") was edited to not actually change the 80-column
maximum line limit as this change has not been mooted for Zephyr.
Linux commit ced69da1db0b57bb ("checkpatch: fix CONST_STRUCT when
const_structs.checkpatch is missing") was edited to the CONST_STRUCT
file as that's not supported in Zephyr.
Signed-off-by: Peter A. Bigot <pab@pabigot.com>
Now that we are standardizing C99 integer types we can revert the commit
that warned about C99 type usage.
Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
This commit reverts the device tree binding vendor prefix check regular
expression change that was introduced by the following commit:
5b10fac97e
The changed regular expression fails to detect the correctly specified
vendor prefixes in the `vendor-prefixes.txt`.
Signed-off-by: Stephanos Ioannidis <root@stephanos.io>
Linux uses for_each in macros that produce loops; Zephyr uses
FOR_EACH. Update the corresponding checks to match Zephyr's spelling.
Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
This takes the linux diffs between
a8c964eacb21288b2dbfa9d80cee5968a3b8fb21 and
16fbf79b0f83bc752cee8589279f1ebfe57b3b6e and applies them to the
Zephyr copy. Three changes did not apply cleanly:
* linux added a comment to the line that enables C99 comments;
Zephyr disallows them.
* linux uses vendor-prefixes.yaml; zephyr uses the older .txt file
* manual addition of colon in a check before BRACKET_SPACE error
Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
There is no code to handle __syscall_inline so it is better removing
it from doxygen and checkpatch.
Signed-off-by: Flavio Ceolin <flavio.ceolin@intel.com>
import linux-v5.1:Documentation/devicetree/bindings/vendor-prefixes.txt
Use vendor-prefixes.txt to check vendor prefixes
used in compatible strings and property names.
Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com>
This was never a long-term solution, more of a gross hack
to get test cases working until we could figure out a good
end-to-end solution for memory domains that generated
appropriate linker sections. Now that we have this with
the app shared memory feature, and have converted all tests
to remove it, delete this feature.
To date all userspace APIs have been tagged as 'experimental'
which sidesteps deprecation policies.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
This is generating lots of false positives and we keep overriding them
manually, so downgrade and mark it as a warning.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
This is pulled from commit a8c964eacb21288b2dbfa9d80cee5968a3b8fb21 of
the Linux kernel with local zephyr related modifications.
Fixes#4135.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
To define a system call, it's now sufficient to simply tag the inline
prototype with "__syscall" or "__syscall_inline" and include a special
generated header at the end of the header file.
The system call dispatch table and enumeration of system call IDs is now
automatically generated.
Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>
Remove MAINTAINERS in favor of CODEOWNERS file which is supported by
github. Also remove scripts using this file and change checkpatch to
reference CODEOWNERS instead.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Only in rare cases should we allow C99 types, so lets warn about it to
catch issues.
Change-Id: I2bacdd4ba98f88482e0b7acc0567ff1139e749bf
Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Adjust the checkpatch script to recognize the volatile keyword as
forming part of a declaration. This prevents inappropriate checkpatch
gripes for patches such as:
https://gerrit.zephyrproject.org/r/#/c/10345/
Change-Id: Ia29a9002bb9ef15f747d29231d938ae12790724d
Signed-off-by: Marcus Shawcroft <marcus.shawcroft@arm.com>
Update with commit 577f12c07e4edd54730dc559a9c7bc44d22bf7dc from
the Linux kernel.
Change-Id: Ie2cabbfea415d26ffacef340d9497342b496dc29
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
This reverts commit 0c9152d42d.
This was not ready for merge yet.
Change-Id: I08f69e27929867606bc58cdeffed6540725069a1
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Update with commit 577f12c07e4edd54730dc559a9c7bc44d22bf7dc from the Linux
kernel.
Change-Id: I6314dd41eb200c87cdcc6f212f13d3351128de0d
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
checkpatch warns about kconfig lines that contains spaces
instead of tabs.
Change-Id: I60aa74072ce424f21db64a7df0804d7d4cbe315b
Jira: ZEP-427
Signed-off-by: Javier B Perez <javier.b.perez.hernandez@intel.com>
when importing code from external sources, we do not want to fail
on checkpatch and want to run full sanity checks.
Using --exclude <dir> we should be able to exclude a list of well
defined locations in the tree that carry sources from other projects
with other styles.
Change-Id: I7d321e85eed6bc37d5c6879ae88e21d20028a433
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
There seems to be no way to configure it, so I've just removed the code.
Change-Id: I35341e8afc0bdfa8b953b833cfb0eecf05b81c16
Signed-off-by: Vlad Dogaru <vlad.dogaru@intel.com>
Vignesh Pandian
Robin Kastberg
Chris Friedt
Carles Cufi
Benjamin Cabé
Jordan Yates
Rubin Gerritsen
Martin Åberg
Alberto Escolar Piedras
Christopher Friedt
Krzysztof Chruściński
Yong Cong Sin
Dmitry Lukyantsev
Keith Packard
Håkon Øye Amundsen
Joe Perches
Gerard Marull-Paretas
Bartosz Bilas
Tom Burdick
Peter A. Bigot
Kumar Gala
Stephanos Ioannidis
Peter Bigot
Flavio Ceolin
Antony Pavlov
Andrew Boie
Anas Nashif
Marcus Shawcroft
Javier B Perez
Vlad Dogaru