On Thu, Feb 13, 2020 at 02:35:18PM +0100, Rasmus Villemoes wrote: > On 13/02/2020 13.56, Greg Kroah-Hartman wrote: > > >> diff --git a/drivers/usb/host/fhci-hcd.c b/drivers/usb/host/fhci-hcd.c > >> index 04733876c9c6..a8e1048278d0 100644 > >> --- a/drivers/usb/host/fhci-hcd.c > >> +++ b/drivers/usb/host/fhci-hcd.c > >> @@ -396,6 +396,7 @@ static int fhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, > >> case PIPE_CONTROL: > >> /* 1 td fro setup,1 for ack */ > >> size = 2; > >> + fallthrough; > > > > We have an attribute for that? > > > > Shouldn't this be /* fall through */ instead? > > > > Gustavo, what's the best practice here, I count only a few > > "fallthrough;" instances in the kernel, although one is in our coding > > style document, and thousands of the /* */ version. > > Yes, I went with the attribute/macro due to that, and the history is > that Linus applied Joe's patches directly > (https://lore.kernel.org/lkml/CAHk-=whOF8heTGz5tfzYUBp_UQQzSWNJ_50M7-ECXkfFRDQWFA@xxxxxxxxxxxxxx/), > so I assumed that meant the Penguin decided that the attribute/macro is > the right thing to do for new code, while existing comment annotations > can be left alone or changed piecemeal as code gets refactored anyway. But, to be fair, Gustavo went and fixed up thousands of these, with the /* */ version, not the attribute. Gustavo, can coverity notice the "fallthrough;" attribute properly? I don't want to start adding things that end up triggering false-positives. thanks, greg k-h