Skip to contentSkip to conversation

net: smc_close: mark expected switch fall-throughs

PATCH In review Gustavo A. R. Silva 2 participants 6 messages Oct 19, 2017 – Oct 24, 2017

Subsystems (4)
Networking Network drivers s390 Shared memory communications (SMC) sockets
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 net: smc_close: mark expected switch fall-throughs

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

Notice that in this particular case I placed a "fall through" comment on its own line, which is what GCC is expecting to find.

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/smc/smc_close.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/smc/smc_close.c +2 -1
index f0d16fb..9b16f40 100644
@@ -360,7 +360,8 @@ static void smc_close_passive_work(struct work_struct *work)
 	case SMC_PEERCLOSEWAIT1:
 		if (rxflags->peer_done_writing)
 			sk->sk_state = SMC_PEERCLOSEWAIT2;
-		/* fall through to check for closing */
+		/* to check for closing */
+		/* fall through */
 	case SMC_PEERCLOSEWAIT2:
 	case SMC_PEERFINCLOSEWAIT:
 		if (!smc_cdc_rxed_any_close(&smc->conn))
Signature · 1 line
2.7.4
maintainer
#1 lore raw

From: "Gustavo A. R. Silva" <[email protected]>
Date: Thu, 19 Oct 2017 17:02:44 -0500

net/smc/smc_close.c static void smc_close_passive_work(struct work_struct *work) · line 364

Quoted 7 lines
net/smc/smc_close.c +2 -1
@@ -360,7 +360,8 @@ static void smc_close_passive_work(struct work_struct *work)
 	case SMC_PEERCLOSEWAIT1:
 		if (rxflags->peer_done_writing)
 			sk->sk_state = SMC_PEERCLOSEWAIT2;
-		/* fall through to check for closing */
+		/* to check for closing */
+		/* fall through */

Gustavo please look at what you are doing to the code.

This was a nice easy to read sentence in the comment, and now you've chopped it up into two pieces and made it awkward and more difficult to read.

author
#2 lore raw

Quoting David Miller <[email protected]>:

Quoted 14 lines

From: "Gustavo A. R. Silva" <[email protected]>
Date: Thu, 19 Oct 2017 17:02:44 -0500

net/smc/smc_close.c static void smc_close_passive_work(struct · line 365

Quoted 8 lines
net/smc/smc_close.c +2 -1
@@ -360,7 +360,8 @@ static void smc_close_passive_work(struct  
 work_struct *work)
 	case SMC_PEERCLOSEWAIT1:
 		if (rxflags->peer_done_writing)
 			sk->sk_state = SMC_PEERCLOSEWAIT2;
-		/* fall through to check for closing */
+		/* to check for closing */
+		/* fall through */

Gustavo please look at what you are doing to the code.

Quoted 3 lines

This was a nice easy to read sentence in the comment, and now you've chopped it up into two pieces and made it awkward and more difficult to read.

You're right.

What about this instead:

/* fall through */
/* to check for closing */

--
Gustavo A. R. Silva

author
#3 lore raw

PATCH v2 1/1 net: smc_close: mark expected switch fall-through

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

Notice that in this particular case I placed the "fall through" comment on its own line, which is what GCC is expecting to find.

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

---
Changes in v2:
Move the "fall through" comment on its own line
above the rest of the sentence.

 net/smc/smc_close.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/smc/smc_close.c +2 -1
index f0d16fb..a6c6559 100644
@@ -360,7 +360,8 @@ static void smc_close_passive_work(struct work_struct *work)
 	case SMC_PEERCLOSEWAIT1:
 		if (rxflags->peer_done_writing)
 			sk->sk_state = SMC_PEERCLOSEWAIT2;
-		/* fall through to check for closing */
+		/* fall through */
+		/* to check for closing */
 	case SMC_PEERCLOSEWAIT2:
 	case SMC_PEERFINCLOSEWAIT:
 		if (!smc_cdc_rxed_any_close(&smc->conn))
Signature · 1 line
2.7.4
maintainer Applied
#4 lore raw

Re: [PATCH v2] net: smc_close: mark expected switch fall-through

From: "Gustavo A. R. Silva" <[email protected]>
Date: Sat, 21 Oct 2017 20:35:30 -0500

Quoted 11 lines

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

Notice that in this particular case I placed the "fall through" comment on its own line, which is what GCC is expecting to find.

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

---
Changes in v2:
Move the "fall through" comment on its own line
above the rest of the sentence.

Applied.

maintainer
#5 lore raw
In reply to Gustavo A. R. Silva

From: "Gustavo A. R. Silva" <[email protected]>
Date: Sat, 21 Oct 2017 20:21:00 -0500

Quoted 21 lines

Quoting David Miller <[email protected]>:

Quoted 14 lines

From: "Gustavo A. R. Silva" <[email protected]>
Date: Thu, 19 Oct 2017 17:02:44 -0500

net/smc/smc_close.c static void smc_close_passive_work(struct · line 365

Quoted 8 lines
net/smc/smc_close.c +2 -1
@@ -360,7 +360,8 @@ static void smc_close_passive_work(struct
 work_struct *work)
 	case SMC_PEERCLOSEWAIT1:
 		if (rxflags->peer_done_writing)
 			sk->sk_state = SMC_PEERCLOSEWAIT2;
-		/* fall through to check for closing */
+		/* to check for closing */
+		/* fall through */

Gustavo please look at what you are doing to the code.

Quoted 3 lines

This was a nice easy to read sentence in the comment, and now you've chopped it up into two pieces and made it awkward and more difficult to read.

Quoted 6 lines

You're right.

What about this instead:

/* fall through */
/* to check for closing */

I'm surprised gcc cares if it's all on one line or not, actually.