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

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

[Sergey Shtylyov]

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);
 

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

[Greg Kroah-Hartman]

On Tue, Feb 08, 2022 at 11:32:28PM +0300, Sergey Shtylyov 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?

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

[Sergey Shtylyov]

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...

MBR, Sergey

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

[Greg Kroah-Hartman]

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