usbip: Use fallthrough pseudo-keyword

PATCH In review Gustavo A. R. Silva 4 participants 9 messages Jul 7, 2020 – Jul 8, 2020

Subsystems (2)

USB USB over IP driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2020-07-07 19:46 UTC #0 lore raw

PATCH 1/1 usbip: Use fallthrough pseudo-keyword

Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.

[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through

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

---
 drivers/usb/usbip/stub_rx.c       |    2 +-
 drivers/usb/usbip/vhci_hcd.c      |    7 +++----
 drivers/usb/usbip/vhci_rx.c       |    2 +-
 drivers/usb/usbip/vudc_transfer.c |    4 ++--
 4 files changed, 7 insertions(+), 8 deletions(-)

drivers/usb/usbip/stub_rx.c +1 -1

index e2b019532234..325c22008e53 100644
@@ -424,7 +424,7 @@ static void masking_bogus_flags(struct urb *urb)
 	case USB_ENDPOINT_XFER_BULK:
 		if (is_out)
 			allowed |= URB_ZERO_PACKET;
-		/* FALLTHROUGH */
+		fallthrough;
 	default:			/* all non-iso endpoints */
 		if (!is_out)
 			allowed |= URB_SHORT_NOT_OK;

drivers/usb/usbip/vhci_hcd.c +3 -4

index 65850e9c7190..1b598db5d8b9 100644
@@ -508,7 +508,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 		case USB_PORT_FEAT_U1_TIMEOUT:
 			usbip_dbg_vhci_rh(
 				" SetPortFeature: USB_PORT_FEAT_U1_TIMEOUT\n");
-			/* Fall through */
+			fallthrough;
 		case USB_PORT_FEAT_U2_TIMEOUT:
 			usbip_dbg_vhci_rh(
 				" SetPortFeature: USB_PORT_FEAT_U2_TIMEOUT\n");

@@ -561,7 +561,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 				       "supported for USB 2.0 roothub\n");
 				goto error;
 			}
-			/* FALLS THROUGH */
+			fallthrough;
 		case USB_PORT_FEAT_RESET:
 			usbip_dbg_vhci_rh(
 				" SetPortFeature: USB_PORT_FEAT_RESET\n");

@@ -584,8 +584,7 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 
 			/* 50msec reset signaling */
 			vhci_hcd->re_timeout = jiffies + msecs_to_jiffies(50);
-
-			/* FALLS THROUGH */
+			fallthrough;
 		default:
 			usbip_dbg_vhci_rh(" SetPortFeature: default %d\n",
 					  wValue);

drivers/usb/usbip/vhci_rx.c +1 -1

index 00fc98741c5d..266024cbb64f 100644
@@ -27,7 +27,7 @@ struct urb *pickup_urb_and_free_priv(struct vhci_device *vdev, __u32 seqnum)
 
 		switch (status) {
 		case -ENOENT:
-			/* fall through */
+			fallthrough;
 		case -ECONNRESET:
 			dev_dbg(&urb->dev->dev,
 				 "urb seq# %u was unlinked %ssynchronously\n",

drivers/usb/usbip/vudc_transfer.c +2 -2

index c9db846ee4f6..7e801fee33bf 100644
@@ -404,7 +404,7 @@ static void v_timer(struct timer_list *t)
 			 * for now, give unlimited bandwidth
 			 */
 			limit += urb->transfer_buffer_length;
-			/* fallthrough */
+			fallthrough;
 		default:
 treat_control_like_bulk:
 			total -= transfer(udc, urb, ep, limit);

@@ -479,7 +479,7 @@ void v_kick_timer(struct vudc *udc, unsigned long time)
 		return;
 	case VUDC_TR_IDLE:
 		t->state = VUDC_TR_RUNNING;
-		/* fallthrough */
+		fallthrough;
 	case VUDC_TR_STOPPED:
 		/* we may want to kick timer to unqueue urbs */
 		mod_timer(&t->timer, time);

Shuah Khan

maintainer Acked-by on v1 1/1

2020-07-07 20:06 UTC #1 lore raw

Quoted 6 lines · Gustavo A. R. Silva

Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through

Is fallthrough syntax supported on our min gcc version?

Does checkpatch or coccicheck catch these cases?

The patch looks good.

Acked-by Shuah Khan <[email protected]>

thanks,
-- Shuah

Joe Perches

2020-07-08 01:56 UTC #2 lore raw

Quoted 9 lines · Shuah Khan

Quoted 6 lines · Gustavo A. R. Silva
Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
Is fallthrough syntax supported on our min gcc version?

No. Introduced in gcc 7.

Quoted 1 line

Does checkpatch or coccicheck catch these cases?

Kinda. checkpatch isn't very good at it.
I _believe_, though I'm not at all sure,
that coccinelle can find these.

Julia Lawall

2020-07-08 10:16 UTC #3 lore raw

Quoted 11 lines · Joe Perches

Quoted 9 lines · Shuah Khan
Quoted 6 lines · Gustavo A. R. Silva
Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
Is fallthrough syntax supported on our min gcc version?

Quoted 7 lines

No. Introduced in gcc 7.
Quoted 1 line
Does checkpatch or coccicheck catch these cases?
Kinda. checkpatch isn't very good at it.
I _believe_, though I'm not at all sure,
that coccinelle can find these.

I would not guarantee anything about the support of Coccinelle for switch. Coccinelle does now have the ability to match on comments. So since there is a distinct comment that it is to be removed, it might be possible to do that part automatically.

Maybe it would have to look something like this:

@r1@
comments c : script:python() { code to recognize the comment }; statement S;
@@

S@c
+ fallthrough(); //or whatever is wanted

@@
statement r1.S;
@@

- S
- fallthrough();
+ S
+ fallthrough();

The second rule probably looks pretty strange, but the goal is to remove the comments between S and fallthrough();

There is an example demos/comments.cocci that shows how to access the comment information using both ocaml and python.

julia

Shuah Khan

maintainer

2020-07-08 14:42 UTC #4 lore raw

Quoted 47 lines · Julia Lawall

Quoted 11 lines · Joe Perches
Quoted 9 lines · Shuah Khan
Quoted 6 lines · Gustavo A. R. Silva
Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
Is fallthrough syntax supported on our min gcc version?
Quoted 7 lines
No. Introduced in gcc 7.
Quoted 1 line
Does checkpatch or coccicheck catch these cases?
Kinda. checkpatch isn't very good at it.
I _believe_, though I'm not at all sure,
that coccinelle can find these.
I would not guarantee anything about the support of Coccinelle for switch. Coccinelle does now have the ability to match on comments. So since there is a distinct comment that it is to be removed, it might be possible to do that part automatically.
Maybe it would have to look something like this:
@r1@
comments c : script:python() { code to recognize the comment }; statement S;
@@
S@c
+ fallthrough(); //or whatever is wanted
@@
statement r1.S;
@@
- S
- fallthrough();
+ S
+ fallthrough();

Quoted 6 lines

The second rule probably looks pretty strange, but the goal is to remove the comments between S and fallthrough();
There is an example demos/comments.cocci that shows how to access the comment information using both ocaml and python.

Thanks Julia. Maybe this is a way to address all of the cases. I am a bit concerned about min gcc which is 4.8 and the fallthrough syntax support is in gcc 7

-- Shuah

Shuah Khan

maintainer

2020-07-08 14:35 UTC #5 lore raw

In reply to Joe Perches

Quoted 6 lines · Joe Perches

Quoted 5 lines · Shuah Khan
Quoted 4 lines · Gustavo A. R. Silva
Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
Is fallthrough syntax supported on our min gcc version?
No. Introduced in gcc 7.

Gustavo,

In which case, this patch would break usbip build on older gcc revisions.

thanks,
-- Shuah

Joe Perches

2020-07-08 15:40 UTC #6 lore raw

Quoted 12 lines · Shuah Khan

Quoted 11 lines · Joe Perches
Quoted 9 lines · Shuah Khan
Quoted 6 lines · Gustavo A. R. Silva
Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
Is fallthrough syntax supported on our min gcc version?

Quoted 7 lines

Quoted 2 lines
No. Introduced in gcc 7.
Gustavo,
In which case, this patch would break usbip build on older gcc revisions.

No it will not. Comment fall through was introduced in gcc 7 and this devolved into a do while 0

Quoted 2 lines

thanks,
-- Shuah

Shuah Khan

maintainer

2020-07-08 16:39 UTC #7 lore raw

Quoted 17 lines · Joe Perches

Quoted 16 lines · Shuah Khan
Quoted 14 lines · Joe Perches
Quoted 10 lines · Shuah Khan
Quoted 7 lines · Gustavo A. R. Silva
Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
Is fallthrough syntax supported on our min gcc version?
No. Introduced in gcc 7.

Quoted 6 lines

Quoted 4 lines
Gustavo,
In which case, this patch would break usbip build on older gcc revisions.
No it will not. Comment fall through was introduced in gcc 7 and this devolved into a do while 0

Ah thanks. I didn't know that. No concerns of patch to go in.

thanks,
--- Shuah

Gustavo A. R. Silva

author

2020-07-08 18:06 UTC #8 lore raw

In reply to Shuah Khan

Quoted 10 lines · Shuah Khan

Quoted 6 lines · Gustavo A. R. Silva
Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case.
[1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through
Is fallthrough syntax supported on our min gcc version?

The __attribute__((__fallthrough__)) has been supported since GCC 7.1 and it should be no problem for early versions because fallthrough is a macro that also expands to: do {} while (0) /* fallthrough */

Quoted 2 lines

Does checkpatch or coccicheck catch these cases?

checkpatch does:

https://git.kernel.org/linus/f36d3eb89a43047d3eba222a8132585da25cebfd

Quoted 4 lines

The patch looks good.
Acked-by Shuah Khan <[email protected]>

Thanks
--
Gustavo