Commit Graph

51 Commits

Author SHA1 Message Date
Oliver Smith 266a4363b3 lint: ignore SYMBOLIC_PERMS
As discussed here:

Change-Id: I47cfeef55c6cece95fed706b67c117274097977d
2023-01-20 09:25:16 +01:00
Oliver Smith 9e24272311 lint: enable BRACES_NOT_NECESSARY check
As seen in code review here:

... 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
2023-01-19 09:17:02 +01:00
Oliver Smith f560ce08ab jenkins-gerrit: fix ambiguous use of review api
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 review 4835a62cd88f0d69db76fb3bfd2df02176a91a6d --json
  fatal: "4835a62cd88f0d69db76fb3bfd2df02176a91a6d" matches multiple patch sets

Change-Id: I2d627f8f3b400fa57a50a228d47df2194f60fd08
2022-11-15 11:38:08 +00:00
Oliver Smith 288fe221e8 lint: enable FUNCTION_WITHOUT_ARGS
Complain if one writes func() instead of func(void).

Change-Id: Ic00041025ac2584f250b5a096eae8fd0d857d9fb
2022-11-04 13:24:10 +00:00
Oliver Smith d1047b3803 lint: accept BE spelling of 'acknowledgement'
Change-Id: Ia1a08295a7c7ed6a77f0055d66a161423d8f17f0
2022-09-22 13:15:02 +00:00
Oliver Smith 4a0b42cc4f lint: ignore MACRO_WITH_FLOW_CONTROL
It seems that we don't care about this one, e.g. here:

Change-Id: I79da5a426db59031e3b16aecedeaa1498c91e847
2022-09-15 10:24:38 +02:00
Alexander Couzens f8710473d1 lint: annotate lines in gerrit
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.


Co-Authored-By: Oliver Smith <>
Change-Id: I1a48ddb976e0f53bfc0552d0be11e42ba68d9e49
2022-09-08 17:10:51 +02:00
Oliver Smith b55a41f2c6 lint/typedefs_osmo: add A_SEQUENCE_OF, A_SET_OF
Fix need consistent spacing around '*' lint errors where these are used.

Change-Id: I50e8e1ddcf6a4f927f533094accf7f8a18b523d1
2022-07-15 11:30:32 +02:00
Vadim Yanitskiy 4f8a4954b9 lint: simplify find command in exclude_paths_common_asn1c()
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
2022-07-03 04:55:54 +07:00
Vadim Yanitskiy e6ad3a2b2f lint: ignore symlinks (no newline at end of file)
Change-Id: I6db6ebb51cdfd54e8f9c2c5cc7affef60a7dec32
2022-07-03 04:49:13 +07:00
Oliver Smith 6dccbdbe92 lint: ignore test output in .ctrl, .vty files
Change-Id: I7611bb1b26c4abbfb572182a490f289216e7b708
2022-06-20 15:29:00 +02:00
Oliver Smith 1abb6f35bf lint: checkpatch: don't require space in T=-1234
Inside struct osmo_tdef we write non-spec timers as T=-1234. Do not
complain about having no space before the minus character.

    { .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

Change-Id: I0885e84ad99c7d333a5930c411fd1273badb0fcb
2022-03-03 10:41:48 +01:00
Oliver Smith fe6512a5ba lint: exclude .patch files
Change-Id: I734452bc78d7e026292883f81bc07c3baf58e309
2022-03-02 11:43:39 +01:00
Oliver Smith c100a6d098 lint: checkpatch: ignore MISSING_SPACE
Don't require breaking strings at spaces. This is not useful for strings
of hex characters, e.g.:

	{ 123, "ffffffffffffffffffffffffffffffffffffffff"

Change-Id: Ib6be7744418530ae3ddbf373e67d1d7f25a60508
2022-02-18 14:18:46 +01:00
Oliver Smith bc9bd3df75 lint: checkpatch: show hint about typedefs_osmo.txt
Change-Id: Icaee700a9cc7896cdbda6cce93db41b6b50a7fa3
2022-02-15 13:51:09 +01:00
Oliver Smith cf1fdbf030 lint: checkpatch: add typedefs_osmo.txt
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
2022-02-15 13:47:06 +01:00
Oliver Smith 41e218a355 lint: catch missing --rm flag for 'docker run'
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: SYS#5827, OS#5099
Change-Id: I8ab9c291504475d670bdefc50c4524c5bdd4c880
2022-02-11 15:44:16 +01:00
Oliver Smith d7fe8fd047 lint: checkpatch_osmo: ignore TRACING_LOGGING
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
Change-Id: Ied23be32d3371a8b9ac9938a336474f71e7a3ccd
2021-12-22 15:10:03 +01:00
Oliver Smith 2b1085886c lint: checkpatch: tweak FSF_MAILING_ADDRESS error
Change-Id: Ic76132c40a9c30a167c24c4c4b53180c4cc0fb86
2021-12-14 12:52:16 +01:00
Oliver Smith 73b33a4e9e lint: checkpatch_osmo: ignore BRACES_NOT_NECESSARY
Don't complain about using braces when they could be omitted, for
	if (condition) {

Another example:
	if (condition) {
	} else {

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
2021-12-08 11:32:24 +01:00
Oliver Smith 425cbb5e28 lint: checkpatch_osmo: ignore VOLATILE
The warning about potentially using volatile wrong is not useful for us,
it makes sense to use it in embedded projects.

Related: OS#5087
Change-Id: Ie81db479c66749531ed1c81cf076ce248aa22f69
2021-12-08 11:24:29 +01:00
Oliver Smith 78c08a33bd lint: checkpatch_osmo: ignore UNNECESSARY_BREAK
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
2021-12-01 10:05:09 +01:00
Oliver Smith ec43684471 lint: checkpatch_osmo: ignore TRAILING_STATEMENTS
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
2021-11-09 11:48:01 +01:00
Oliver Smith 761df2c7e6 lint: break exclude loop early
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
2021-11-02 13:40:22 +00:00
Oliver Smith 623b0b60f9 lint: exclude asn1c generated
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
2021-11-02 13:40:22 +00:00
Vadim Yanitskiy 5b72401b59 lint/checkpatch: ignore csn1_(enc|dec).c files in osmo-pcu.git
Change-Id: I5f8c83e3a8b9f5280779d6a6cf0736b9d3f4e958
Related: I7d1b1f7e6d7f89b052b3fd73a960419bb2673020
2021-10-20 21:25:31 +03:00
Oliver Smith b00eb7b012 lint: osmo-pcu: exclude imported wireshark code
Related: OS#5087
Change-Id: I70814512bdba50363edd4195b5b073698ea6532c
2021-10-08 14:15:40 +02:00
Oliver Smith b228c23e08 lint: support project-specific exclude paths
While at it, put in the exact path to spelling.txt in osmo-ci.git.

Related: OS#5087
Change-Id: Ib23f9c65da1916ebf4654c5e641eaffe6c75315c
2021-10-08 14:15:09 +02:00
Oliver Smith 477218474f lint: ignore debian/changelog
Auto-generated from commit log, may contain spelling errors.

Fixes: OS#5232
Change-Id: Id0ccfbff73464dac7f7f939c88a9e2677fa2a37f
2021-09-15 16:41:18 +02:00
Oliver Smith cbc58a6874 lint: add STRCPY_OSMO
Add Osmocom specific check to forbid using strncpy() and strcpy().
Instead, osmo_strlcpy() or OSMO_STRLCPY_ARRAY() should be used.

Related: OS#5087
Change-Id: I6fa96c8f3d15110dd3d3509faa593285a78f469e
2021-09-15 11:08:40 +02:00
Oliver Smith 654cfb3192 lint: ignore UNSPECIFIED_INT
As suggested by Pau, let's remove this check because it isn't useful for us.

Related: OS#5087
Change-Id: I15345a154f490362093e46fc0df75e75bbb237fe
2021-09-13 10:37:06 +02:00
Oliver Smith 94cf194666 lint: ignore UNNECESSARY_INT
Related: OS#5087
Change-Id: I5be890a296b37511affc17cb786f0709de822826
2021-09-13 10:32:15 +02:00
Oliver Smith 4f75177e8c lint: ignore LONG_LINE_*
According to a comment in, 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
2021-09-03 08:27:28 +00:00
Oliver Smith 8cbd4bf779 lint: ignore EMBEDDED_FILENAME
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
2021-08-16 11:27:11 +02:00
Oliver Smith b70fede452 lint: ignore LONG_LINE
With recent code reviews I've realized that in Osmocom we do often use
more than 120 characters per line for various reasons. So adjust the
linter to not fail anymore if that is the case.

Related: OS#5087
Change-Id: I88fd86ac550fddb3017aeceb647c3d9e75367372
2021-07-30 10:11:11 +02:00
Oliver Smith dc66e9a394 lint: exclude kdf, milenage for libosmocore
Imported code that doesn't follow our guidelines (e.g. spaces used
instead of tabs).

Related: OS#5087
Change-Id: Iccf59d66f35ede9710258faf9d4257584214bb17
2021-07-19 14:39:43 +02:00
Oliver Smith dbc4e6e78f lint: no spaces required for tdef
Don't require spaces around equals sign for tdef entries.

Related: OS#5087
Change-Id: I1f0b9ed5bd49ef9b5ab0e347b9260e71df34ff9c
2021-07-19 14:39:39 +02:00
Oliver Smith bedf7ed0fb lint: complain about %i in printf
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
2021-07-15 10:21:47 +02:00
Oliver Smith 7f0fdda23c lint: ignore PREFER_DEFINED_ATTRIBUTE_MACRO
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
2021-07-15 10:21:47 +02:00
Oliver Smith 7c3c0bdaa5 lint: ignore DEEP_INDENTATION
Related: OS#5087
Change-Id: Iff29f0a97dbfed904705f543541441f761370602
2021-07-15 10:21:47 +02:00
Oliver Smith 972e8bcbb0 lint: ignore PREFER_FALLTHROUGH
Related: OS#5087
Change-Id: I373a231cf08fd23312ad9a85d8e2855a736e331a
2021-07-15 10:21:47 +02:00
Oliver Smith b6e2adb0aa lint: allow spaces below (g)DEFUN
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

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
2021-07-15 10:21:05 +02:00
Oliver Smith 444ca841e6 lint: fix && complaints
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
2021-07-07 11:28:38 +02:00
Oliver Smith 5409188d55 lint: ignore LINE_CONTINUATIONS
Causes false positives, e.g. in tests/mgcp/mgcp_test.c in osmo-mgw:

  #define MDCX4_PT2 \
	"MDCX 18983218 1@mgw MGCP 1.0\r\n" \
	"M: sendrecv\r" \
	"C: 2\r\n" \
	"I: %s\r\n" \
	"L: p:20-20, a:AMR, nt:IN\r\n" \

tests/mgcp/mgcp_test.c:189: WARNING:LINE_CONTINUATIONS: Avoid unnecessary line continuations

Related: OS#5087
Change-Id: I8d8004f2a8ef926588487187af7cdef8254c7248
2021-07-07 11:28:38 +02:00
Oliver Smith 2cceff4535 lint: ignore COMPLEX_MACRO
Complains about, for example:
  #define MDCX4 \
	"MDCX 18983217 1@mgw MGCP 1.0\r\n" \
	"M: sendrecv\r" \
	"C: 2\r\n" \
	"I: %s\r\n" \
	"L: p:20, a:AMR, nt:IN\r\n" \
	"\n" \
	"v=0\r\n" \
	"o=- %s 23 IN IP4\r\n" \
	"c=IN IP4\r\n" \
	"t=0 0\r\n" \
	"m=audio 4441 RTP/AVP 99\r\n" \
	"a=rtpmap:99 AMR/8000\r\n" \

Related: OS#5087
Change-Id: Ic9d752ca841161a62e3631c84b4237a0d8594363
2021-07-07 11:28:38 +02:00
Oliver Smith 497ce6743a lint: exclude *.ok, *.err
Complains about e.g. \r\n in *.ok files.

Related: OS#5087
Change-Id: I79004cec3e2eb753951a7f886318ac0db4ea2c06
2021-07-07 11:28:37 +02:00
Oliver Smith 57804faacc lint: exclude pattern, not dir
Allow to exclude files in a follow-up commit, not just directories.

Related: OS#5087
Change-Id: Ic3990fba60060c331c479174183924b9cdbdb4c8
2021-07-07 11:28:16 +02:00
Oliver Smith d58b999e0f lint: add helper scripts
Add, 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, 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
2021-06-16 16:49:47 +02:00
Oliver Smith 1e750ed6dc lint/checkpatch: add Osmocom specific wrapper
Related: OS#5087
Change-Id: I0ec6a3bc57a4d31c821fa83370f05c6d4ac2a5b0
2021-06-16 16:42:18 +02:00
Oliver Smith 6c497c076b lint/checkpatch: add --exclude argument
Apply patch from, so we
can exclude specific directories.

Related: OS#5087
Change-Id: Ia980814895249f839873c5002f0d21c0e59ee01d
2021-06-16 16:42:18 +02:00