I wished there was a way to do this in smaller steps, but with
every line fixed an error somewhere else became visible. Here
is a (probably incomplete) list of the issues:
* Only one set of parentheses was supported. This is a hard
to solve problem without a real parser (one solution is to
use an recursive RE, see below).
* The precedence order was wrong. Might have been adapted just
to give a positive result for the arbitrary state of the tree.
* Numbered match variables (e.g. $1, $2, etc.) are not local.
Calling handle_expressions() recursively once with $1, then
with $2, resulted in using the final $2 after the first
recursive call (garbage, practically).
Also, symbol and expression parsing was mixed, making things
harder to follow.
To remedy the issues:
* Split handle_symbol() out. It is called with whitespace
stripped, to keep the uglier REs in handle_expressions().
* Match balanced parentheses and quotes when splitting
expressions. In this recursive RE
/(\((?:[^\(\)]++|(?-1))*\))/
the `(?-1)` references the outer-most group, thus the whole
expression itself. So it matches a pair of parentheses with
a mix of non-parentheses and the recursive rule itself inside.
This allows us to:
* Order the expression matches according to their precedence
rules. Now we can match `<expr> '||' <expr>` first as we should
and everything else falls into its place.
* Remove the bail-out that silenced the undefined behavior.
Change-Id: Ibc1be79adc07792f0721f0dc08b50422b6da88a9
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52067
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Gerrit is able to add reviewers based on entries in the `MAINTAINERS`
file. For inclusion and exclusion matches either paths or regular
expressions can be used. The syntax is described in the header of the
file.
When matching a path, there are two sensible possibilities:
- `path/to/file` matches a file.
- `path/to/dir/` matches a folder including its contents recursively.
- `path/to/dir/*` matches all files in that folder, without recursing
into its subfolders.
The trailing slash in the second example is essential. Without it, only
the directory entry itself matches when, for example, the folder gets
deleted, renamed or its permissions get modified. Reviewers in the list
won't get added to changes of any files or directories below that path.
Thus, add a linter script to ensure a path match on a directory always
ends with `/` or `/*` as shown above.
Change-Id: I9873184c0df4a0b4455f803828e2719887e545db
Signed-off-by: Michael Niewöhner <foss@mniewoehner.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52210
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Some of the src/vendorcode/ directories are used to import a whole
codebase from somewhere else which uses a completely different coding
style. For those directories, excluding them from checkpatch makes
sense. However, other directories are simply implementing
vendor-specific extensions that were written by coreboot developers
specifically for coreboot in coreboot's coding style. Those directories
should be covered by checkpatch.
This patch narrows the existing blanket exception of src/vendorcode/ to
the amd, cavium, intel and mediatek directories (which actually include
large amounts of foreign source). The eltan, google and siemens
directories (which seem to contain code specifically written for
coreboot) will now be covered by checkpatch.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I1feaba37c469714217fff4d160e595849e0230b9
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51827
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-by: David Hendricks <david.hendricks@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This reverts commit 15e379aaf3.
It triggers on directories that only contain artifacts and no
checked in code. As this happens a lot when switching branches,
it makes it impossible to commit new code.
Change-Id: I38a86c8a5d5dc14ca5f6cba789bcb8c0fcaefb0b
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50354
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The codebase currently has only unix line endings, so add a lint tool
to check for windows line endings.
BUG=None
TEST=Verify that line endings are caught both inside and outside a git
repo.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: I6faf99a3184e4843640fb8965f8124de0bc52ce7
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50851
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Instead of hardcoding paths to the executables, use the version in the
path. This allows the scripts to work on more systems, and allows the
binary version to be changed more easily if needed.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: Ifcc56aa21092cd3866eacb6a02d198110ec6051d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48904
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
It would seem that `pres` is an abbreviation for `presence`. Personally,
over the last ~2.5 years, I have seen checkpatch complaints about `pres`
on several occasions, and all of them were abbreviations for `presence`.
Given the high false positive rate for this entry, comment it out.
Change-Id: I72f1811fb1f766e7de7c4957fd9ba844c0728029
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49463
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Check that nobody misuses the Kconfigs SUBSYSTEM_*_ID. They are meant to
be used for overriding the devicetree subsystem ids locally but shall
not be added to a board's Kconfig. Instead, the devicetree option
`subsystemid` should be used.
Add a linter script for this that finds and warns about such misuse.
Also add a note in the Kconfigs' description.
TEST=CB:45513
Change-Id: I21c021c718154f1396f795a555af47a76d6efe03
Signed-off-by: Michael Niewöhner <foss@mniewoehner.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45513
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
The lint-stable makefile target only watches for errors in the Kconfig
file, so has not protected additional "Naked" references to BOOL type
Kconfig symbols from entering the tree. Update it to an error so that
they can't continue coming into the codebase.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: Icce2a9a627c4fbcaa220df18474cb8bfea8b2a8c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43826
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The majority of the codebase has been converted to use SPDX identifiers
now, so let's enforce those by default. The only exceptions are
src/include and src/lib, which are not being checked since many of the
files there do not have license headers at all. Files with custom
licenses that aren't covered by SPDX can be listed as exceptions at the
top of lint-000-license-headers.
Change-Id: Ie6642153793d5735c74c5950bc9e27ee7eecacbc
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41602
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Convert the remaining files in src/drivers to use SPDX identifiers.
int15.h and default_brightness_levels.asl did not have license headers,
but they were both copied from other GPL2 files, so they should be under
the GPL2 as well.
ne2k.c and drm_dp_helper.h are licensed under custom BSD-like licenses
that do not have an SPDX equivalent, so they are added as exceptions
to the license header lint.
Change-Id: I87fb1c637b8d11b0463f7c19f70b847413e14aed
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41601
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The Historical Permission Notice and Disclaimer (with and without
permission to sell) is a BSD-style license family that OSI and SPDX
consider deprecated - and yet, it's right here in our tree.
Change-Id: I61624b6e54e9aba6e2f54822c1f68967c416ad3d
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41221
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Stefan thinks they don't add value.
Command used:
sed -i -e '/file is part of /d' $(git grep "file is part of " |egrep ":( */\*.*\*/\$|#|;#|-- | *\* )" | cut -d: -f1 |grep -v crossgcc |grep -v gcov | grep -v /elf.h |grep -v nvramtool)
The exceptions are for:
- crossgcc (patch file)
- gcov (imported from gcc)
- elf.h (imported from GNU's libc)
- nvramtool (more complicated header)
The removed lines are:
- fmt.Fprintln(f, "/* This file is part of the coreboot project. */")
-# This file is part of a set of unofficial pre-commit hooks available
-/* This file is part of coreboot */
-# This file is part of msrtool.
-/* This file is part of msrtool. */
- * This file is part of ncurses, designed to be appended after curses.h.in
-/* This file is part of pgtblgen. */
- * This file is part of the coreboot project.
- /* This file is part of the coreboot project. */
-# This file is part of the coreboot project.
-# This file is part of the coreboot project.
-## This file is part of the coreboot project.
--- This file is part of the coreboot project.
-/* This file is part of the coreboot project */
-/* This file is part of the coreboot project. */
-;## This file is part of the coreboot project.
-# This file is part of the coreboot project. It originated in the
- * This file is part of the coreinfo project.
-## This file is part of the coreinfo project.
- * This file is part of the depthcharge project.
-/* This file is part of the depthcharge project. */
-/* This file is part of the ectool project. */
- * This file is part of the GNU C Library.
- * This file is part of the libpayload project.
-## This file is part of the libpayload project.
-/* This file is part of the Linux kernel. */
-## This file is part of the superiotool project.
-/* This file is part of the superiotool project */
-/* This file is part of uio_usbdebug */
Change-Id: I82d872b3b337388c93d5f5bf704e9ee9e53ab3a9
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41194
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This is all code coming from the outside, so let's keep these files
untouched as much as possible.
A couple of files is added to the list by name because their license,
while free, can't be properly modelled in SPDX:
- lzmadecode is (LGPL OR CPL) WITH special-exception
- stack.c and start16 are some weird (but free) US Gov't license grant
- two XGI related files have "BSD except for Linux, where it's GPL"
Change-Id: I42dec503b9c427a66792d3fec99ca8df1a360e47
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41193
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
If they were removed instead, it would be too easy to end up adding them
back again. They are kept in a comment so that they can be tracked.
Also, explain why these two entries have been commented out.
Change-Id: I8225944b5e3d1e022af169dda33e0344d4c3bccd
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39618
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Otherwise there will, after make gitconfig,
be (hidden) shell command failures with 'git commit -s':
gmake: util/lint/check-style: Command not found
gmake: *** [Makefile.inc:632: check-style] Error 127
Change-Id: I3891dee53702ee10e5e44dae408193e49d7a89f1
Signed-off-by: Idwer Vollering <vidwer@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38227
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
commit 1191c09201b43aab55333a70d056d0c355abe329 at
https://salsa.debian.org/agx/lintian/tree/master/data/spelling provides
a much more comprehensive collection of misspellings, so merge it in.
While at it, also sort the file for future easier merging which is the
main reason that some lines appear to be removed: they're merely moved.
For sorting, I adapted their make rule:
make -f - sort-spelling.txt <<'EOF'
.RECIPEPREFIX=%
sort-%: %
%csplit --prefix $<- $< '/^$$/'
%LC_ALL=en_US sort -u $<-01 | cat $<-00 - > $<
%rm -f $<-0[01]
EOF
Change-Id: I939e3a8820c88d0e639bd29b46a86b72bce1a098
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/38632
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
In preparation to update to SPDX license headers, add identifiers
for the licenses seen in the coreboot project and create a command
line parameter allowing only SPDX license identifiers to be detected.
Here are example locations of these licenses:
Apache-2.0 - src/soc/sifive
BSD-3-Clause - Throughout coreboot & libpayload source
GPL-2.0-only - Throughout coreboot source
GPL-2.0-or-later - Throughout coreboot source
GPL-3.0-only - util/amdtools
GPL-3.0-or-later - src/lib/[gcov/libgcov/gnat]
ISC - src/lib/ubsan.c, soc/qualcomm/ipq806x/include/soc/gsbi.h, others
MIT - soc/nvidia/tegra210/mipi_dsi.c, files in mainboard/cavium/
X11 - include/device/drm_dp_helper.h, drivers/aspeed/common/ast_tables.h
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: I07a7ca408ac8563e03e189d05ef7729dfb6fc24e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36175
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
The current clang-format configuration is completely broken. It forces
one to change the code style of patches before pushing them, only to
find out that checkpatch now complains about it. This means newcomers
get scared away, and developers only get angered and frustrated about
it, and end up working around clang-format's requirements anyway.
For now, make clang-format's complaints non-fatal, reducing them to text
noise. However, since clang-format is currently unusable, reverting it
out would be preferred.
Change-Id: Iffa8934efa1c27c04e10545f66d8f9976e74c367
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35176
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
