| Message ID | 20230314122714.1494260-1-Igor.A.Artemiev@mcst.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [lvc-project] Input: trackpoint - remove unreachable code | expand |
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 >
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.
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; }
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(-)