mtd: nuc900_nand: mark expected switch fall-through

PATCH In review Gustavo A. R. Silva 2 participants 4 messages Jul 10, 2018 – Jul 19, 2018

Subsystems (2)

Memory technology devices (MTD) NAND flash

lore

Share & tools

lore

Gustavo A. R. Silva

author

2018-07-10 13:29 UTC #0 lore raw

PATCH 1/1 mtd: nuc900_nand: mark expected switch fall-through

In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.

Addresses-Coverity-ID: 1471717 ("Missing break in switch")

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/mtd/nand/raw/nuc900_nand.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/mtd/nand/raw/nuc900_nand.c +2 -1

index af5b32c9..53a9f6c 100644
@@ -191,8 +191,9 @@ static void nuc900_nand_command_lp(struct mtd_info *mtd, unsigned int command,
 		return;
 
 	case NAND_CMD_READ0:
-
 		write_cmd_reg(nand, NAND_CMD_READSTART);
+		/* fall through */
+
 	default:
 
 		if (!chip->dev_ready) {

Signature · 1 line

2.7.4

Miquel Raynal

maintainer

2018-07-18 08:03 UTC #1 lore raw

Hi Gustavo,

Prefix should be "mtd: rawnand: nuc900:"

"Gustavo A. R. Silva" <[email protected]> wrote on Tue, 10 Jul 2018 08:29:02 -0500:

Quoted 14 lines

In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
Addresses-Coverity-ID: 1471717 ("Missing break in switch")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/mtd/nand/raw/nuc900_nand.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/mtd/nand/raw/nuc900_nand.c +0 -0
index af5b32c9..53a9f6c 100644

drivers/mtd/nand/raw/nuc900_nand.c static void nuc900_nand_command_lp(struct mtd_info *mtd, unsigned int command, · line 195

Quoted 6 lines

drivers/mtd/nand/raw/nuc900_nand.c +1 -1

@@ -191,8 +191,9 @@ static void nuc900_nand_command_lp(struct mtd_info *mtd, unsigned int command,
 		return;
 
 	case NAND_CMD_READ0:
-
 		write_cmd_reg(nand, NAND_CMD_READSTART);
+		/* fall through */

Have you checked this is actually the right thing to do?

Thanks,
Miqu?l

Gustavo A. R. Silva

author

2018-07-19 16:09 UTC #2 lore raw

Hi Miquel,

Quoted 4 lines · Miquel Raynal

Hi Gustavo,
Prefix should be "mtd: rawnand: nuc900:"

Oh OK. I'll fix it.

Quoted 19 lines

"Gustavo A. R. Silva" <[email protected]> wrote on Tue, 10 Jul 2018 08:29:02 -0500:
Quoted 16 lines
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
Addresses-Coverity-ID: 1471717 ("Missing break in switch")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/mtd/nand/raw/nuc900_nand.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/mtd/nand/raw/nuc900_nand.c +0 -0
index af5b32c9..53a9f6c 100644
@@ -191,8 +191,9 @@ static void nuc900_nand_command_lp(struct mtd_info *mtd, unsigned int command,
 		return;
 

Quoted 7 lines

drivers/mtd/nand/raw/nuc900_nand.c static void nuc900_nand_command_lp(struct mtd_info *mtd, unsigned int command, · line 195
Quoted 4 lines
drivers/mtd/nand/raw/nuc900_nand.c +1 -1
 	case NAND_CMD_READ0:
-
 		write_cmd_reg(nand, NAND_CMD_READSTART);
+		/* fall through */
Have you checked this is actually the right thing to do?

Actually, no. My first impression was that due to the time this code has been there, this might be a missing-break false positive. But, now that I'm double checking, it may well be that this is an actual missing-break bug.

I can send a patch to fix this, but as I'm not familiar with the code, it'd be of great help if someone could help me to verify this.

Thanks for the feedback.
--
Gustavo

Gustavo A. R. Silva

author

2018-07-19 16:20 UTC #3 lore raw

In reply to Miquel Raynal

Hi Miquel,

Quoted 4 lines · Miquel Raynal

Hi Gustavo,
Prefix should be "mtd: rawnand: nuc900:"

Oh OK. I'll fix it. Thanks.

Quoted 19 lines

"Gustavo A. R. Silva" <[email protected]> wrote on Tue, 10 Jul 2018 08:29:02 -0500:
Quoted 16 lines
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
Addresses-Coverity-ID: 1471717 ("Missing break in switch")
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/mtd/nand/raw/nuc900_nand.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/mtd/nand/raw/nuc900_nand.c +0 -0
index af5b32c9..53a9f6c 100644
@@ -191,8 +191,9 @@ static void nuc900_nand_command_lp(struct mtd_info *mtd, unsigned int command,
 		return;
 

Quoted 7 lines

drivers/mtd/nand/raw/nuc900_nand.c static void nuc900_nand_command_lp(struct mtd_info *mtd, unsigned int command, · line 195
Quoted 4 lines
drivers/mtd/nand/raw/nuc900_nand.c +1 -1
 	case NAND_CMD_READ0:
-
 		write_cmd_reg(nand, NAND_CMD_READSTART);
+		/* fall through */
Have you checked this is actually the right thing to do?

Actually, no. My first impression was that due to the long time this code has been there, this might be a missing-break-in-switch false positive. But, due to your comments and a double check at the code, it may well be that this is an actual bug and that a return statement should be added after calling write_cmd_reg. Similar to the cases above.

I can send a patch to fix this, but it would be great if someone could help me to verify this first. :)

Thanks for the feedback!
--
Gustavo