[lvc-project] [PATCH] Input: trackpoint - remove unreachable code

[lvc-project] [PATCH] Input: trackpoint - remove unreachable code

[Igor Artemiev]

The trackpoint_sync() function always returnd 0.
And there is no need to check its result.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 2a924d71794c ("Input: trackpoint - only expose supported controls for Elan, ALPS and NXP")
Signed-off-by: Igor Artemiev <Igor.A.Artemiev@mcst.ru>
---
 drivers/input/mouse/trackpoint.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index 4a86b3e31d3b..561a4d2d81ff 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -386,9 +386,7 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
 	was_reset = tp->variant_id == TP_VARIANT_IBM &&
 		    trackpoint_power_on_reset(&psmouse->ps2dev) == 0;
 
-	error = trackpoint_sync(psmouse, was_reset);
-	if (error)
-		return error;
+	trackpoint_sync(psmouse, was_reset);
 
 	return 0;
 }
-- 
2.30.2

Re: [lvc-project] [PATCH] Input: trackpoint - remove unreachable code

[Andi Shyti]

Hi Igor,

On Tue, Mar 14, 2023 at 03:27:14PM +0300, Igor Artemiev wrote:
> The trackpoint_sync() function always returnd 0.
> And there is no need to check its result.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: 2a924d71794c ("Input: trackpoint - only expose supported controls for Elan, ALPS and NXP")
> Signed-off-by: Igor Artemiev <Igor.A.Artemiev@mcst.ru>

I don't think it requires the Fixes tag... it's not really
broken.

> ---
>  drivers/input/mouse/trackpoint.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> index 4a86b3e31d3b..561a4d2d81ff 100644
> --- a/drivers/input/mouse/trackpoint.c
> +++ b/drivers/input/mouse/trackpoint.c
> @@ -386,9 +386,7 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
>  	was_reset = tp->variant_id == TP_VARIANT_IBM &&
>  		    trackpoint_power_on_reset(&psmouse->ps2dev) == 0;
>  
> -	error = trackpoint_sync(psmouse, was_reset);
> -	if (error)
> -		return error;
> +	trackpoint_sync(psmouse, was_reset);

what worries me here is that if this returns always '0' who tells
me that it will always return '0'?

One day someone might add an error return path and you would miss
it here.

Would it make sense to make the trackpoint_sync() a void function
as well?

Andi

>  
>  	return 0;
>  }
> -- 
> 2.30.2
> 

Re: [lvc-project] [PATCH] Input: trackpoint - remove unreachable code

[Dmitry Torokhov]

On Tue, Mar 14, 2023 at 02:13:56PM +0100, Andi Shyti wrote:
> Hi Igor,
> 
> On Tue, Mar 14, 2023 at 03:27:14PM +0300, Igor Artemiev wrote:
> > The trackpoint_sync() function always returnd 0.
> > And there is no need to check its result.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > 
> > Fixes: 2a924d71794c ("Input: trackpoint - only expose supported controls for Elan, ALPS and NXP")
> > Signed-off-by: Igor Artemiev <Igor.A.Artemiev@mcst.ru>
> 
> I don't think it requires the Fixes tag... it's not really
> broken.
> 
> > ---
> >  drivers/input/mouse/trackpoint.c | 4 +---
> >  1 file changed, 1 insertion(+), 3 deletions(-)
> > 
> > diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
> > index 4a86b3e31d3b..561a4d2d81ff 100644
> > --- a/drivers/input/mouse/trackpoint.c
> > +++ b/drivers/input/mouse/trackpoint.c
> > @@ -386,9 +386,7 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
> >  	was_reset = tp->variant_id == TP_VARIANT_IBM &&
> >  		    trackpoint_power_on_reset(&psmouse->ps2dev) == 0;
> >  
> > -	error = trackpoint_sync(psmouse, was_reset);
> > -	if (error)
> > -		return error;
> > +	trackpoint_sync(psmouse, was_reset);
> 
> what worries me here is that if this returns always '0' who tells
> me that it will always return '0'?
> 
> One day someone might add an error return path and you would miss
> it here.
> 
> Would it make sense to make the trackpoint_sync() a void function
> as well?

Yes if we are dropping the check we should also change the function to
not return anything.

Thanks.

-- 
Dmitry

[lvc-project] [PATCH v2] Input: trackpoint - remove unreachable code

[Igor Artemiev]

The trackpoint_sync() function always returns 0.
And there is no need to check its result.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Igor Artemiev <Igor.A.Artemiev@mcst.ru>
---
v2: make the trackpoint_sync() a void function 
as Andi Shyti <andi.shyti@kernel.org> suggested.
 
 drivers/input/mouse/trackpoint.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index 4a86b3e31d3b..2c381377ad4b 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -300,7 +300,7 @@ static int trackpoint_start_protocol(struct psmouse *psmouse,
  *		      power-on reset was run). If so, values will only be
  *		      written to TP if they differ from power-on default.
  */
-static int trackpoint_sync(struct psmouse *psmouse, bool in_power_on_state)
+static void trackpoint_sync(struct psmouse *psmouse, bool in_power_on_state)
 {
 	struct trackpoint_data *tp = psmouse->private;
 
@@ -340,8 +340,6 @@ static int trackpoint_sync(struct psmouse *psmouse, bool in_power_on_state)
 	TRACKPOINT_UPDATE(in_power_on_state, psmouse, tp, press_to_select);
 	TRACKPOINT_UPDATE(in_power_on_state, psmouse, tp, skipback);
 	TRACKPOINT_UPDATE(in_power_on_state, psmouse, tp, ext_dev);
-
-	return 0;
 }
 
 static void trackpoint_defaults(struct trackpoint_data *tp)
@@ -386,9 +384,7 @@ static int trackpoint_reconnect(struct psmouse *psmouse)
 	was_reset = tp->variant_id == TP_VARIANT_IBM &&
 		    trackpoint_power_on_reset(&psmouse->ps2dev) == 0;
 
-	error = trackpoint_sync(psmouse, was_reset);
-	if (error)
-		return error;
+	trackpoint_sync(psmouse, was_reset);
 
 	return 0;
 }
-- 
2.30.2