Patchwork [v1] usb: dwc3: trace: Refactor nested switch to make compiler happy

login
register
mail settings
Submitter Andy Shevchenko
Date Dec. 3, 2018, 9:28 a.m.
Message ID <20181203092847.76613-1-andriy.shevchenko@linux.intel.com>
Download mbox | patch
Permalink /patch/670277/
State New
Headers show

Comments

Andy Shevchenko - Dec. 3, 2018, 9:28 a.m.
The missed break statement in the outer switch makes the code fall through
always and thus always same value will be printed.

Besides that, compiler warns about missed fall through marker:

drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’:
drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
    switch (pcm) {
    ^~~~~~

Refactor nested switch statements to work correctly without
compilation warnings.

Fixes: fa8d965d736b ("usb: dwc3: trace: pretty print high-bandwidth transfers too")
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/usb/dwc3/trace.h | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
Felipe Balbi - Dec. 5, 2018, 9:10 a.m.
Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:

> The missed break statement in the outer switch makes the code fall through
> always and thus always same value will be printed.
>
> Besides that, compiler warns about missed fall through marker:
>
> drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’:
> drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     switch (pcm) {
>     ^~~~~~
>
> Refactor nested switch statements to work correctly without
> compilation warnings.
>
> Fixes: fa8d965d736b ("usb: dwc3: trace: pretty print high-bandwidth transfers too")
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/usb/dwc3/trace.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
> index f22714cce070..8e1625a6c19f 100644
> --- a/drivers/usb/dwc3/trace.h
> +++ b/drivers/usb/dwc3/trace.h
> @@ -238,7 +238,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
>  	),
>  	TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x (%c%c%c%c:%c%c:%s)",
>  		__get_str(name), __entry->trb, __entry->bph, __entry->bpl,
> -		({char *s;
> +		({ char *s = "";
>  		int pcm = ((__entry->size >> 24) & 3) + 1;
>  		switch (__entry->type) {
>  		case USB_ENDPOINT_XFER_INT:
> @@ -254,8 +254,6 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
>  				s = "3x ";
>  				break;
>  			}

easier to add "break" here, no? That would be the minimal fix.
Andy Shevchenko - Dec. 5, 2018, 9:18 a.m.
On Wed, Dec 05, 2018 at 11:10:46AM +0200, Felipe Balbi wrote:
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> 
> > The missed break statement in the outer switch makes the code fall through
> > always and thus always same value will be printed.
> >
> > Besides that, compiler warns about missed fall through marker:
> >
> > drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’:
> > drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
> >     switch (pcm) {
> >     ^~~~~~
> >
> > Refactor nested switch statements to work correctly without
> > compilation warnings.
> >
> > Fixes: fa8d965d736b ("usb: dwc3: trace: pretty print high-bandwidth transfers too")
> > Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/usb/dwc3/trace.h | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
> > index f22714cce070..8e1625a6c19f 100644
> > --- a/drivers/usb/dwc3/trace.h
> > +++ b/drivers/usb/dwc3/trace.h
> > @@ -238,7 +238,7 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
> >  	),
> >  	TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x (%c%c%c%c:%c%c:%s)",
> >  		__get_str(name), __entry->trb, __entry->bph, __entry->bpl,
> > -		({char *s;
> > +		({ char *s = "";
> >  		int pcm = ((__entry->size >> 24) & 3) + 1;
> >  		switch (__entry->type) {
> >  		case USB_ENDPOINT_XFER_INT:
> > @@ -254,8 +254,6 @@ DECLARE_EVENT_CLASS(dwc3_log_trb,
> >  				s = "3x ";
> >  				break;
> >  			}
> 
> easier to add "break" here, no? That would be the minimal fix.

No. Then you would need to add same default to the inner switch.
Andy Shevchenko - Dec. 5, 2018, 9:42 a.m.
On Wed, Dec 05, 2018 at 11:18:45AM +0200, Andy Shevchenko wrote:
> On Wed, Dec 05, 2018 at 11:10:46AM +0200, Felipe Balbi wrote:
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> writes:
> > 
> > > The missed break statement in the outer switch makes the code fall through
> > > always and thus always same value will be printed.
> > >
> > > Besides that, compiler warns about missed fall through marker:
> > >
> > > drivers/usb/dwc3/./trace.h: In function ‘trace_raw_output_dwc3_log_trb’:
> > > drivers/usb/dwc3/./trace.h:246:4: warning: this statement may fall through [-Wimplicit-fallthrough=]
> > >     switch (pcm) {
> > >     ^~~~~~

> > easier to add "break" here, no? That would be the minimal fix.
> 
> No. Then you would need to add same default to the inner switch.

Ah, you meant that pcm would be never outside of the given cases.
Yes, that's fine then, consider my patch as a bugreport.

Patch

diff --git a/drivers/usb/dwc3/trace.h b/drivers/usb/dwc3/trace.h
index f22714cce070..8e1625a6c19f 100644
--- a/drivers/usb/dwc3/trace.h
+++ b/drivers/usb/dwc3/trace.h
@@ -238,7 +238,7 @@  DECLARE_EVENT_CLASS(dwc3_log_trb,
 	),
 	TP_printk("%s: trb %p buf %08x%08x size %s%d ctrl %08x (%c%c%c%c:%c%c:%s)",
 		__get_str(name), __entry->trb, __entry->bph, __entry->bpl,
-		({char *s;
+		({ char *s = "";
 		int pcm = ((__entry->size >> 24) & 3) + 1;
 		switch (__entry->type) {
 		case USB_ENDPOINT_XFER_INT:
@@ -254,8 +254,6 @@  DECLARE_EVENT_CLASS(dwc3_log_trb,
 				s = "3x ";
 				break;
 			}
-		default:
-			s = "";
 		} s; }),
 		DWC3_TRB_SIZE_LENGTH(__entry->size), __entry->ctrl,
 		__entry->ctrl & DWC3_TRB_CTRL_HWO ? 'H' : 'h',