Re: [PATCH] usb: common: usb-otg-fsm: drop unreachable code in otg_statemachine()

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Wed, Feb 09, 2022 at 01:33:53PM +0300, Sergey Shtylyov wrote:
> On 2/9/22 8:35 AM, Greg Kroah-Hartman wrote:
> 
> >> The *switch* statement in otg_statemachine() does handle all possible OTG
> >> states explicitly, so the *default* label is unreachable.
> >>
> >> Found by Linux Verification Center (linuxtesting.org) with the SVACE static
> >> analysis tool.
> >>
> >> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> >>
> >> ---
> >> This patch is against the 'usb-next' branch of Greg KH's 'usb.git' repo.
> >> Peter Chen's 'usb.git' repo seems outdated, so I chose to ignore it...
> >>
> >>  drivers/usb/common/usb-otg-fsm.c |    2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> Index: usb/drivers/usb/common/usb-otg-fsm.c
> >> ===================================================================
> >> --- usb.orig/drivers/usb/common/usb-otg-fsm.c
> >> +++ usb/drivers/usb/common/usb-otg-fsm.c
> >> @@ -440,8 +440,6 @@ int otg_statemachine(struct otg_fsm *fsm
> >>  		if (fsm->id || fsm->a_bus_drop || fsm->a_clr_err)
> >>  			otg_set_state(fsm, OTG_STATE_A_WAIT_VFALL);
> >>  		break;
> >> -	default:
> >> -		break;
> >>  	}
> >>  	mutex_unlock(&fsm->lock);
> >>  
> > 
> > There is nothing wrong with leaving lines like this in the code to
> > handle any potential bugs.
> > Why do you think it needs to be removed?  What benefit does this patch
> > have?
> 
>    These lines as they are bring no value at all.
>    Note that (as I said)  all the values of 'enum usb_otg_state' are
> already handled with explicit *case* label...

And so now you will trigger the checkers that ask "you do not have a
default: line for your case statement!"

This is safer as is, please do not "clean up" things that are not
actually a problem.

thanks,

greg k-h