| Message ID | aab7cf16bf43cc7c3e9c9930d2dae850c1d07a3c.1605896059.git.gustavoars@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Fix fall-through warnings for Clang | expand |
On 11/20/20 7:34 PM, Gustavo A. R. Silva wrote: > In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning > by explicitly adding a break statement instead of letting the code fall > through to the next case. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> > --- > drivers/net/can/usb/peak_usb/pcan_usb_core.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c > index c2764799f9ef..fd65a155be3b 100644 > --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c > +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c > @@ -299,6 +299,8 @@ static void peak_usb_write_bulk_callback(struct urb *urb) > if (net_ratelimit()) > netdev_err(netdev, "Tx urb aborted (%d)\n", > urb->status); > + break; > + > case -EPROTO: > case -ENOENT: > case -ECONNRESET: > What about moving the default to the end if the case, which is more common anyways: diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c index 204ccb27d6d9..e8977dd10902 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c @@ -295,16 +295,16 @@ static void peak_usb_write_bulk_callback(struct urb *urb) netif_trans_update(netdev); break; - default: - if (net_ratelimit()) - netdev_err(netdev, "Tx urb aborted (%d)\n", - urb->status); case -EPROTO: case -ENOENT: case -ECONNRESET: case -ESHUTDOWN: - break; + + default: + if (net_ratelimit()) + netdev_err(netdev, "Tx urb aborted (%d)\n", + urb->status); } /* should always release echo skb and corresponding context */ Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> Marc
On Sat, 2020-11-21 at 14:17 +0100, Marc Kleine-Budde wrote: > On 11/20/20 7:34 PM, Gustavo A. R. Silva wrote: > > In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning > > by explicitly adding a break statement instead of letting the code fall > > through to the next case. > > > > Link: https://github.com/KSPP/linux/issues/115 > > Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> [] > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c [] > > @@ -299,6 +299,8 @@ static void peak_usb_write_bulk_callback(struct urb *urb) > > if (net_ratelimit()) > > netdev_err(netdev, "Tx urb aborted (%d)\n", > > urb->status); > > + break; > > + > > case -EPROTO: > > case -ENOENT: > > case -ECONNRESET: > > > > What about moving the default to the end if the case, which is more common anyways: > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c [] > @@ -295,16 +295,16 @@ static void peak_usb_write_bulk_callback(struct urb *urb) > netif_trans_update(netdev); > break; > > > - default: > - if (net_ratelimit()) > - netdev_err(netdev, "Tx urb aborted (%d)\n", > - urb->status); > case -EPROTO: > case -ENOENT: > case -ECONNRESET: > case -ESHUTDOWN: > - > break; > + > + default: > + if (net_ratelimit()) > + netdev_err(netdev, "Tx urb aborted (%d)\n", > + urb->status); That's fine and is more generally used style but this default: case should IMO also end with a break; + break; > }
On 11/21/20 8:50 PM, Joe Perches wrote: >> What about moving the default to the end if the case, which is more common anyways: >> >> diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c > [] >> @@ -295,16 +295,16 @@ static void peak_usb_write_bulk_callback(struct urb *urb) >> netif_trans_update(netdev); >> break; >> >> >> - default: >> - if (net_ratelimit()) >> - netdev_err(netdev, "Tx urb aborted (%d)\n", >> - urb->status); >> case -EPROTO: >> case -ENOENT: >> case -ECONNRESET: >> case -ESHUTDOWN: >> - >> break; >> + >> + default: >> + if (net_ratelimit()) >> + netdev_err(netdev, "Tx urb aborted (%d)\n", >> + urb->status); > > That's fine and is more generally used style but this > default: case should IMO also end with a break; > > + break; I don't mind. process/coding-style.rst is not totally clear about the break after the default, if this is the lase one the switch statement. Marc
On Sun, 2020-11-22 at 00:04 +0100, Marc Kleine-Budde wrote: > On 11/21/20 8:50 PM, Joe Perches wrote: > > > What about moving the default to the end if the case, which is more common anyways: > > > > > > diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c > > [] > > > @@ -295,16 +295,16 @@ static void peak_usb_write_bulk_callback(struct urb *urb) > > > netif_trans_update(netdev); > > > break; > > > > > > > > > - default: > > > - if (net_ratelimit()) > > > - netdev_err(netdev, "Tx urb aborted (%d)\n", > > > - urb->status); > > > case -EPROTO: > > > case -ENOENT: > > > case -ECONNRESET: > > > case -ESHUTDOWN: > > > - > > > break; > > > + > > > + default: > > > + if (net_ratelimit()) > > > + netdev_err(netdev, "Tx urb aborted (%d)\n", > > > + urb->status); > > > > That's fine and is more generally used style but this > > default: case should IMO also end with a break; > > > > + break; > > I don't mind. > > process/coding-style.rst is not totally clear about the break after the default, > if this is the lase one the switch statement. deprecated.rst has: All switch/case blocks must end in one of: * break; * fallthrough; * continue; * goto <label>; * return [expression]; I suppose that could be moved into coding-style along with maybe a change to "all switch/case/default blocks"
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.c b/drivers/net/can/usb/peak_usb/pcan_usb_core.c index c2764799f9ef..fd65a155be3b 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.c +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.c @@ -299,6 +299,8 @@ static void peak_usb_write_bulk_callback(struct urb *urb) if (net_ratelimit()) netdev_err(netdev, "Tx urb aborted (%d)\n", urb->status); + break; + case -EPROTO: case -ENOENT: case -ECONNRESET:
In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning by explicitly adding a break statement instead of letting the code fall through to the next case. Link: https://github.com/KSPP/linux/issues/115 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> --- drivers/net/can/usb/peak_usb/pcan_usb_core.c | 2 ++ 1 file changed, 2 insertions(+)