As seen in code review here:
https://gerrit.osmocom.org/c/libosmocore/+/30934
... we do care about not using braces for single statement blocks. Let
the linter comment on it.
src/osmo_io.c:143: WARNING:BRACES_NOT_NECESSARY: braces {} are not necessary for single statement blocks
src/osmo_io.c:271: WARNING:BRACES_NOT_NECESSARY: braces {} are not necessary for single statement blocks
src/osmo_io.c:306: WARNING:BRACES_NOT_NECESSARY: braces {} are not necessary for single statement blocks
src/osmo_io_poll.c:63: WARNING:BRACES_NOT_NECESSARY: braces {} are not necessary for single statement blocks
src/osmo_io_poll.c:117: WARNING:BRACES_NOT_NECESSARY: braces {} are not necessary for single statement blocks
Change-Id: I481d1b24a909173520a544ffd567bb8357729f2a
Fix an error when the same Change-Id is used on multiple branches or in
theory projects. This is actually allowed by gerrit, and we use this
e.g. when backporting patches from master.
Use the project, change number (e.g. 30147) and patchset number (e.g. 2)
instead of the Change-Id.
Fix for:
+ ssh -p 29418 -l jenkins gerrit.osmocom.org gerrit review 4835a62cd88f0d69db76fb3bfd2df02176a91a6d --json
fatal: "4835a62cd88f0d69db76fb3bfd2df02176a91a6d" matches multiple patch sets
Related: https://gerrit-review.googlesource.com/Documentation/cmd-review.html
Change-Id: I2d627f8f3b400fa57a50a228d47df2194f60fd08
Use robot comments to add line based comments for the lint output.
Add checkpatch_json from coreboot 9cae17d0 to parse the checkpatch
output and convert it into a gerrit parsable format.
Example:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/29294
Co-Authored-By: Oliver Smith <osmith@sysmocom.de>
Change-Id: I1a48ddb976e0f53bfc0552d0be11e42ba68d9e49
The purpose of "cut -d / -f 2-" is to strip off leading "./" in
the paths. The same can be achieved by doing "-printf '%P\n'".
Matching "*.c" and "*.h" files can be expressed as "-name *.[hc]".
Change-Id: Iae3fc5c8842df6926e6ff16a41be5663f1dedd1b
Inside struct osmo_tdef we write non-spec timers as T=-1234. Do not
complain about having no space before the minus character.
Example:
{ .T=-25, .default_val=5, .desc="Timeout for ..." },
Note that the Linux kernel coding style also requires spaces around the
equals signs. We follow that everywhere except for struct osmo_tdef,
and the linter has already been adjusted for that in
I1f0b9ed5bd49ef9b5ab0e347b9260e71df34ff9c.
Change-Id: I0885e84ad99c7d333a5930c411fd1273badb0fcb
Don't require breaking strings at spaces. This is not useful for strings
of hex characters, e.g.:
{ 123, "ffffffffffffffffffffffffffffffffffffffff"
"ffffffffffffffffffffffffffffffffffffffff"
"ffffffffffffffffffffffffffffffffffffffff"
Change-Id: Ib6be7744418530ae3ddbf373e67d1d7f25a60508
Fix false error:
ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
in a line with a typedef:
int handle_rab_ass_req(..., ranap_message *message);
Change-Id: Icbc46d0c788ca75684a48a73f739265f4a09b09c
Not having the --rm flag causes docker containers to be stored forever,
so make sure we always add it to the "docker run" commands that we run
on jenkins via scripts in various repositories.
Depends: docker-playground I48b01c43fedf379b8a565eaab0369806d7831bd8
Related: https://osmocom.org/projects/osmocom-servers/wiki/Docker_cache_clean_up
Related: SYS#5827, OS#5099
Change-Id: I8ab9c291504475d670bdefc50c4524c5bdd4c880
Don't recommend to use ftrace instead of using __func__ inside printf.
This only applies to the linux kernel, not to the Osmocom code base.
Related: OS#5087
Related: https://gerrit.osmocom.org/c/libosmocore/+/6357
Change-Id: Ied23be32d3371a8b9ac9938a336474f71e7a3ccd
Don't complain about using braces when they could be omitted, for
example:
if (condition) {
single_statement();
}
Another example:
if (condition) {
single_statement();
} else {
another_single_statement();
}
This is not something we would care about in code review either from
what I've seen and so it's probably just annoying for patch authors to
fix up.
Related: OS#5087
Change-Id: Ice08d5b88c683a59bacff999a1d6c07754663d39
Not necessarily followed in Osmocom code, as Daniel wrote:
> The lint complains about this break, I don't agree with it though.
> Without we're one (or two) refactors away from an unintended
> fall-through
Related: OS#5087
Change-Id: I3f106510953b0b1bf70c28ceb55a431c5c03854e
In Osmocom code, we have the following written in one line:
while (osmo_select_main_ctx(1) > 0);
This currently causes the following linter error:
ERROR:TRAILING_STATEMENTS: trailing statements should be on next line
According to the linter, we should write it as follows:
while (osmo_select_main_ctx(1) > 0)
;
But this is not followed in Osmocom code, so let's ignore the check.
Related: OS#5087
Change-Id: Iaffe979b771c97c77edaf4aa0d232cb8939d1279
When running the linter on a patch with lots of files changed, and lots
of exclude files, this makes a big difference. On my machine, running an
osmo-iuh patch with 1580 files changed, and the high amount of asn1c
related excludes, the time for linting is reduced from 50s to 25s.
This should be acceptable, since typically we change only few files.
Change-Id: I6fb41dec25ecc1f2df7242ae041a8685a696c3fd
Does not make a noticable speed difference on a typical patch with few
changed files, but makes linting on big patches with ~1000 files and
lots of asn1c generated files in the repository significantly slower.
The next patch will optimize that case.
Change-Id: I7437d888b433fec8a444e4d7c285fff47d16c0c7
Add Osmocom specific check to forbid using strncpy() and strcpy().
Instead, osmo_strlcpy() or OSMO_STRLCPY_ARRAY() should be used.
Related: OS#5087
Related: https://lists.osmocom.org/pipermail/openbsc/2021-September/013538.html
Change-Id: I6fa96c8f3d15110dd3d3509faa593285a78f469e
According to a comment in checkpatch.pl, these should automatically be
ignored when LONG_LINE is ignored. However I found that it's still
complaining about LONG_LINE_COMMENT, so explicitly disable it.
Related: OS#5087
Change-Id: I7aed3bfbfcb0b9e2f1d743b111e8418846f031d2
Mentioning the file in itself is useful sometimes (e.g. when explaining
how to use a contrib script). So do not let the linter fail here.
Related: OS#5087
Change-Id: I151b97bc7f2fe83898c0598db54360807956993c
Pau has to remind me often that %d is used instead of %i in Osmocom
trees, so let's automate it.
Complain like this:
src/vty/command.c:3052: WARNING:PRINTF_I_OSMO: Use %d instead of %i
Related: OS#5087
Change-Id: I1a98326f1cbf4d2e0bb948558e5cd1726b0a9868
Don't complain that macros such as __packed should be used, which are
defined in the Linux kernel but not in libosmocore.
For example:
src/gsm/gsm0808.c:85: WARNING:PREFER_DEFINED_ATTRIBUTE_MACRO: Prefer __packed over __attribute__((packed))
Related: OS#5087
Change-Id: I2bf3b7d60e99cf91f7b619af54167a11cdfae8c6
Do not complain if indenting with exactly 6/7 spaces below (g)DEFUN(,
as it's often done in VTY-related code in Osmocom. This patch assumes
that if the line starts with 6/7 spaces and " or a word, it's probably
below DEFUN( or gDEFUN(. I've considered implementing a more accurate
check, but that would be too much effort (e.g. when more macros are
involved).
Other related macros, such as DEFUN_ATTR are longer. Indentation below
those should be done with one tab (+ spaces for padding), the linter
doesn't need to be adjusted for those.
Related: OS#5087
Change-Id: I0934b63a62500e7a3e09c753cc63aa331e580cc6
Don't complain with:
ERROR:SPACING: space prohibited after that '&&' (ctx:ExW)
in code similar to:
if (conn->conn->mode != MGCP_CONN_LOOPBACK
&& conn->conn->mode != MGCP_CONN_RECV_ONLY
&& !mgcp_rtp_end_remote_addr_available(&conn->end)) {
The check was supposed to complain about spaces if the && is used as
unary operator (to get the address of a goto label). But it's clearly
producing false positives in the Osmocom context with use as non-unary
operator, so remove this check.
Related: OS#5087
Related: 0d413866c7
Change-Id: I7ce79e6b291b3a3dab6587a589eeef0a0bc53de9
Add lint_diff.sh, which runs checkpatch on git diff to either HEAD~1 (if
the tree is clean) or HEAD. This can be used as pre-commit hook, and
it's what jenkins will run.
Add lint_all.sh, which runs checkpatch on a whole repository to test if
the rules we are checking for make sense in Osmocom context.
Related: OS#5087
Change-Id: I1d02c169b05fb05b87209a444a5ddb86efc99d04