* [PATCH] mtd: cfi_util: mark expected switch fall-throughs @ 2019-02-08 18:02 Gustavo A. R. Silva 2019-03-20 20:20 ` Gustavo A. R. Silva 2019-04-10 21:46 ` Kees Cook 0 siblings, 2 replies; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-02-08 18:02 UTC (permalink / raw) To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut, Richard Weinberger Cc: linux-mtd, linux-kernel, Gustavo A. R. Silva In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. This patch fixes the following warnings: drivers/mtd/chips/cfi_util.c: In function ‘cfi_build_cmd’: drivers/mtd/chips/cfi_util.c:110:10: warning: this statement may fall through [-Wimplicit-fallthrough=] onecmd |= (onecmd << (chip_mode * 32)); ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/mtd/chips/cfi_util.c:112:2: note: here case 4: ^~~~ drivers/mtd/chips/cfi_util.c:113:10: warning: this statement may fall through [-Wimplicit-fallthrough=] onecmd |= (onecmd << (chip_mode * 16)); ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/mtd/chips/cfi_util.c:114:2: note: here case 2: ^~~~ drivers/mtd/chips/cfi_util.c: In function ‘cfi_merge_status’: drivers/mtd/chips/cfi_util.c:163:7: warning: this statement may fall through [-Wimplicit-fallthrough=] res |= (onestat >> (chip_mode * 32)); ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/mtd/chips/cfi_util.c:165:2: note: here case 4: ^~~~ drivers/mtd/chips/cfi_util.c:166:7: warning: this statement may fall through [-Wimplicit-fallthrough=] res |= (onestat >> (chip_mode * 16)); ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/mtd/chips/cfi_util.c:167:2: note: here case 2: ^~~~ Warning level 3 was used: -Wimplicit-fallthrough=3 This patch is part of the ongoing efforts to enabling -Wimplicit-fallthrough. Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/mtd/chips/cfi_util.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/mtd/chips/cfi_util.c b/drivers/mtd/chips/cfi_util.c index 6f16552cd59f..e3b266ee06af 100644 --- a/drivers/mtd/chips/cfi_util.c +++ b/drivers/mtd/chips/cfi_util.c @@ -109,10 +109,13 @@ map_word cfi_build_cmd(u_long cmd, struct map_info *map, struct cfi_private *cfi case 8: onecmd |= (onecmd << (chip_mode * 32)); #endif + /* fall through */ case 4: onecmd |= (onecmd << (chip_mode * 16)); + /* fall through */ case 2: onecmd |= (onecmd << (chip_mode * 8)); + /* fall through */ case 1: ; } @@ -162,10 +165,13 @@ unsigned long cfi_merge_status(map_word val, struct map_info *map, case 8: res |= (onestat >> (chip_mode * 32)); #endif + /* fall through */ case 4: res |= (onestat >> (chip_mode * 16)); + /* fall through */ case 2: res |= (onestat >> (chip_mode * 8)); + /* fall through */ case 1: ; } -- 2.20.1 ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-02-08 18:02 [PATCH] mtd: cfi_util: mark expected switch fall-throughs Gustavo A. R. Silva @ 2019-03-20 20:20 ` Gustavo A. R. Silva 2019-03-20 21:05 ` Richard Weinberger 2019-04-10 21:16 ` Gustavo A. R. Silva 2019-04-10 21:46 ` Kees Cook 1 sibling, 2 replies; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-03-20 20:20 UTC (permalink / raw) To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut, Richard Weinberger Cc: linux-mtd, linux-kernel, Kees Cook Hi all, Friendly ping: Who can take this? Thanks -- Gustavo On 2/8/19 12:02 PM, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch > cases where we are expecting to fall through. > > This patch fixes the following warnings: > > drivers/mtd/chips/cfi_util.c: In function ‘cfi_build_cmd’: > drivers/mtd/chips/cfi_util.c:110:10: warning: this statement may fall through [-Wimplicit-fallthrough=] > onecmd |= (onecmd << (chip_mode * 32)); > ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/mtd/chips/cfi_util.c:112:2: note: here > case 4: > ^~~~ > drivers/mtd/chips/cfi_util.c:113:10: warning: this statement may fall through [-Wimplicit-fallthrough=] > onecmd |= (onecmd << (chip_mode * 16)); > ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/mtd/chips/cfi_util.c:114:2: note: here > case 2: > ^~~~ > drivers/mtd/chips/cfi_util.c: In function ‘cfi_merge_status’: > drivers/mtd/chips/cfi_util.c:163:7: warning: this statement may fall through [-Wimplicit-fallthrough=] > res |= (onestat >> (chip_mode * 32)); > ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/mtd/chips/cfi_util.c:165:2: note: here > case 4: > ^~~~ > drivers/mtd/chips/cfi_util.c:166:7: warning: this statement may fall through [-Wimplicit-fallthrough=] > res |= (onestat >> (chip_mode * 16)); > ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/mtd/chips/cfi_util.c:167:2: note: here > case 2: > ^~~~ > > Warning level 3 was used: -Wimplicit-fallthrough=3 > > This patch is part of the ongoing efforts to enabling > -Wimplicit-fallthrough. > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > --- > drivers/mtd/chips/cfi_util.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/mtd/chips/cfi_util.c b/drivers/mtd/chips/cfi_util.c > index 6f16552cd59f..e3b266ee06af 100644 > --- a/drivers/mtd/chips/cfi_util.c > +++ b/drivers/mtd/chips/cfi_util.c > @@ -109,10 +109,13 @@ map_word cfi_build_cmd(u_long cmd, struct map_info *map, struct cfi_private *cfi > case 8: > onecmd |= (onecmd << (chip_mode * 32)); > #endif > + /* fall through */ > case 4: > onecmd |= (onecmd << (chip_mode * 16)); > + /* fall through */ > case 2: > onecmd |= (onecmd << (chip_mode * 8)); > + /* fall through */ > case 1: > ; > } > @@ -162,10 +165,13 @@ unsigned long cfi_merge_status(map_word val, struct map_info *map, > case 8: > res |= (onestat >> (chip_mode * 32)); > #endif > + /* fall through */ > case 4: > res |= (onestat >> (chip_mode * 16)); > + /* fall through */ > case 2: > res |= (onestat >> (chip_mode * 8)); > + /* fall through */ > case 1: > ; > } > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-03-20 20:20 ` Gustavo A. R. Silva @ 2019-03-20 21:05 ` Richard Weinberger 2019-03-20 21:23 ` Gustavo A. R. Silva 2019-04-10 21:16 ` Gustavo A. R. Silva 1 sibling, 1 reply; 16+ messages in thread From: Richard Weinberger @ 2019-03-20 21:05 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Kees Cook, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi! Am Mittwoch, 20. März 2019, 21:20:51 CET schrieb Gustavo A. R. Silva: > Hi all, > > Friendly ping: > > Who can take this? Hmmm, for MTD I think we can schedule these patches for the next merge window. But I'm not sure whether these comments are a good solution. I much more prefer the compiler attribute solution. Also a tree-wide (sane) coccinelle script would be better IMHO, and not zillions of individual patches via different trees. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-03-20 21:05 ` Richard Weinberger @ 2019-03-20 21:23 ` Gustavo A. R. Silva 2019-03-20 23:25 ` Richard Weinberger 0 siblings, 1 reply; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-03-20 21:23 UTC (permalink / raw) To: Richard Weinberger Cc: Kees Cook, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse On 3/20/19 4:05 PM, Richard Weinberger wrote: > Hi! > > Am Mittwoch, 20. März 2019, 21:20:51 CET schrieb Gustavo A. R. Silva: >> Hi all, >> >> Friendly ping: >> >> Who can take this? > > Hmmm, for MTD I think we can schedule these patches for the next merge window. > But I'm not sure whether these comments are a good solution. > I much more prefer the compiler attribute solution. > Also a tree-wide (sane) coccinelle script would be better IMHO, > and not zillions of individual patches via different trees. > If that script is sophisticated enough to spot both false positives and actual bugs, then yeah, I'd love that solution. But it's actually not that simple. The thing about the compiler attributes is not a big deal, once we finally enable -Wimplicit-fallthrough, for which we first need to address each one of these cases. But we are getting close, there are less than 100 of these issues in linux-next. :) So, if this is too much of a burden for people, I can add all these MTD patches to my own tree for 5.2. Just, please, let me know. Thanks -- Gustavo ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-03-20 21:23 ` Gustavo A. R. Silva @ 2019-03-20 23:25 ` Richard Weinberger 2019-03-20 23:36 ` Gustavo A. R. Silva 0 siblings, 1 reply; 16+ messages in thread From: Richard Weinberger @ 2019-03-20 23:25 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Kees Cook, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Am Mittwoch, 20. März 2019, 22:23:21 CET schrieb Gustavo A. R. Silva: > So, if this is too much of a burden for people, I can add all these > MTD patches to my own tree for 5.2. Taking the patches via MTD is not a big deal, let's go that path. For you it is more work to send many patches and getting them into the individual trees. :) Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-03-20 23:25 ` Richard Weinberger @ 2019-03-20 23:36 ` Gustavo A. R. Silva 0 siblings, 0 replies; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-03-20 23:36 UTC (permalink / raw) To: Richard Weinberger Cc: Kees Cook, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse On 3/20/19 6:25 PM, Richard Weinberger wrote: > Am Mittwoch, 20. März 2019, 22:23:21 CET schrieb Gustavo A. R. Silva: >> So, if this is too much of a burden for people, I can add all these >> MTD patches to my own tree for 5.2. > > Taking the patches via MTD is not a big deal, let's go that path. Cool. > For you it is more work to send many patches and getting them into the individual > trees. :) > Here applies this wise phrase: It is what it is. :) Thanks -- Gustavo ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-03-20 20:20 ` Gustavo A. R. Silva 2019-03-20 21:05 ` Richard Weinberger @ 2019-04-10 21:16 ` Gustavo A. R. Silva 2019-04-15 8:44 ` Miquel Raynal 1 sibling, 1 reply; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-04-10 21:16 UTC (permalink / raw) To: David Woodhouse, Brian Norris, Boris Brezillon, Marek Vasut, Richard Weinberger Cc: linux-mtd, linux-kernel, Kees Cook Hi all, If no one cares I'll add this to my tree for 5.2. Thanks -- Gustavo On 3/20/19 3:20 PM, Gustavo A. R. Silva wrote: > Hi all, > > Friendly ping: > > Who can take this? > > Thanks > -- > Gustavo > > On 2/8/19 12:02 PM, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wimplicit-fallthrough, mark switch >> cases where we are expecting to fall through. >> >> This patch fixes the following warnings: >> >> drivers/mtd/chips/cfi_util.c: In function ‘cfi_build_cmd’: >> drivers/mtd/chips/cfi_util.c:110:10: warning: this statement may fall through [-Wimplicit-fallthrough=] >> onecmd |= (onecmd << (chip_mode * 32)); >> ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/mtd/chips/cfi_util.c:112:2: note: here >> case 4: >> ^~~~ >> drivers/mtd/chips/cfi_util.c:113:10: warning: this statement may fall through [-Wimplicit-fallthrough=] >> onecmd |= (onecmd << (chip_mode * 16)); >> ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/mtd/chips/cfi_util.c:114:2: note: here >> case 2: >> ^~~~ >> drivers/mtd/chips/cfi_util.c: In function ‘cfi_merge_status’: >> drivers/mtd/chips/cfi_util.c:163:7: warning: this statement may fall through [-Wimplicit-fallthrough=] >> res |= (onestat >> (chip_mode * 32)); >> ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/mtd/chips/cfi_util.c:165:2: note: here >> case 4: >> ^~~~ >> drivers/mtd/chips/cfi_util.c:166:7: warning: this statement may fall through [-Wimplicit-fallthrough=] >> res |= (onestat >> (chip_mode * 16)); >> ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> drivers/mtd/chips/cfi_util.c:167:2: note: here >> case 2: >> ^~~~ >> >> Warning level 3 was used: -Wimplicit-fallthrough=3 >> >> This patch is part of the ongoing efforts to enabling >> -Wimplicit-fallthrough. >> >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> --- >> drivers/mtd/chips/cfi_util.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/mtd/chips/cfi_util.c b/drivers/mtd/chips/cfi_util.c >> index 6f16552cd59f..e3b266ee06af 100644 >> --- a/drivers/mtd/chips/cfi_util.c >> +++ b/drivers/mtd/chips/cfi_util.c >> @@ -109,10 +109,13 @@ map_word cfi_build_cmd(u_long cmd, struct map_info *map, struct cfi_private *cfi >> case 8: >> onecmd |= (onecmd << (chip_mode * 32)); >> #endif >> + /* fall through */ >> case 4: >> onecmd |= (onecmd << (chip_mode * 16)); >> + /* fall through */ >> case 2: >> onecmd |= (onecmd << (chip_mode * 8)); >> + /* fall through */ >> case 1: >> ; >> } >> @@ -162,10 +165,13 @@ unsigned long cfi_merge_status(map_word val, struct map_info *map, >> case 8: >> res |= (onestat >> (chip_mode * 32)); >> #endif >> + /* fall through */ >> case 4: >> res |= (onestat >> (chip_mode * 16)); >> + /* fall through */ >> case 2: >> res |= (onestat >> (chip_mode * 8)); >> + /* fall through */ >> case 1: >> ; >> } >> ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-04-10 21:16 ` Gustavo A. R. Silva @ 2019-04-15 8:44 ` Miquel Raynal 2019-04-15 12:57 ` Gustavo A. R. Silva 0 siblings, 1 reply; 16+ messages in thread From: Miquel Raynal @ 2019-04-15 8:44 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Kees Cook, Boris Brezillon, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi Gustavo, "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Wed, 10 Apr 2019 16:16:51 -0500: > Hi all, > > If no one cares I'll add this to my tree for 5.2. Which tree are you talking about? Please let the MTD maintainers take patches through their tree. We might be late but this is definitely not a good reason to bypass us. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-04-15 8:44 ` Miquel Raynal @ 2019-04-15 12:57 ` Gustavo A. R. Silva 2019-04-16 17:24 ` Miquel Raynal 0 siblings, 1 reply; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-04-15 12:57 UTC (permalink / raw) To: Miquel Raynal Cc: Kees Cook, Boris Brezillon, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi Miquel, On 4/15/19 3:44 AM, Miquel Raynal wrote: > Hi Gustavo, > > "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Wed, 10 Apr > 2019 16:16:51 -0500: > >> Hi all, >> >> If no one cares I'll add this to my tree for 5.2. > > Which tree are you talking about? > This one: https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/log/?h=for-next/kspp > Please let the MTD maintainers take patches through their tree. We > might be late but this is definitely not a good reason to bypass us. > It's a bit confusing when patches are being ignored for more than two months: https://lore.kernel.org/patchwork/patch/1040099/ https://lore.kernel.org/patchwork/patch/1040100/ https://lore.kernel.org/patchwork/patch/1040098/ Certainly, Richard Weinberger replied to this one. But I couldn't find a tree to which this patch was applied, in case it actually was. It's a common practice for maintainers to reply saying that a patch has been finally applied, and in most cases they also explicitly mention the tree and branch to which it was applied. All this info is really helpful for people working all over the tree. I'm only taking this type of patches in my tree when they are being ignored for a considerable amount of time and even after pinging the maintainers. Thanks -- Gustavo ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-04-15 12:57 ` Gustavo A. R. Silva @ 2019-04-16 17:24 ` Miquel Raynal 2019-04-16 20:49 ` Gustavo A. R. Silva 0 siblings, 1 reply; 16+ messages in thread From: Miquel Raynal @ 2019-04-16 17:24 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Kees Cook, Boris Brezillon, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi Gustavo, "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Mon, 15 Apr 2019 07:57:11 -0500: > Hi Miquel, > > On 4/15/19 3:44 AM, Miquel Raynal wrote: > > Hi Gustavo, > > > > "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Wed, 10 Apr > > 2019 16:16:51 -0500: > > > >> Hi all, > >> > >> If no one cares I'll add this to my tree for 5.2. > > > > Which tree are you talking about? > > > > This one: > > https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/log/?h=for-next/kspp > > > Please let the MTD maintainers take patches through their tree. We > > might be late but this is definitely not a good reason to bypass us. > > > It's a bit confusing when patches are being ignored for more than two > months: > > https://lore.kernel.org/patchwork/patch/1040099/ > https://lore.kernel.org/patchwork/patch/1040100/ > https://lore.kernel.org/patchwork/patch/1040098/ > Patches posted at -rc6 right before the last release? Come on! Gustavo, we always spend more time for you than for other contributors because we do not trust your changes. We could apply them blindly but we don't do that for other (worthy) contributions, so why shall we do it for you? I think you could at least flag these changes as "automatic and unverified" in the commit log so that when git blaming, people could know that the additional explicit /* fallthrough */ comment might be wrong and was just added in order to limit the number of warnings when enabling the extra GCC warning. > Certainly, Richard Weinberger replied to this one. But I couldn't > find a tree to which this patch was applied, in case it actually > was. > > It's a common practice for maintainers to reply saying that a patch > has been finally applied, and in most cases they also explicitly > mention the tree and branch to which it was applied. All this info > is really helpful for people working all over the tree. It is common practice for contributors to understand what they are doing before submitting a change and this is something that you clearly don't try to do. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-04-16 17:24 ` Miquel Raynal @ 2019-04-16 20:49 ` Gustavo A. R. Silva 2019-05-07 14:54 ` Gustavo A. R. Silva 0 siblings, 1 reply; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-04-16 20:49 UTC (permalink / raw) To: Miquel Raynal Cc: Kees Cook, Boris Brezillon, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi Miquel, On 4/16/19 12:24 PM, Miquel Raynal wrote: > Hi Gustavo, > > "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Mon, 15 Apr > 2019 07:57:11 -0500: > >> Hi Miquel, >> >> On 4/15/19 3:44 AM, Miquel Raynal wrote: >>> Hi Gustavo, >>> >>> "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Wed, 10 Apr >>> 2019 16:16:51 -0500: >>> >>>> Hi all, >>>> >>>> If no one cares I'll add this to my tree for 5.2. >>> >>> Which tree are you talking about? >>> >> >> This one: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/log/?h=for-next/kspp >> >>> Please let the MTD maintainers take patches through their tree. We >>> might be late but this is definitely not a good reason to bypass us. >>> >> It's a bit confusing when patches are being ignored for more than two >> months: >> >> https://lore.kernel.org/patchwork/patch/1040099/ >> https://lore.kernel.org/patchwork/patch/1040100/ >> https://lore.kernel.org/patchwork/patch/1040098/ >> > > Patches posted at -rc6 right before the last release? Come on! Gustavo, > we always spend more time for you than for other contributors because we > do not trust your changes. We could apply them blindly but we don't do > that for other (worthy) contributions, so why shall we do it for you? > Oh, I didn't know about that. You don't have to blindly trust me. I sincerely think people should always double check any changes, regardless of their level of trust in a particular person/entity. Anyway, I really appreciate your sincerity because now I think we can come up with a good strategy to collaborate with each other smoothly. It seems to me that the root cause for this lack of trust, and, maybe even despite towards these type of patches, is basically misunderstanding of what I'm trying to accomplish and, more importantly, how I do it. > I think you could at least flag these changes as "automatic and > unverified" in the commit log so that when git blaming, people could > know that the additional explicit /* fallthrough */ comment might be > wrong and was just added in order to limit the number of warnings when > enabling the extra GCC warning. > I don't do that because that's not how I'm tackling this task. I'm not sending these patches with the intention of merely accumulate contributions --and I'm not saying you say so, this is just for clarification-- or because of a lack of more technically interesting things to do in the kernel --this is certainly not the only thing I'm working on. What I'm trying to accomplish is to be able to add -Wimplicit-fallthrough to the build so that the kernel will stay entirely free of this class of bug going forward; is that simple. Now, why is that? because sometimes people forget to place a break/return and a bug is introduced, and it could take up to 7 years to fix it [1]. Now, I really try to determine if I'm dealing with a false positive or an actual bug every time. I read the code and try to understand the context around which each warning is reported. You can tell it's not the most sexy and glamorous thing. And a static analyzer is clearly not sophisticated enough to spot actual bugs in this situation, not even the Coccinelle tool. I had a similar conversation with a wireless maintainer a while ago. He claimed I was not even looking at the code and that I was blindly using a transformation tool [2]. Please, take a look at it, so you can better understand my workflow. I have gone through this process of reading code all over the tree and trying to understand it hundreds of times; there were more than 2000 of these warnings at the time I started working on this, and there are are around 50 left in linux-next. Of course, the vast majority of cases have resulted to be obvious false positives, but it's me who have determined that, by auditing each case, so I haven't blindly placed any fall-through comment. Now, have I made any mistake? Of course! but I have also amended it immediately [3][4]. And the number of bugs I have fixed while working on this task is much bigger. A clear example of how hard this can be is documented in this thread, in which you, being an MTD maintainer, cannot clearly determine if this is a false positive or an actual bug [5]. It can be troublesome for you for a number of reasons --I'm not judging that. I'm trying to illustrate the magnitude of the task as a whole. So, this patches together with the related bugfixes are part of that whole. And, although sometimes painful for everyone, that whole is what's important, and worth it. >> Certainly, Richard Weinberger replied to this one. But I couldn't >> find a tree to which this patch was applied, in case it actually >> was. >> >> It's a common practice for maintainers to reply saying that a patch >> has been finally applied, and in most cases they also explicitly >> mention the tree and branch to which it was applied. All this info >> is really helpful for people working all over the tree. > > It is common practice for contributors to understand what they > are doing before submitting a change and this is something that you > clearly don't try to do. > This is too much to say, and sadly, it's not uncommon for even the most senior people to assume others don't even make an effort to think through their work, before at least asking. But I have already explained myself above. Regarding this: > Patches posted at -rc6 right before the last release? Come on! Gustavo, I don't expect people to send an urgent pull-request to merge this patches into mainline as soon as they arrive, and I have never requested such thing. Lastly, what I really want we *all* get out of this conversation is a better way to collaborate with each other. For me, and I guess for most contributors, it's good enough to have a confirmation that the accepted patch has been applied to a certain branch in a certain tree. I understand this may sound like an special request, in particular because, currently, the number of people working all over the tree is not that big, so it is not that critical for maintainers to adopt certain practices that benefits this small group of contributors, but thanks to recent initiatives as The Linux Kernel Mentorship project I think this is going to change and it will force us all to evolve in the right direction. By the way, notice that these are the last patches for MTD. :) Thank you -- Gustavo References: [1] https://lore.kernel.org/patchwork/patch/1042976/ [2] https://lore.kernel.org/patchwork/patch/1002568/ [3] https://lore.kernel.org/patchwork/patch/970617/ [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.1-rc5&id=ad0eaee6195db1db1749dd46b9e6f4466793d178 [5] https://lore.kernel.org/patchwork/patch/1036251/ ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-04-16 20:49 ` Gustavo A. R. Silva @ 2019-05-07 14:54 ` Gustavo A. R. Silva 2019-05-07 15:49 ` Richard Weinberger 0 siblings, 1 reply; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-05-07 14:54 UTC (permalink / raw) To: Miquel Raynal Cc: Kees Cook, Boris Brezillon, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi all, Thanks a lot for this, Richard: https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=mtd%2Fnext&qt=grep&q=fall-through There are only two of these warnings left to be addressed in MTD[1]: > @@ -3280,12 +3280,14 @@ static void onenand_check_features(struct mtd_info *mtd) > if ((this->version_id & 0xf) == 0xe) > this->options |= ONENAND_HAS_NOP_1; > } > + /* fall through */ > > case ONENAND_DEVICE_DENSITY_2Gb: > /* 2Gb DDP does not have 2 plane */ > if (!ONENAND_IS_DDP(this)) > this->options |= ONENAND_HAS_2PLANE; > this->options |= ONENAND_HAS_UNLOCK_ALL; > + /* fall through */ This looks strange. In ONENAND_DEVICE_DENSITY_2Gb: ONENAND_HAS_UNLOCK_ALL is set unconditionally. But then, under ONENAND_DEVICE_DENSITY_1Gb, the same option is set only if process is evaluated to true. Same problem with ONENAND_HAS_2PLANE: - it is set in ONENAND_DEVICE_DENSITY_4Gb only if ONENAND_IS_DDP() - it is unset in ONENAND_DEVICE_DENSITY_2Gb only if !ONENAND_IS_DDP() Maybe this portion should be reworked because I am unsure if this is a missing fall through or a bug. Thanks -- Gustavo [1] https://lore.kernel.org/patchwork/patch/1036251/ On 4/16/19 3:49 PM, Gustavo A. R. Silva wrote: > Hi Miquel, > > On 4/16/19 12:24 PM, Miquel Raynal wrote: >> Hi Gustavo, >> >> "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Mon, 15 Apr >> 2019 07:57:11 -0500: >> >>> Hi Miquel, >>> >>> On 4/15/19 3:44 AM, Miquel Raynal wrote: >>>> Hi Gustavo, >>>> >>>> "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Wed, 10 Apr >>>> 2019 16:16:51 -0500: >>>> >>>>> Hi all, >>>>> >>>>> If no one cares I'll add this to my tree for 5.2. >>>> >>>> Which tree are you talking about? >>>> >>> >>> This one: >>> >>> https://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux.git/log/?h=for-next/kspp >>> >>>> Please let the MTD maintainers take patches through their tree. We >>>> might be late but this is definitely not a good reason to bypass us. >>>> >>> It's a bit confusing when patches are being ignored for more than two >>> months: >>> >>> https://lore.kernel.org/patchwork/patch/1040099/ >>> https://lore.kernel.org/patchwork/patch/1040100/ >>> https://lore.kernel.org/patchwork/patch/1040098/ >>> >> >> Patches posted at -rc6 right before the last release? Come on! Gustavo, >> we always spend more time for you than for other contributors because we >> do not trust your changes. We could apply them blindly but we don't do >> that for other (worthy) contributions, so why shall we do it for you? >> > > Oh, I didn't know about that. You don't have to blindly trust me. > I sincerely think people should always double check any changes, > regardless of their level of trust in a particular person/entity. > > Anyway, I really appreciate your sincerity because now I think we can > come up with a good strategy to collaborate with each other smoothly. > > It seems to me that the root cause for this lack of trust, and, maybe > even despite towards these type of patches, is basically misunderstanding > of what I'm trying to accomplish and, more importantly, how I do it. > >> I think you could at least flag these changes as "automatic and >> unverified" in the commit log so that when git blaming, people could >> know that the additional explicit /* fallthrough */ comment might be >> wrong and was just added in order to limit the number of warnings when >> enabling the extra GCC warning. >> > > I don't do that because that's not how I'm tackling this task. > > I'm not sending these patches with the intention of merely accumulate > contributions --and I'm not saying you say so, this is just for > clarification-- or because of a lack of more technically interesting > things to do in the kernel --this is certainly not the only thing I'm > working on. What I'm trying to accomplish is to be able to add > -Wimplicit-fallthrough to the build so that the kernel will stay > entirely free of this class of bug going forward; is that simple. Now, > why is that? because sometimes people forget to place a break/return > and a bug is introduced, and it could take up to 7 years to fix it [1]. > > Now, I really try to determine if I'm dealing with a false positive or > an actual bug every time. I read the code and try to understand the > context around which each warning is reported. You can tell it's not > the most sexy and glamorous thing. And a static analyzer is clearly not > sophisticated enough to spot actual bugs in this situation, not even > the Coccinelle tool. > > I had a similar conversation with a wireless maintainer a while ago. He > claimed I was not even looking at the code and that I was blindly using > a transformation tool [2]. Please, take a look at it, so you can better > understand my workflow. > > I have gone through this process of reading code all over the tree and > trying to understand it hundreds of times; there were more than 2000 of > these warnings at the time I started working on this, and there are are > around 50 left in linux-next. Of course, the vast majority of cases have > resulted to be obvious false positives, but it's me who have determined > that, by auditing each case, so I haven't blindly placed any fall-through > comment. > > Now, have I made any mistake? Of course! but I have also amended it > immediately [3][4]. And the number of bugs I have fixed while working > on this task is much bigger. A clear example of how hard this can be is > documented in this thread, in which you, being an MTD maintainer, cannot > clearly determine if this is a false positive or an actual bug [5]. It > can be troublesome for you for a number of reasons --I'm not judging that. > I'm trying to illustrate the magnitude of the task as a whole. > > So, this patches together with the related bugfixes are part of that > whole. And, although sometimes painful for everyone, that whole is > what's important, and worth it. > >>> Certainly, Richard Weinberger replied to this one. But I couldn't >>> find a tree to which this patch was applied, in case it actually >>> was. >>> >>> It's a common practice for maintainers to reply saying that a patch >>> has been finally applied, and in most cases they also explicitly >>> mention the tree and branch to which it was applied. All this info >>> is really helpful for people working all over the tree. >> >> It is common practice for contributors to understand what they >> are doing before submitting a change and this is something that you >> clearly don't try to do. >> > > This is too much to say, and sadly, it's not uncommon for even the most > senior people to assume others don't even make an effort to think through > their work, before at least asking. But I have already explained myself > above. > > Regarding this: > >> Patches posted at -rc6 right before the last release? Come on! Gustavo, > > I don't expect people to send an urgent pull-request to merge this > patches into mainline as soon as they arrive, and I have never requested > such thing. > > Lastly, what I really want we *all* get out of this conversation is a > better way to collaborate with each other. For me, and I guess for most > contributors, it's good enough to have a confirmation that the accepted > patch has been applied to a certain branch in a certain tree. I understand > this may sound like an special request, in particular because, currently, > the number of people working all over the tree is not that big, so it > is not that critical for maintainers to adopt certain practices that > benefits this small group of contributors, but thanks to recent initiatives > as The Linux Kernel Mentorship project I think this is going to change and > it will force us all to evolve in the right direction. > > By the way, notice that these are the last patches for MTD. :) > > Thank you > -- > Gustavo > > References: > > [1] https://lore.kernel.org/patchwork/patch/1042976/ > [2] https://lore.kernel.org/patchwork/patch/1002568/ > [3] https://lore.kernel.org/patchwork/patch/970617/ > [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.1-rc5&id=ad0eaee6195db1db1749dd46b9e6f4466793d178 > [5] https://lore.kernel.org/patchwork/patch/1036251/ > > > ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-05-07 14:54 ` Gustavo A. R. Silva @ 2019-05-07 15:49 ` Richard Weinberger 2019-05-07 15:59 ` Gustavo A. R. Silva 0 siblings, 1 reply; 16+ messages in thread From: Richard Weinberger @ 2019-05-07 15:49 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Kees Cook, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse ----- Ursprüngliche Mail ----- > Von: "Gustavo A. R. Silva" <gustavo@embeddedor.com> > An: "Miquel Raynal" <miquel.raynal@bootlin.com> > CC: "David Woodhouse" <dwmw2@infradead.org>, "Brian Norris" <computersforpeace@gmail.com>, "Boris Brezillon" > <bbrezillon@kernel.org>, "Marek Vasut" <marek.vasut@gmail.com>, "richard" <richard@nod.at>, "linux-mtd" > <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>, "Kees Cook" <keescook@chromium.org> > Gesendet: Dienstag, 7. Mai 2019 16:54:12 > Betreff: Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs > Hi all, > > Thanks a lot for this, Richard: > > https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=mtd%2Fnext&qt=grep&q=fall-through > > There are only two of these warnings left to be addressed in > MTD[1]: > > > @@ -3280,12 +3280,14 @@ static void onenand_check_features(struct mtd_info *mtd) > > if ((this->version_id & 0xf) == 0xe) > > this->options |= ONENAND_HAS_NOP_1; > > } > > + /* fall through */ > > > > case ONENAND_DEVICE_DENSITY_2Gb: > > /* 2Gb DDP does not have 2 plane */ > > if (!ONENAND_IS_DDP(this)) > > this->options |= ONENAND_HAS_2PLANE; > > this->options |= ONENAND_HAS_UNLOCK_ALL; > > + /* fall through */ > > This looks strange. > > In ONENAND_DEVICE_DENSITY_2Gb: > ONENAND_HAS_UNLOCK_ALL is set unconditionally. > > But then, under ONENAND_DEVICE_DENSITY_1Gb, the same option is set only > if process is evaluated to true. > > Same problem with ONENAND_HAS_2PLANE: > - it is set in ONENAND_DEVICE_DENSITY_4Gb only if ONENAND_IS_DDP() > - it is unset in ONENAND_DEVICE_DENSITY_2Gb only if !ONENAND_IS_DDP() > > Maybe this portion should be reworked because I am unsure if this is a > missing fall through or a bug. > > > Thanks > -- > Gustavo > > [1] https://lore.kernel.org/patchwork/patch/1036251/ Did we miss this patch? AFAICT it is in -next too. Thanks, //richard ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-05-07 15:49 ` Richard Weinberger @ 2019-05-07 15:59 ` Gustavo A. R. Silva 2019-05-09 6:53 ` Miquel Raynal 0 siblings, 1 reply; 16+ messages in thread From: Gustavo A. R. Silva @ 2019-05-07 15:59 UTC (permalink / raw) To: Richard Weinberger Cc: Kees Cook, Boris Brezillon, linux-kernel, Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris, David Woodhouse On 5/7/19 10:49 AM, Richard Weinberger wrote: >> Hi all, >> >> Thanks a lot for this, Richard: >> >> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=mtd%2Fnext&qt=grep&q=fall-through >> >> There are only two of these warnings left to be addressed in >> MTD[1]: >> >> > @@ -3280,12 +3280,14 @@ static void onenand_check_features(struct mtd_info *mtd) >> > if ((this->version_id & 0xf) == 0xe) >> > this->options |= ONENAND_HAS_NOP_1; >> > } >> > + /* fall through */ >> > >> > case ONENAND_DEVICE_DENSITY_2Gb: >> > /* 2Gb DDP does not have 2 plane */ >> > if (!ONENAND_IS_DDP(this)) >> > this->options |= ONENAND_HAS_2PLANE; >> > this->options |= ONENAND_HAS_UNLOCK_ALL; >> > + /* fall through */ >> >> This looks strange. >> >> In ONENAND_DEVICE_DENSITY_2Gb: >> ONENAND_HAS_UNLOCK_ALL is set unconditionally. >> >> But then, under ONENAND_DEVICE_DENSITY_1Gb, the same option is set only >> if process is evaluated to true. >> >> Same problem with ONENAND_HAS_2PLANE: >> - it is set in ONENAND_DEVICE_DENSITY_4Gb only if ONENAND_IS_DDP() >> - it is unset in ONENAND_DEVICE_DENSITY_2Gb only if !ONENAND_IS_DDP() >> >> Maybe this portion should be reworked because I am unsure if this is a >> missing fall through or a bug. >> >> >> Thanks >> -- >> Gustavo >> >> [1] https://lore.kernel.org/patchwork/patch/1036251/ > > Did we miss this patch? AFAICT it is in -next too. > What is pending to be resolved are these warnings: drivers/mtd/nand/onenand/onenand_base.c: In function ‘onenand_check_features’: drivers/mtd/nand/onenand/onenand_base.c:3264:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if (ONENAND_IS_DDP(this)) ^ drivers/mtd/nand/onenand/onenand_base.c:3284:2: note: here case ONENAND_DEVICE_DENSITY_2Gb: ^~~~ drivers/mtd/nand/onenand/onenand_base.c:3288:17: warning: this statement may fall through [-Wimplicit-fallthrough=] this->options |= ONENAND_HAS_UNLOCK_ALL; drivers/mtd/nand/onenand/onenand_base.c:3290:2: note: here case ONENAND_DEVICE_DENSITY_1Gb: ^~~~ The final version of the patch in -next does not address them. Thanks -- Gustavo ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-05-07 15:59 ` Gustavo A. R. Silva @ 2019-05-09 6:53 ` Miquel Raynal 0 siblings, 0 replies; 16+ messages in thread From: Miquel Raynal @ 2019-05-09 6:53 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Kees Cook, Boris Brezillon, Richard Weinberger, linux-kernel, Marek Vasut, linux-mtd, Brian Norris, David Woodhouse Hi Gustavo, "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Tue, 7 May 2019 10:59:38 -0500: > On 5/7/19 10:49 AM, Richard Weinberger wrote: > > >> Hi all, > >> > >> Thanks a lot for this, Richard: > >> > >> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/log/?h=mtd%2Fnext&qt=grep&q=fall-through > >> > >> There are only two of these warnings left to be addressed in > >> MTD[1]: > >> > >> > @@ -3280,12 +3280,14 @@ static void onenand_check_features(struct mtd_info *mtd) > >> > if ((this->version_id & 0xf) == 0xe) > >> > this->options |= ONENAND_HAS_NOP_1; > >> > } > >> > + /* fall through */ > >> > > >> > case ONENAND_DEVICE_DENSITY_2Gb: > >> > /* 2Gb DDP does not have 2 plane */ > >> > if (!ONENAND_IS_DDP(this)) > >> > this->options |= ONENAND_HAS_2PLANE; > >> > this->options |= ONENAND_HAS_UNLOCK_ALL; > >> > + /* fall through */ > >> > >> This looks strange. > >> > >> In ONENAND_DEVICE_DENSITY_2Gb: > >> ONENAND_HAS_UNLOCK_ALL is set unconditionally. > >> > >> But then, under ONENAND_DEVICE_DENSITY_1Gb, the same option is set only > >> if process is evaluated to true. > >> > >> Same problem with ONENAND_HAS_2PLANE: > >> - it is set in ONENAND_DEVICE_DENSITY_4Gb only if ONENAND_IS_DDP() > >> - it is unset in ONENAND_DEVICE_DENSITY_2Gb only if !ONENAND_IS_DDP() > >> > >> Maybe this portion should be reworked because I am unsure if this is a > >> missing fall through or a bug. > >> > >> > >> Thanks > >> -- > >> Gustavo > >> > >> [1] https://lore.kernel.org/patchwork/patch/1036251/ > > > > Did we miss this patch? AFAICT it is in -next too. > > > > What is pending to be resolved are these warnings: > > drivers/mtd/nand/onenand/onenand_base.c: In function ‘onenand_check_features’: > drivers/mtd/nand/onenand/onenand_base.c:3264:6: warning: this statement may fall through [-Wimplicit-fallthrough=] > if (ONENAND_IS_DDP(this)) > ^ > drivers/mtd/nand/onenand/onenand_base.c:3284:2: note: here > case ONENAND_DEVICE_DENSITY_2Gb: > ^~~~ > drivers/mtd/nand/onenand/onenand_base.c:3288:17: warning: this statement may fall through [-Wimplicit-fallthrough=] > this->options |= ONENAND_HAS_UNLOCK_ALL; > drivers/mtd/nand/onenand/onenand_base.c:3290:2: note: here > case ONENAND_DEVICE_DENSITY_1Gb: > ^~~~ > > The final version of the patch in -next does not address them. > Send a commit for these two warnings stating very clearly close to the top of the commit log that we don't know whether we need fallthroughs or breaks there and that this is just a change to avoid having new warnings when switching to -Wimplicit-fallthrough but this change might be entirely wrong. Thanks, Miquèl ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] mtd: cfi_util: mark expected switch fall-throughs 2019-02-08 18:02 [PATCH] mtd: cfi_util: mark expected switch fall-throughs Gustavo A. R. Silva 2019-03-20 20:20 ` Gustavo A. R. Silva @ 2019-04-10 21:46 ` Kees Cook 1 sibling, 0 replies; 16+ messages in thread From: Kees Cook @ 2019-04-10 21:46 UTC (permalink / raw) To: Gustavo A. R. Silva Cc: Boris Brezillon, Richard Weinberger, LKML, Marek Vasut, Linux mtd, Brian Norris, David Woodhouse On Fri, Feb 8, 2019 at 10:02 AM Gustavo A. R. Silva <gustavo@embeddedor.com> wrote: > > In preparation to enabling -Wimplicit-fallthrough, mark switch > cases where we are expecting to fall through. > > This patch fixes the following warnings: > > drivers/mtd/chips/cfi_util.c: In function ‘cfi_build_cmd’: > drivers/mtd/chips/cfi_util.c:110:10: warning: this statement may fall through [-Wimplicit-fallthrough=] > onecmd |= (onecmd << (chip_mode * 32)); > ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/mtd/chips/cfi_util.c:112:2: note: here > case 4: > ^~~~ > drivers/mtd/chips/cfi_util.c:113:10: warning: this statement may fall through [-Wimplicit-fallthrough=] > onecmd |= (onecmd << (chip_mode * 16)); > ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/mtd/chips/cfi_util.c:114:2: note: here > case 2: > ^~~~ > drivers/mtd/chips/cfi_util.c: In function ‘cfi_merge_status’: > drivers/mtd/chips/cfi_util.c:163:7: warning: this statement may fall through [-Wimplicit-fallthrough=] > res |= (onestat >> (chip_mode * 32)); > ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/mtd/chips/cfi_util.c:165:2: note: here > case 4: > ^~~~ > drivers/mtd/chips/cfi_util.c:166:7: warning: this statement may fall through [-Wimplicit-fallthrough=] > res |= (onestat >> (chip_mode * 16)); > ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/mtd/chips/cfi_util.c:167:2: note: here > case 2: > ^~~~ > > Warning level 3 was used: -Wimplicit-fallthrough=3 > > This patch is part of the ongoing efforts to enabling > -Wimplicit-fallthrough. > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > --- > drivers/mtd/chips/cfi_util.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/mtd/chips/cfi_util.c b/drivers/mtd/chips/cfi_util.c > index 6f16552cd59f..e3b266ee06af 100644 > --- a/drivers/mtd/chips/cfi_util.c > +++ b/drivers/mtd/chips/cfi_util.c > @@ -109,10 +109,13 @@ map_word cfi_build_cmd(u_long cmd, struct map_info *map, struct cfi_private *cfi > case 8: > onecmd |= (onecmd << (chip_mode * 32)); > #endif > + /* fall through */ > case 4: > onecmd |= (onecmd << (chip_mode * 16)); > + /* fall through */ > case 2: > onecmd |= (onecmd << (chip_mode * 8)); > + /* fall through */ > case 1: > ; > } > @@ -162,10 +165,13 @@ unsigned long cfi_merge_status(map_word val, struct map_info *map, > case 8: > res |= (onestat >> (chip_mode * 32)); > #endif > + /* fall through */ > case 4: > res |= (onestat >> (chip_mode * 16)); > + /* fall through */ > case 2: > res |= (onestat >> (chip_mode * 8)); > + /* fall through */ > case 1: > ; > } > -- > 2.20.1 > -- Kees Cook ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/ ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-05-09 6:53 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-02-08 18:02 [PATCH] mtd: cfi_util: mark expected switch fall-throughs Gustavo A. R. Silva 2019-03-20 20:20 ` Gustavo A. R. Silva 2019-03-20 21:05 ` Richard Weinberger 2019-03-20 21:23 ` Gustavo A. R. Silva 2019-03-20 23:25 ` Richard Weinberger 2019-03-20 23:36 ` Gustavo A. R. Silva 2019-04-10 21:16 ` Gustavo A. R. Silva 2019-04-15 8:44 ` Miquel Raynal 2019-04-15 12:57 ` Gustavo A. R. Silva 2019-04-16 17:24 ` Miquel Raynal 2019-04-16 20:49 ` Gustavo A. R. Silva 2019-05-07 14:54 ` Gustavo A. R. Silva 2019-05-07 15:49 ` Richard Weinberger 2019-05-07 15:59 ` Gustavo A. R. Silva 2019-05-09 6:53 ` Miquel Raynal 2019-04-10 21:46 ` Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).