mark expected switch fall-throughs

PATCH In review Gustavo A. R. Silva 2 participants 15 messages Oct 25, 2017 – Nov 1, 2017

Subsystems (4)

USB USB xHCI driver USB ehci driver USB ohci driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2017-10-25 18:48 UTC #0 lore raw

cover mark expected switch fall-throughs

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

In Kees Cook words:
"This is an unfortunate omission in the C language, and thankfully both gcc and clang have stepped up to solve this the same way static analyzers have solved it. It does both document the intention for humans and provide a way for analyzers to report issues.

Having the compiler help us not make mistakes is quite handy."

In some cases there were "else FALL THROUGH" or "then fallthrough..." comments already in place. So I replaced them with proper "fall through" comments, which is what GCC is expecting to find.

Thanks!

Gustavo A. R. Silva (9):
usb: host: fotg210-hcd: mark expected switch fall-through
usb: host: xhci: mark expected switch fall-through
usb: host: xhci-mem: mark expected switch fall-through
usb: host: ohci-hcd: mark expected switch fall-through
usb: host: ehci-hcd: mark expected switch fall-through
usb: host: isp1362-hcd: mark expected switch fall-through
usb: host: oxu210hp-hcd: mark expected switch fall-through
usb: host: xhci-hub: mark expected switch fall-through
usb: host: pci-quirks: mark expected switch fall-through

 drivers/usb/host/ehci-hcd.c     | 2 +-
 drivers/usb/host/fotg210-hcd.c  | 2 +-
 drivers/usb/host/isp1362-hcd.c  | 1 +
 drivers/usb/host/ohci-hcd.c     | 2 +-
 drivers/usb/host/oxu210hp-hcd.c | 2 +-
 drivers/usb/host/pci-quirks.c   | 2 +-
 drivers/usb/host/xhci-hub.c     | 1 +
 drivers/usb/host/xhci-mem.c     | 1 +
 drivers/usb/host/xhci.c         | 1 +
 9 files changed, 9 insertions(+), 5 deletions(-)

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 18:53 UTC #1 lore raw

PATCH 1/9 usb: host: fotg210-hcd: mark expected switch fall-through

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

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

---
 drivers/usb/host/fotg210-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/host/fotg210-hcd.c +1 -1

index 457cc65..33a4f7e 100644
@@ -5449,7 +5449,7 @@ static void fotg210_endpoint_disable(struct usb_hcd *hcd,
 			qh_destroy(fotg210, qh);
 			break;
 		}
-		/* else FALL THROUGH */
+		/* fall through */
 	default:
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  just leak this memory.

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 18:53 UTC #2 lore raw

PATCH 2/9 usb: host: xhci: mark expected switch fall-through

In reply to Gustavo A. R. Silva

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

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

---
 drivers/usb/host/xhci.c | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/host/xhci.c +1 -0

index ee077a2..05db6e97 100644
@@ -4294,6 +4294,7 @@ static unsigned long long xhci_calculate_intel_u1_timeout(
 			break;
 		}
 		/* Otherwise the calculation is the same as isoc eps */
+		/* fall through */
 	case USB_ENDPOINT_XFER_ISOC:
 		timeout_ns = xhci_service_interval_to_ns(desc);
 		timeout_ns = DIV_ROUND_UP_ULL(timeout_ns * 105, 100);

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 18:53 UTC #3 lore raw

PATCH 3/9 usb: host: xhci-mem: mark expected switch fall-through

In reply to Gustavo A. R. Silva

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

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

---
 drivers/usb/host/xhci-mem.c | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/host/xhci-mem.c +1 -0

index 795219a..57be885 100644
@@ -1311,6 +1311,7 @@ static unsigned int xhci_get_endpoint_interval(struct usb_device *udev,
 		 * since it uses the same rules as low speed interrupt
 		 * endpoints.
 		 */
+		/* fall through */
 
 	case USB_SPEED_LOW:
 		if (usb_endpoint_xfer_int(&ep->desc) ||

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 18:49 UTC #4 lore raw

PATCH 4/9 usb: host: ohci-hcd: mark expected switch fall-through

In reply to Gustavo A. R. Silva

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

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

---
 drivers/usb/host/ohci-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/host/ohci-hcd.c +1 -1

index 4492482..15ec8f9 100644
@@ -382,7 +382,7 @@ ohci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
 			ed_free (ohci, ed);
 			break;
 		}
-		/* else FALL THROUGH */
+		/* fall through */
 	default:
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  can't recover; must leak ed.

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 18:49 UTC #5 lore raw

PATCH 5/9 usb: host: ehci-hcd: mark expected switch fall-through

In reply to Gustavo A. R. Silva

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

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

---
 drivers/usb/host/ehci-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/host/ehci-hcd.c +1 -1

index 6e834b83..c560a01 100644
@@ -1012,7 +1012,7 @@ ehci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
 			qh_destroy(ehci, qh);
 			break;
 		}
-		/* else FALL THROUGH */
+		/* fall through */
 	default:
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  just leak this memory.

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 18:49 UTC #6 lore raw

PATCH 6/9 usb: host: isp1362-hcd: mark expected switch fall-through

In reply to Gustavo A. R. Silva

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

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

---
 drivers/usb/host/isp1362-hcd.c | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/host/isp1362-hcd.c +1 -0

index 9b7e307..753d576 100644
@@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			spin_lock_irqsave(&isp1362_hcd->lock, flags);
 			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
 			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
+			/* fall through */
 		case C_HUB_LOCAL_POWER:
 			DBG(0, "C_HUB_LOCAL_POWER\n");
 			break;

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 19:05 UTC #7 lore raw

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

Greg,

Quoting "Gustavo A. R. Silva" <[email protected]>:

Quoted 13 lines

In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/host/isp1362-hcd.c | 1 +
 1 file changed, 1 insertion(+)
drivers/usb/host/isp1362-hcd.c +0 -0
index 9b7e307..753d576 100644

drivers/usb/host/isp1362-hcd.c static int isp1362_hub_control(struct usb_hcd · line 1582

Quoted 5 lines

drivers/usb/host/isp1362-hcd.c +1 -0

@@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd  
 *hcd, u16 typeReq, u16 wValue,
 			spin_lock_irqsave(&isp1362_hcd->lock, flags);
 			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
 			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
+			/* fall through */

I'm suspicious this should be a 'break' instead.

What do you think?

Quoted 5 lines

drivers/usb/host/isp1362-hcd.c +0 -0
 		case C_HUB_LOCAL_POWER:
 			DBG(0, "C_HUB_LOCAL_POWER\n");
break;
--
2.7.4

Thanks
--
Gustavo A. R. Silva

Greg Kroah-Hartman

maintainer

2017-11-01 16:01 UTC #8 lore raw

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

Quoted 25 lines · Gustavo A. R. Silva

Greg,
Quoting "Gustavo A. R. Silva" <[email protected]>:
Quoted 13 lines
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/host/isp1362-hcd.c | 1 +
 1 file changed, 1 insertion(+)
drivers/usb/host/isp1362-hcd.c +0 -0
index 9b7e307..753d576 100644
drivers/usb/host/isp1362-hcd.c static int isp1362_hub_control(struct usb_hcd · line 1582
Quoted 5 lines
drivers/usb/host/isp1362-hcd.c +1 -0
@@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd
 *hcd, u16 typeReq, u16 wValue,
 			spin_lock_irqsave(&isp1362_hcd->lock, flags);
 			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
 			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
+			/* fall through */
I'm suspicious this should be a 'break' instead.

Quoted 1 line

What do you think?

Yeah, this should be a 'break', care to make that patch up instead?

thanks,

greg k-h

Gustavo A. R. Silva

author

2017-11-01 17:27 UTC #9 lore raw

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

Quoting Greg Kroah-Hartman <[email protected]>:

Quoted 27 lines · Gustavo A. R. Silva

Quoted 26 lines
Greg,
Quoting "Gustavo A. R. Silva" <[email protected]>:
Review
Quoted 9 lines
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/host/isp1362-hcd.c | 1 +
 1 file changed, 1 insertion(+)
(quoted diff) +0 -0
b/drivers/usb/host/isp1362-hcd.c
drivers/usb/host/isp1362-hcd.c static int isp1362_hub_control(struct usb_hcd · line 1582
Quoted 9 lines
index 9b7e307..753d576 100644
drivers/usb/host/isp1362-hcd.c +1 -0
@@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd
 *hcd, u16 typeReq, u16 wValue,
 			spin_lock_irqsave(&isp1362_hcd->lock, flags);
 			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
 			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
+			/* fall through */
I'm suspicious this should be a 'break' instead.

Quoted 4 lines

Quoted 1 line
What do you think?
Yeah, this should be a 'break', care to make that patch up instead?

Sure thing.

Just some questions about the process to follow:

Should I send a v2 replying to this particular thread only? like [PATCH v2 6/9] or should I send just a new patch separated from this patch series? I guess this is the case.

Some maintainers have told me that in cases where a particular patch in the series needs an update, the complete patchset should be sent again. But I think that depends on the functional impact the patch has over the whole patchset.

Thanks
--
Gustavo A. R. Silva

Greg Kroah-Hartman

maintainer

2017-11-01 17:38 UTC #10 lore raw

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

Quoted 35 lines · Gustavo A. R. Silva

Quoting Greg Kroah-Hartman <[email protected]>:
Quoted 27 lines · Gustavo A. R. Silva
Quoted 26 lines
Greg,
Quoting "Gustavo A. R. Silva" <[email protected]>:
Review
Quoted 9 lines
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through.
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/host/isp1362-hcd.c | 1 +
 1 file changed, 1 insertion(+)
(quoted diff) +0 -0
b/drivers/usb/host/isp1362-hcd.c
drivers/usb/host/isp1362-hcd.c static int isp1362_hub_control(struct usb_hcd · line 1582
Quoted 9 lines
index 9b7e307..753d576 100644
drivers/usb/host/isp1362-hcd.c +1 -0
@@ -1578,6 +1578,7 @@ static int isp1362_hub_control(struct usb_hcd
 *hcd, u16 typeReq, u16 wValue,
 			spin_lock_irqsave(&isp1362_hcd->lock, flags);
 			isp1362_write_reg32(isp1362_hcd, HCRHSTATUS, RH_HS_OCIC);
 			spin_unlock_irqrestore(&isp1362_hcd->lock, flags);
+			/* fall through */
I'm suspicious this should be a 'break' instead.
Quoted 4 lines
Quoted 1 line
What do you think?
Yeah, this should be a 'break', care to make that patch up instead?

Quoted 8 lines

Sure thing.
Just some questions about the process to follow:
Should I send a v2 replying to this particular thread only? like [PATCH v2 6/9]
or should I send just a new patch separated from this patch series? I guess this is the case.

Brand new patch is fine, this is gone from my patch queue.

Quoted 4 lines

Some maintainers have told me that in cases where a particular patch in the series needs an update, the complete patchset should be sent again. But I think that depends on the functional impact the patch has over the whole patchset.

Yes, it all depends, the rest of these patches are already in my tree.

thanks,

greg k-h

Gustavo A. R. Silva

author

2017-11-01 17:41 UTC #11 lore raw

Re: [PATCH 6/9] usb: host: isp1362-hcd: mark expected switch fall-through

Quoting Greg Kroah-Hartman <[email protected]>:

[..]

Quoted 10 lines

Quoted 9 lines
Sure thing.
Just some questions about the process to follow:
Should I send a v2 replying to this particular thread only? like [PATCH v2 6/9]
or should I send just a new patch separated from this patch series? I guess this is the case.

Quoted 9 lines

Brand new patch is fine, this is gone from my patch queue.
Quoted 4 lines
Some maintainers have told me that in cases where a particular patch in the series needs an update, the complete patchset should be sent again. But I think that depends on the functional impact the patch has over the whole patchset.
Yes, it all depends, the rest of these patches are already in my tree.

OK. I'll send a new patch shortly.

Thanks!
--
Gustavo A. R. Silva

Gustavo A. R. Silva

author

2017-10-25 18:49 UTC #12 lore raw

PATCH 7/9 usb: host: oxu210hp-hcd: mark expected switch fall-through

In reply to Gustavo A. R. Silva

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

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

---
 drivers/usb/host/oxu210hp-hcd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/host/oxu210hp-hcd.c +1 -1

index ed20fb3..9f8c61e 100644
@@ -3040,7 +3040,7 @@ static void oxu_endpoint_disable(struct usb_hcd *hcd,
 			qh_put(qh);
 			break;
 		}
-		/* else FALL THROUGH */
+		/* fall through */
 	default:
 nogood:
 		/* caller was supposed to have unlinked any requests;

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 18:49 UTC #13 lore raw

PATCH 8/9 usb: host: xhci-hub: mark expected switch fall-through

In reply to Gustavo A. R. Silva

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

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

---
 drivers/usb/host/xhci-hub.c | 1 +
 1 file changed, 1 insertion(+)

drivers/usb/host/xhci-hub.c +1 -0

index 9762333..3693b1f 100644
@@ -1377,6 +1377,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue,
 			break;
 		case USB_PORT_FEAT_C_SUSPEND:
 			bus_state->port_c_suspend &= ~(1 << wIndex);
+			/* fall through */
 		case USB_PORT_FEAT_C_RESET:
 		case USB_PORT_FEAT_C_BH_PORT_RESET:
 		case USB_PORT_FEAT_C_CONNECTION:

Signature · 1 line

2.7.4

Gustavo A. R. Silva

author

2017-10-25 19:10 UTC #14 lore raw

PATCH 9/9 usb: host: pci-quirks: mark expected switch fall-through

In reply to Gustavo A. R. Silva

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

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

---
 drivers/usb/host/pci-quirks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/usb/host/pci-quirks.c +1 -1

index 6dda362..6731f8d8d 100644
@@ -841,7 +841,7 @@ static void quirk_usb_disable_ehci(struct pci_dev *pdev)
 			ehci_bios_handoff(pdev, op_reg_base, cap, offset);
 			break;
 		case 0: /* Illegal reserved cap, set cap=0 so we exit */
-			cap = 0; /* then fallthrough... */
+			cap = 0; /* fall through */
 		default:
 			dev_warn(&pdev->dev,
 				 "EHCI: unrecognized capability %02x\n",

Signature · 1 line

2.7.4