| Message ID | 20190320182715.GA9406@embeddedor (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Commit | 296bd5aea223d1d04f4e113ceec74d1fb3e422c5 |
| Headers | show |
| Series | PCI: Mark expected switch fall-throughs | expand |
On Wed, Mar 20, 2019 at 01:27:15PM -0500, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch > cases where we are expecting to fall through. Does this fix all the remaining cases in drivers/pci? I'd like to fix them all at once. What's the best way to watch for new warnings being added? I fiddled with "make W=2" etc but it didn't seem useful. Does the 0-day robot warn when -Wimplicit-fallthrough warnings are added? Bjorn > This patch fixes the following warnings: > > drivers/pci/proc.c: In function ‘proc_bus_pci_ioctl’: > drivers/pci/proc.c:216:6: warning: this statement may fall through [-Wimplicit-fallthrough=] > if (arch_can_pci_mmap_wc()) { > ^ > drivers/pci/proc.c:225:2: note: here > default: > ^~~~~~~ > > drivers/pci/xen-pcifront.c: In function ‘pcifront_backend_changed’: > drivers/pci/xen-pcifront.c:1105:6: warning: this statement may fall through [-Wimplicit-fallthrough=] > if (xdev->state == XenbusStateClosed) > ^ > drivers/pci/xen-pcifront.c:1108:2: note: here > case XenbusStateClosing: > ^~~~ > > Notice that, in this particular case, the /* fall through */ > comment is placed at the very bottom of the case statement, > which is what GCC is expecting to find. > > Warning level 3 was used: -Wimplicit-fallthrough=3 > > This patch is part of the ongoing efforts to enable > -Wimplicit-fallthrough. > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > --- > drivers/pci/proc.c | 1 + > drivers/pci/xen-pcifront.c | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c > index 6fa1627ce08d..445b51db75b0 100644 > --- a/drivers/pci/proc.c > +++ b/drivers/pci/proc.c > @@ -222,6 +222,7 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd, > } > /* If arch decided it can't, fall through... */ > #endif /* HAVE_PCI_MMAP */ > + /* fall through */ > default: > ret = -EINVAL; > break; > diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c > index eba6e33147a2..14cf0f41ecf0 100644 > --- a/drivers/pci/xen-pcifront.c > +++ b/drivers/pci/xen-pcifront.c > @@ -1104,7 +1104,7 @@ static void __ref pcifront_backend_changed(struct xenbus_device *xdev, > case XenbusStateClosed: > if (xdev->state == XenbusStateClosed) > break; > - /* Missed the backend's CLOSING state -- fallthrough */ > + /* fall through - Missed the backend's CLOSING state. */ > case XenbusStateClosing: > dev_warn(&xdev->dev, "backend going away!\n"); > pcifront_try_disconnect(pdev); > -- > 2.21.0 >
On 20/03/2019 18:27, Gustavo A. R. Silva wrote: > diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c > index 6fa1627ce08d..445b51db75b0 100644 > --- a/drivers/pci/proc.c > +++ b/drivers/pci/proc.c > @@ -222,6 +222,7 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd, > } > /* If arch decided it can't, fall through... */ > #endif /* HAVE_PCI_MMAP */ > + /* fall through */ Surely it would be better to transpose the #endif and its previous line, than to add a second fallthrough ? ~Andrew
On 3/20/19 2:24 PM, Bjorn Helgaas wrote: > On Wed, Mar 20, 2019 at 01:27:15PM -0500, Gustavo A. R. Silva wrote: >> In preparation to enabling -Wimplicit-fallthrough, mark switch >> cases where we are expecting to fall through. > > Does this fix all the remaining cases in drivers/pci? I'd like to fix > them all at once. > These are actually the last ones. > What's the best way to watch for new warnings being added? I fiddled > with "make W=2" etc but it didn't seem useful. Does the 0-day robot > warn when -Wimplicit-fallthrough warnings are added? > 0-day robot doesn't know about these warnings yet. But it certainly will once we finally enable -Wimplicit-fallthrough. Add this to your Makefile to see the warnings: KBUILD_CFLAGS += $(call cc-option,-Wimplicit-fallthrough=3,) Thanks -- Gustavo > Bjorn > >> This patch fixes the following warnings: >> >> drivers/pci/proc.c: In function ‘proc_bus_pci_ioctl’: >> drivers/pci/proc.c:216:6: warning: this statement may fall through [-Wimplicit-fallthrough=] >> if (arch_can_pci_mmap_wc()) { >> ^ >> drivers/pci/proc.c:225:2: note: here >> default: >> ^~~~~~~ >> >> drivers/pci/xen-pcifront.c: In function ‘pcifront_backend_changed’: >> drivers/pci/xen-pcifront.c:1105:6: warning: this statement may fall through [-Wimplicit-fallthrough=] >> if (xdev->state == XenbusStateClosed) >> ^ >> drivers/pci/xen-pcifront.c:1108:2: note: here >> case XenbusStateClosing: >> ^~~~ >> >> Notice that, in this particular case, the /* fall through */ >> comment is placed at the very bottom of the case statement, >> which is what GCC is expecting to find. >> >> Warning level 3 was used: -Wimplicit-fallthrough=3 >> >> This patch is part of the ongoing efforts to enable >> -Wimplicit-fallthrough. >> >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> >> --- >> drivers/pci/proc.c | 1 + >> drivers/pci/xen-pcifront.c | 2 +- >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c >> index 6fa1627ce08d..445b51db75b0 100644 >> --- a/drivers/pci/proc.c >> +++ b/drivers/pci/proc.c >> @@ -222,6 +222,7 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd, >> } >> /* If arch decided it can't, fall through... */ >> #endif /* HAVE_PCI_MMAP */ >> + /* fall through */ >> default: >> ret = -EINVAL; >> break; >> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c >> index eba6e33147a2..14cf0f41ecf0 100644 >> --- a/drivers/pci/xen-pcifront.c >> +++ b/drivers/pci/xen-pcifront.c >> @@ -1104,7 +1104,7 @@ static void __ref pcifront_backend_changed(struct xenbus_device *xdev, >> case XenbusStateClosed: >> if (xdev->state == XenbusStateClosed) >> break; >> - /* Missed the backend's CLOSING state -- fallthrough */ >> + /* fall through - Missed the backend's CLOSING state. */ >> case XenbusStateClosing: >> dev_warn(&xdev->dev, "backend going away!\n"); >> pcifront_try_disconnect(pdev); >> -- >> 2.21.0 >>
On 3/20/19 2:27 PM, Andrew Cooper wrote: > On 20/03/2019 18:27, Gustavo A. R. Silva wrote: >> diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c >> index 6fa1627ce08d..445b51db75b0 100644 >> --- a/drivers/pci/proc.c >> +++ b/drivers/pci/proc.c >> @@ -222,6 +222,7 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd, >> } >> /* If arch decided it can't, fall through... */ >> #endif /* HAVE_PCI_MMAP */ >> + /* fall through */ > > Surely it would be better to transpose the #endif and its previous line, > than to add a second fallthrough ? > I agree. The thing is that, currently, GCC is expecting to find the fall-through "annotations" at the very bottom of the case statement, as I mentioned it in the changelog text. That's the reason why I decided to left in place the original comment. Thanks -- Gustavo
On Wed, Mar 20, 2019 at 01:27:15PM -0500, Gustavo A. R. Silva wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch > cases where we are expecting to fall through. > > This patch fixes the following warnings: > > drivers/pci/proc.c: In function ‘proc_bus_pci_ioctl’: > drivers/pci/proc.c:216:6: warning: this statement may fall through [-Wimplicit-fallthrough=] > if (arch_can_pci_mmap_wc()) { > ^ > drivers/pci/proc.c:225:2: note: here > default: > ^~~~~~~ > > drivers/pci/xen-pcifront.c: In function ‘pcifront_backend_changed’: > drivers/pci/xen-pcifront.c:1105:6: warning: this statement may fall through [-Wimplicit-fallthrough=] > if (xdev->state == XenbusStateClosed) > ^ > drivers/pci/xen-pcifront.c:1108:2: note: here > case XenbusStateClosing: > ^~~~ > > Notice that, in this particular case, the /* fall through */ > comment is placed at the very bottom of the case statement, > which is what GCC is expecting to find. > > Warning level 3 was used: -Wimplicit-fallthrough=3 > > This patch is part of the ongoing efforts to enable > -Wimplicit-fallthrough. > > Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> Applied to pci/misc for v5.2, thanks! > --- > drivers/pci/proc.c | 1 + > drivers/pci/xen-pcifront.c | 2 +- > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c > index 6fa1627ce08d..445b51db75b0 100644 > --- a/drivers/pci/proc.c > +++ b/drivers/pci/proc.c > @@ -222,6 +222,7 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd, > } > /* If arch decided it can't, fall through... */ > #endif /* HAVE_PCI_MMAP */ > + /* fall through */ > default: > ret = -EINVAL; > break; > diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c > index eba6e33147a2..14cf0f41ecf0 100644 > --- a/drivers/pci/xen-pcifront.c > +++ b/drivers/pci/xen-pcifront.c > @@ -1104,7 +1104,7 @@ static void __ref pcifront_backend_changed(struct xenbus_device *xdev, > case XenbusStateClosed: > if (xdev->state == XenbusStateClosed) > break; > - /* Missed the backend's CLOSING state -- fallthrough */ > + /* fall through - Missed the backend's CLOSING state. */ > case XenbusStateClosing: > dev_warn(&xdev->dev, "backend going away!\n"); > pcifront_try_disconnect(pdev); > -- > 2.21.0 >
On 3/20/19 3:13 PM, Bjorn Helgaas wrote: > On Wed, Mar 20, 2019 at 01:27:15PM -0500, Gustavo A. R. Silva wrote: [..] >> >> Warning level 3 was used: -Wimplicit-fallthrough=3 >> >> This patch is part of the ongoing efforts to enable >> -Wimplicit-fallthrough. >> >> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> > > Applied to pci/misc for v5.2, thanks! > Awesome! Thanks -- Gustavo
>>> On 20.03.19 at 19:27, <gustavo@embeddedor.com> wrote: > In preparation to enabling -Wimplicit-fallthrough, mark switch > cases where we are expecting to fall through. > > This patch fixes the following warnings: > > drivers/pci/proc.c: In function ‘proc_bus_pci_ioctl’: > drivers/pci/proc.c:216:6: warning: this statement may fall through > [-Wimplicit-fallthrough=] > if (arch_can_pci_mmap_wc()) { > ^ > drivers/pci/proc.c:225:2: note: here > default: > ^~~~~~~ > > drivers/pci/xen-pcifront.c: In function ‘pcifront_backend_changed’: > drivers/pci/xen-pcifront.c:1105:6: warning: this statement may fall through > [-Wimplicit-fallthrough=] > if (xdev->state == XenbusStateClosed) > ^ > drivers/pci/xen-pcifront.c:1108:2: note: here > case XenbusStateClosing: > ^~~~ > > Notice that, in this particular case, the /* fall through */ > comment is placed at the very bottom of the case statement, > which is what GCC is expecting to find. > > Warning level 3 was used: -Wimplicit-fallthrough=3 > > This patch is part of the ongoing efforts to enable > -Wimplicit-fallthrough. None of the above explains why ... > --- a/drivers/pci/xen-pcifront.c > +++ b/drivers/pci/xen-pcifront.c > @@ -1104,7 +1104,7 @@ static void __ref pcifront_backend_changed(struct xenbus_device *xdev, > case XenbusStateClosed: > if (xdev->state == XenbusStateClosed) > break; > - /* Missed the backend's CLOSING state -- fallthrough */ > + /* fall through - Missed the backend's CLOSING state. */ ... the original comment here wasn't good enough. Jan
diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c index 6fa1627ce08d..445b51db75b0 100644 --- a/drivers/pci/proc.c +++ b/drivers/pci/proc.c @@ -222,6 +222,7 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd, } /* If arch decided it can't, fall through... */ #endif /* HAVE_PCI_MMAP */ + /* fall through */ default: ret = -EINVAL; break; diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c index eba6e33147a2..14cf0f41ecf0 100644 --- a/drivers/pci/xen-pcifront.c +++ b/drivers/pci/xen-pcifront.c @@ -1104,7 +1104,7 @@ static void __ref pcifront_backend_changed(struct xenbus_device *xdev, case XenbusStateClosed: if (xdev->state == XenbusStateClosed) break; - /* Missed the backend's CLOSING state -- fallthrough */ + /* fall through - Missed the backend's CLOSING state. */ case XenbusStateClosing: dev_warn(&xdev->dev, "backend going away!\n"); pcifront_try_disconnect(pdev);
In preparation to enabling -Wimplicit-fallthrough, mark switch cases where we are expecting to fall through. This patch fixes the following warnings: drivers/pci/proc.c: In function ‘proc_bus_pci_ioctl’: drivers/pci/proc.c:216:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if (arch_can_pci_mmap_wc()) { ^ drivers/pci/proc.c:225:2: note: here default: ^~~~~~~ drivers/pci/xen-pcifront.c: In function ‘pcifront_backend_changed’: drivers/pci/xen-pcifront.c:1105:6: warning: this statement may fall through [-Wimplicit-fallthrough=] if (xdev->state == XenbusStateClosed) ^ drivers/pci/xen-pcifront.c:1108:2: note: here case XenbusStateClosing: ^~~~ Notice that, in this particular case, the /* fall through */ comment is placed at the very bottom of the case statement, which is what GCC is expecting to find. Warning level 3 was used: -Wimplicit-fallthrough=3 This patch is part of the ongoing efforts to enable -Wimplicit-fallthrough. Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- drivers/pci/proc.c | 1 + drivers/pci/xen-pcifront.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)