From: Kalle Valo <hidden> Date: 2021-03-05 13:41:35
"Gustavo A. R. Silva" [off-list ref] writes:
In preparation to enable -Wimplicit-fallthrough for Clang, fix
multiple warnings by replacing /* fall through */ comments with
the new pseudo-keyword macro fallthrough; instead of letting the
code fall through to the next case.
Notice that Clang doesn't recognize /* fall through */ comments as
implicit fall-through markings.
Link: https://github.com/KSPP/linux/issues/115
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
From: Gustavo A. R. Silva <hidden> Date: 2021-03-05 17:11:09
On 3/5/21 07:40, Kalle Valo wrote:
"Gustavo A. R. Silva" [off-list ref] writes:
quoted
In preparation to enable -Wimplicit-fallthrough for Clang, fix
multiple warnings by replacing /* fall through */ comments with
the new pseudo-keyword macro fallthrough; instead of letting the
code fall through to the next case.
Notice that Clang doesn't recognize /* fall through */ comments as
implicit fall-through markings.
Link: https://github.com/KSPP/linux/issues/115
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
It's not cool that you ignore the comments you got in [1], then after a
while mark the patch as "RESEND" and not even include a changelog why it
was resent.
I'm a bit confused. I replied to the same thread at the time. You can see
my response there. No one replied back. :/
--
Gustavo
On Fri, Mar 05, 2021 at 03:40:33PM +0200, Kalle Valo wrote:
"Gustavo A. R. Silva" [off-list ref] writes:
quoted
In preparation to enable -Wimplicit-fallthrough for Clang, fix
multiple warnings by replacing /* fall through */ comments with
the new pseudo-keyword macro fallthrough; instead of letting the
code fall through to the next case.
Notice that Clang doesn't recognize /* fall through */ comments as
implicit fall-through markings.
Link: https://github.com/KSPP/linux/issues/115
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Hm, this conversation looks like a miscommunication, mainly? I see
Gustavo, as requested by many others[1], replacing the fallthrough
comments with the "fallthrough" statement. (This is more than just a
"Clang doesn't parse comments" issue.)
This could be a tree-wide patch and not bother you, but Greg KH has
generally advised us to send these changes broken out. Anyway, this
change still needs to land, so what would be the preferred path? I think
Gustavo could just carry it for Linus to merge without bothering you if
that'd be preferred?
-Kees
[1] https://git.kernel.org/linus/294f69e662d1570703e9b56e95be37a9fd3afba5
--
Kees Cook
On Fri, Mar 05, 2021 at 03:40:33PM +0200, Kalle Valo wrote:
quoted
"Gustavo A. R. Silva" [off-list ref] writes:
quoted
In preparation to enable -Wimplicit-fallthrough for Clang, fix
multiple warnings by replacing /* fall through */ comments with
the new pseudo-keyword macro fallthrough; instead of letting the
code fall through to the next case.
Notice that Clang doesn't recognize /* fall through */ comments as
implicit fall-through markings.
Link: https://github.com/KSPP/linux/issues/115
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Hm, this conversation looks like a miscommunication, mainly? I see
Gustavo, as requested by many others[1], replacing the fallthrough
comments with the "fallthrough" statement. (This is more than just a
"Clang doesn't parse comments" issue.)
This could be a tree-wide patch and not bother you, but Greg KH has
generally advised us to send these changes broken out. Anyway, this
change still needs to land, so what would be the preferred path? I think
Gustavo could just carry it for Linus to merge without bothering you if
that'd be preferred?
I'll respond with the same I did last time, fallthrough is not C and
it's ugly.
Instead of mutilating the kernel, Gustavo should invest in fixing the
broken clang compiler.
Thanks,
Jes
On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
On 3/10/21 2:14 PM, Kees Cook wrote:
quoted
On Fri, Mar 05, 2021 at 03:40:33PM +0200, Kalle Valo wrote:
quoted
"Gustavo A. R. Silva" [off-list ref] writes:
quoted
In preparation to enable -Wimplicit-fallthrough for Clang, fix
multiple warnings by replacing /* fall through */ comments with
the new pseudo-keyword macro fallthrough; instead of letting the
code fall through to the next case.
Notice that Clang doesn't recognize /* fall through */ comments as
implicit fall-through markings.
Link: https://github.com/KSPP/linux/issues/115
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Hm, this conversation looks like a miscommunication, mainly? I see
Gustavo, as requested by many others[1], replacing the fallthrough
comments with the "fallthrough" statement. (This is more than just a
"Clang doesn't parse comments" issue.)
This could be a tree-wide patch and not bother you, but Greg KH has
generally advised us to send these changes broken out. Anyway, this
change still needs to land, so what would be the preferred path? I think
Gustavo could just carry it for Linus to merge without bothering you if
that'd be preferred?
I'll respond with the same I did last time, fallthrough is not C and
it's ugly.
I understand your point of view, but this is not the consensus[1] of
the community. "fallthrough" is a macro, using the GCC fallthrough
attribute, with the expectation that we can move to the C17/C18
"[[fallthrough]]" statement once it is finalized by the C standards
body.
-Kees
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html#implicit-switch-case-fall-through
--
Kees Cook
On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
quoted
On 3/10/21 2:14 PM, Kees Cook wrote:
quoted
Hm, this conversation looks like a miscommunication, mainly? I see
Gustavo, as requested by many others[1], replacing the fallthrough
comments with the "fallthrough" statement. (This is more than just a
"Clang doesn't parse comments" issue.)
This could be a tree-wide patch and not bother you, but Greg KH has
generally advised us to send these changes broken out. Anyway, this
change still needs to land, so what would be the preferred path? I think
Gustavo could just carry it for Linus to merge without bothering you if
that'd be preferred?
I'll respond with the same I did last time, fallthrough is not C and
it's ugly.
I understand your point of view, but this is not the consensus[1] of
the community. "fallthrough" is a macro, using the GCC fallthrough
attribute, with the expectation that we can move to the C17/C18
"[[fallthrough]]" statement once it is finalized by the C standards
body.
I don't know who decided on that, but I still disagree. It's an ugly and
pointless change that serves little purpose. We shouldn't have allowed
the ugly /* fall-through */ comments in either, but at least they didn't
mess with the code. I guess when you give someone an inch, they take a mile.
Last time this came up, the discussion was that clang refused to fix
their brokenness and therefore this nonsense was being pushed into the
kernel. It's still a pointless argument, if clang can't fix it's crap,
then stop using it.
As Kalle correctly pointed out, none of the previous comments to this
were addressed, the patches were just reposted as fact. Not exactly a
nice way to go about it either.
Jes
On Wed, Mar 10, 2021 at 02:51:24PM -0500, Jes Sorensen wrote:
On 3/10/21 2:45 PM, Kees Cook wrote:
quoted
On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
quoted
On 3/10/21 2:14 PM, Kees Cook wrote:
quoted
Hm, this conversation looks like a miscommunication, mainly? I see
Gustavo, as requested by many others[1], replacing the fallthrough
comments with the "fallthrough" statement. (This is more than just a
"Clang doesn't parse comments" issue.)
This could be a tree-wide patch and not bother you, but Greg KH has
generally advised us to send these changes broken out. Anyway, this
change still needs to land, so what would be the preferred path? I think
Gustavo could just carry it for Linus to merge without bothering you if
that'd be preferred?
I'll respond with the same I did last time, fallthrough is not C and
it's ugly.
I understand your point of view, but this is not the consensus[1] of
the community. "fallthrough" is a macro, using the GCC fallthrough
attribute, with the expectation that we can move to the C17/C18
"[[fallthrough]]" statement once it is finalized by the C standards
body.
I don't know who decided on that, but I still disagree. It's an ugly and
pointless change that serves little purpose. We shouldn't have allowed
the ugly /* fall-through */ comments in either, but at least they didn't
mess with the code. I guess when you give someone an inch, they take a mile.
Last time this came up, the discussion was that clang refused to fix
their brokenness and therefore this nonsense was being pushed into the
kernel. It's still a pointless argument, if clang can't fix it's crap,
then stop using it.
As Kalle correctly pointed out, none of the previous comments to this
were addressed, the patches were just reposted as fact. Not exactly a
nice way to go about it either.
Do you mean changing the commit log to re-justify these changes? I
guess that could be done, but based on the thread, it didn't seem to
be needed. The change is happening to match the coding style consensus
reached to give the kernel the flexibility to move from a gcc extension
to the final C standards committee results without having to do treewide
commits again (i.e. via the macro).
--
Kees Cook
On Wed, Mar 10, 2021 at 02:51:24PM -0500, Jes Sorensen wrote:
quoted
On 3/10/21 2:45 PM, Kees Cook wrote:
quoted
On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
quoted
On 3/10/21 2:14 PM, Kees Cook wrote:
quoted
Hm, this conversation looks like a miscommunication, mainly? I see
Gustavo, as requested by many others[1], replacing the fallthrough
comments with the "fallthrough" statement. (This is more than just a
"Clang doesn't parse comments" issue.)
This could be a tree-wide patch and not bother you, but Greg KH has
generally advised us to send these changes broken out. Anyway, this
change still needs to land, so what would be the preferred path? I think
Gustavo could just carry it for Linus to merge without bothering you if
that'd be preferred?
I'll respond with the same I did last time, fallthrough is not C and
it's ugly.
I understand your point of view, but this is not the consensus[1] of
the community. "fallthrough" is a macro, using the GCC fallthrough
attribute, with the expectation that we can move to the C17/C18
"[[fallthrough]]" statement once it is finalized by the C standards
body.
I don't know who decided on that, but I still disagree. It's an ugly and
pointless change that serves little purpose. We shouldn't have allowed
the ugly /* fall-through */ comments in either, but at least they didn't
mess with the code. I guess when you give someone an inch, they take a mile.
Last time this came up, the discussion was that clang refused to fix
their brokenness and therefore this nonsense was being pushed into the
kernel. It's still a pointless argument, if clang can't fix it's crap,
then stop using it.
As Kalle correctly pointed out, none of the previous comments to this
were addressed, the patches were just reposted as fact. Not exactly a
nice way to go about it either.
Do you mean changing the commit log to re-justify these changes? I
guess that could be done, but based on the thread, it didn't seem to
be needed. The change is happening to match the coding style consensus
reached to give the kernel the flexibility to move from a gcc extension
to the final C standards committee results without having to do treewide
commits again (i.e. via the macro).
No, I am questioning why Gustavo continues to push this nonsense that
serves no purpose whatsoever. In addition he has consistently ignored
comments and just keep reposting it. But I guess that is how it works,
ignore feedback, repost junk, repeat.
Jes
From: Gustavo A. R. Silva <hidden> Date: 2021-04-17 19:24:50
On 4/17/21 13:29, Jes Sorensen wrote:
On 3/10/21 3:59 PM, Kees Cook wrote:
quoted
On Wed, Mar 10, 2021 at 02:51:24PM -0500, Jes Sorensen wrote:
quoted
On 3/10/21 2:45 PM, Kees Cook wrote:
quoted
On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
quoted
On 3/10/21 2:14 PM, Kees Cook wrote:
quoted
Hm, this conversation looks like a miscommunication, mainly? I see
Gustavo, as requested by many others[1], replacing the fallthrough
comments with the "fallthrough" statement. (This is more than just a
"Clang doesn't parse comments" issue.)
This could be a tree-wide patch and not bother you, but Greg KH has
generally advised us to send these changes broken out. Anyway, this
change still needs to land, so what would be the preferred path? I think
Gustavo could just carry it for Linus to merge without bothering you if
that'd be preferred?
I'll respond with the same I did last time, fallthrough is not C and
it's ugly.
I understand your point of view, but this is not the consensus[1] of
the community. "fallthrough" is a macro, using the GCC fallthrough
attribute, with the expectation that we can move to the C17/C18
"[[fallthrough]]" statement once it is finalized by the C standards
body.
I don't know who decided on that, but I still disagree. It's an ugly and
pointless change that serves little purpose. We shouldn't have allowed
the ugly /* fall-through */ comments in either, but at least they didn't
mess with the code. I guess when you give someone an inch, they take a mile.
Last time this came up, the discussion was that clang refused to fix
their brokenness and therefore this nonsense was being pushed into the
kernel. It's still a pointless argument, if clang can't fix it's crap,
then stop using it.
As Kalle correctly pointed out, none of the previous comments to this
were addressed, the patches were just reposted as fact. Not exactly a
nice way to go about it either.
Do you mean changing the commit log to re-justify these changes? I
guess that could be done, but based on the thread, it didn't seem to
be needed. The change is happening to match the coding style consensus
reached to give the kernel the flexibility to move from a gcc extension
to the final C standards committee results without having to do treewide
commits again (i.e. via the macro).
No, I am questioning why Gustavo continues to push this nonsense that
serves no purpose whatsoever. In addition he has consistently ignored
comments and just keep reposting it. But I guess that is how it works,
ignore feedback, repost junk, repeat.
On Wed, Mar 10, 2021 at 02:51:24PM -0500, Jes Sorensen wrote:
quoted
On 3/10/21 2:45 PM, Kees Cook wrote:
quoted
On Wed, Mar 10, 2021 at 02:31:57PM -0500, Jes Sorensen wrote:
quoted
On 3/10/21 2:14 PM, Kees Cook wrote:
quoted
Hm, this conversation looks like a miscommunication, mainly? I see
Gustavo, as requested by many others[1], replacing the fallthrough
comments with the "fallthrough" statement. (This is more than just a
"Clang doesn't parse comments" issue.)
This could be a tree-wide patch and not bother you, but Greg KH has
generally advised us to send these changes broken out. Anyway, this
change still needs to land, so what would be the preferred path? I think
Gustavo could just carry it for Linus to merge without bothering you if
that'd be preferred?
I'll respond with the same I did last time, fallthrough is not C and
it's ugly.
I understand your point of view, but this is not the consensus[1] of
the community. "fallthrough" is a macro, using the GCC fallthrough
attribute, with the expectation that we can move to the C17/C18
"[[fallthrough]]" statement once it is finalized by the C standards
body.
I don't know who decided on that, but I still disagree. It's an ugly and
pointless change that serves little purpose. We shouldn't have allowed
the ugly /* fall-through */ comments in either, but at least they didn't
mess with the code. I guess when you give someone an inch, they take a mile.
Last time this came up, the discussion was that clang refused to fix
their brokenness and therefore this nonsense was being pushed into the
kernel. It's still a pointless argument, if clang can't fix it's crap,
then stop using it.
As Kalle correctly pointed out, none of the previous comments to this
were addressed, the patches were just reposted as fact. Not exactly a
nice way to go about it either.
Do you mean changing the commit log to re-justify these changes? I
guess that could be done, but based on the thread, it didn't seem to
be needed. The change is happening to match the coding style consensus
reached to give the kernel the flexibility to move from a gcc extension
to the final C standards committee results without having to do treewide
commits again (i.e. via the macro).
No, I am questioning why Gustavo continues to push this nonsense that
serves no purpose whatsoever. In addition he has consistently ignored
comments and just keep reposting it. But I guess that is how it works,
ignore feedback, repost junk, repeat.
I was asking for feedback here[1] and here[2] after people (you and Kalle)
commented on this patch. How is that ignoring people? And -again- why
people ignored my requests for feedback in this conversation? It's a mystery
to me, honestly.
All you did was post a pointer to the fact that some other people
couldn't be bothered speaking out against the patch, and let it go in.
You haven't addressed any of the original concerns raised.
The big mistake here was of course to allow the pointless /* fallthrough
*/ changes to go in in the first place.
Jes