summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJulius Werner <jwerner@chromium.org>2021-03-16 15:59:01 -0700
committerPatrick Georgi <pgeorgi@google.com>2021-04-06 16:03:21 +0000
commit89ec3844a567fc4ea94c3618ebed45dea9c5d72b (patch)
treec6812fb4a0ca4ae2c447a6480f5d19ed40726b6d
parent7014f8258e6e015fe91d6928266d10ec536e9001 (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.conf2
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