diff options
author | Julius Werner <jwerner@chromium.org> | 2021-03-16 15:59:01 -0700 |
---|---|---|
committer | Patrick Georgi <pgeorgi@google.com> | 2021-04-06 16:03:21 +0000 |
commit | 89ec3844a567fc4ea94c3618ebed45dea9c5d72b (patch) | |
tree | c6812fb4a0ca4ae2c447a6480f5d19ed40726b6d | |
parent | 7014f8258e6e015fe91d6928266d10ec536e9001 (diff) |
lint: checkpatch: Ignore ASSIGN_IN_IF and UNNECESSARY_ELSE errors
This patch disables checkpatch warnings about two style constructs that
are not illegal in coreboot style and can in my opinion be useful in
certain situations.
The first is an assignment in if conditions like this:
if ((ret = func()))
return ret;
This can save a line compared to the alternative construct which may
help readability, especially in functions that need to do a lot of
these. More importantly, the while-equivalent of this construct is not
forbidden (and a lot more useful, because certain things become very
complicated to write without it), and it seems weird to forbid one but
not the other. We already have GCC warnings that enforce an extra set
of parenthesis to highlight that this is an assignment instead of a
comparison, so the risk of typos or confusion between those two is
already mitigated anyway.
The second is the use of `else` after return like this:
if (CONFIG(TYPE_A))
return response_for_type_a;
else
return response_for_type_b;
While the else is redundant in this case, it serves to highlight the
symmetry and equivalence in importance of the two paths. There are
certainly other situations where the construct of
if (something_went_wrong)
return error;
if (something_else_went_wrong)
return other_error;
if (...)
is more useful, but this usually suggests an "either abort here or
continue on the main path" style flow, whereas the code with `else` is
more suitable to highlight an "either one or the other" flow with two
equal-weighted options. I think the programmer should pick which style
best represents the intentions of their code in these cases, and don't
understand why one of the two should be categorically forbidden.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I130598057c1800277a129ae6b927e961d6e26e42
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51551
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-by: David Hendricks <david.hendricks@gmail.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Frans Hendriks <fhendriks@eltan.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
-rw-r--r-- | .checkpatch.conf | 2 |
1 files changed, 2 insertions, 0 deletions
diff --git a/.checkpatch.conf b/.checkpatch.conf index c294e7bf40..1bf0a321fb 100644 --- a/.checkpatch.conf +++ b/.checkpatch.conf @@ -20,6 +20,8 @@ --ignore SPDX_LICENSE_TAG --ignore UNDOCUMENTED_DT_STRING --ignore PRINTK_WITHOUT_KERN_LEVEL +--ignore ASSIGN_IN_IF +--ignore UNNECESSARY_ELSE # FILE_PATH_CHANGES seems to not be working correctly. It will # choke on added / deleted files even if the MAINTAINERS file |