From: Johan Hovold <johan@kernel.org>
To: Alex Elder <elder@ieee.org>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>,
Johan Hovold <johan@kernel.org>, Alex Elder <elder@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
greybus-dev@lists.linaro.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH][next] greybus: Use fallthrough pseudo-keyword
Date: Wed, 5 Aug 2020 18:21:02 +0200 [thread overview]
Message-ID: <20200805162102.GK3634@localhost> (raw)
In-Reply-To: <e36013ba-e19a-86c9-cb68-d7ad5559c292@ieee.org>
On Wed, Aug 05, 2020 at 08:14:47AM -0500, Alex Elder wrote:
> On 7/28/20 5:37 PM, Alex Elder wrote:
> > On 7/27/20 1:32 PM, Gustavo A. R. Silva wrote:
> >> Replace the existing /* fall through */ comments and its variants with
> >> the new pseudo-keyword macro fallthrough[1].
> >>
> >> [1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
> >
> > Thanks for the patch. It looks good, but it raises
> > another question I'd like discussion on.
Sorry about the late reply on this.
> It's been a week, and we heard back from Viresh (and Joe) on
> this, but no one else. Viresh left out the break statement on
> the last case of the switch statement intentionally, arguing
> that it is not needed (much like a return statement at the end
> of a void function). But he doesn't feel strongly enough
> insist it should stay that way. I'm sure the others omitted
> the break statement intentionally as well.
I really don't mind break statements in the final case and often do add
them, but I'm a bit reluctant to suggest that this is something that
need "fixing". There are a ton of these all over the kernel, and I think
we have too many opportunities for people to do mechanical clean ups
already.
Especially after Gustavo's work, the only real argument for adding them
is mostly moot as the compiler would catch it if anyone adds a new case
and forgets about the break statement.
> Given no strong pushback, I'll ask you (Gustavo) to post a
> second patch adding the missing break statements I described
> (and look for any others I might have missed). If you would
> prefer not to do that, just say so, and I will send out such
> a patch myself.
I'd probably just leave it as is, if only to not inspire others to send
copy-cat "fixes".
Johan
prev parent reply other threads:[~2020-08-05 20:15 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-27 18:32 [PATCH][next] greybus: Use fallthrough pseudo-keyword Gustavo A. R. Silva
2020-07-28 22:37 ` Alex Elder
2020-07-29 0:25 ` Joe Perches
2020-07-29 10:51 ` [greybus-dev] " Viresh Kumar
2020-07-29 12:15 ` Alex Elder
2020-08-05 13:14 ` Alex Elder
2020-08-05 16:21 ` Johan Hovold [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200805162102.GK3634@localhost \
--to=johan@kernel.org \
--cc=elder@ieee.org \
--cc=elder@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=greybus-dev@lists.linaro.org \
--cc=gustavoars@kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox