Change in coreboot[master]: lint: checkpatch: Ignore ASSIGN_IN_IF and UNNECESSARY_ELSE errors

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/51551 )

Change subject: lint: checkpatch: Ignore ASSIGN_IN_IF and UNNECESSARY_ELSE errors
......................................................................

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 
Change-Id: I130598057c1800277a129ae6b927e961d6e26e42
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51551
Reviewed-by: Felix Held 
Reviewed-by: Werner Zeh 
Reviewed-by: David Hendricks 
Reviewed-by: Angel Pons 
Reviewed-by: Frans Hendriks 
Tested-by: build bot (Jenkins) 
---
M .checkpatch.conf
1 file changed, 2 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  David Hendricks: Looks good to me, approved
  Felix Held: Looks good to me, but someone else must approve
  Werner Zeh: Looks good to me, approved
  Frans Hendriks: Looks good to me, but someone else must approve
  Angel Pons: Looks good to me, approved

diff --git a/.checkpatch.conf b/.checkpatch.conf
index c294e7b..1bf0a32 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