tree 3075a3ec2b5d5064a26c508bbcf51de32b1f1b27
parent e241d39f0f86b45e604bae1bc716e92d25b713bd
author Julius Werner <jwerner@chromium.org> 1615935541 -0700
committer Commit Bot <commit-bot@chromium.org> 1617752999 +0000

UPSTREAM: 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.

BUG=none
BRANCH=none
TEST=none

Change-Id: Iaf2bf3efafbf17f7087859a2c0557c9fe68f0bca
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Original-Commit-Id: 89ec3844a567fc4ea94c3618ebed45dea9c5d72b
Original-Signed-off-by: Julius Werner <jwerner@chromium.org>
Original-Change-Id: I130598057c1800277a129ae6b927e961d6e26e42
Original-Reviewed-on: https://review.coreboot.org/c/coreboot/+/51551
Original-Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Original-Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Original-Reviewed-by: David Hendricks <david.hendricks@gmail.com>
Original-Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Original-Reviewed-by: Frans Hendriks <fhendriks@eltan.com>
Original-Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+/2807721
Reviewed-by: Patrick Georgi <pgeorgi@chromium.org>
Commit-Queue: Patrick Georgi <pgeorgi@chromium.org>
Tested-by: Patrick Georgi <pgeorgi@chromium.org>