Re: [PATCH] input/mouse/elantech.c: Remove trackpoint related label

Re: [PATCH] input/mouse/elantech.c: Remove trackpoint related label

[Benjamin Tissoires]

Hi,

On Jul 23 2016 or thereabouts, Marcos Paulo de Souza wrote:
> by rearranging the code to release the trackpoint resource when some error
> happens.

Is there any advantage of doing so beside trying to remove one goto?
Because if not, that's the kernel failure style and this is much better
given that if somebody needs to add other commands after initializing the
trackstick, and if those command fail, we can then remove the
trackstick.

Cheers,
Benjamin

> 
> Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> ---
>  drivers/input/mouse/elantech.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index be5b399..8c53a17 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1725,8 +1725,10 @@ int elantech_init(struct psmouse *psmouse)
>  		__set_bit(INPUT_PROP_POINTING_STICK, tp_dev->propbit);
>  
>  		error = input_register_device(etd->tp_dev);
> -		if (error < 0)
> -			goto init_fail_tp_reg;
> +		if (error < 0) {
> +			input_free_device(tp_dev);
> +			goto init_fail_tp_alloc;
> +		}
>  	}
>  
>  	psmouse->protocol_handler = elantech_process_byte;
> @@ -1735,8 +1737,6 @@ int elantech_init(struct psmouse *psmouse)
>  	psmouse->pktsize = etd->hw_version > 1 ? 6 : 4;
>  
>  	return 0;
> - init_fail_tp_reg:
> -	input_free_device(tp_dev);
>   init_fail_tp_alloc:
>  	sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
>  			   &elantech_attr_group);
> -- 
> 2.7.4
> 

Re: [PATCH] input/mouse/elantech.c: Remove trackpoint related label

[Dmitry Torokhov]

On Mon, Jul 25, 2016 at 10:34:28AM +0000, Marcos Souza wrote:
> Hi,
> 
> On Mon, Jul 25, 2016 at 6:54 AM Benjamin Tissoires <
> benjamin.tissoires@redhat.com> wrote:
> 
> > Hi,
> >
> > On Jul 23 2016 or thereabouts, Marcos Paulo de Souza wrote:
> > > by rearranging the code to release the trackpoint resource when some
> > error
> > > happens.
> >
> > Is there any advantage of doing so beside trying to remove one goto?
> > Because if not, that's the kernel failure style and this is much better
> > given that if somebody needs to add other commands after initializing the
> > trackstick, and if those command fail, we can then remove the
> > trackstick.
> >
> 
> My only reason was to shrink the code a little, by removing a label used

Shrink executable code or source code? I do not see either of them
shrinking, you have the same number of code lines and compiler is going
to rearrange the instructions the way it sees fit anyway.

> only once. Yes, we may need to add more more and the this label could be
> used to avoid duplication, but for now we don't have such code.
> 
> But OK, you can drop it if you don't think it's worth applying.
> 
> Thanks,
> 
> 
> >
> > Cheers,
> > Benjamin
> >
> > >
> > > Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>
> > > ---
> > >  drivers/input/mouse/elantech.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/input/mouse/elantech.c
> > b/drivers/input/mouse/elantech.c
> > > index be5b399..8c53a17 100644
> > > --- a/drivers/input/mouse/elantech.c
> > > +++ b/drivers/input/mouse/elantech.c
> > > @@ -1725,8 +1725,10 @@ int elantech_init(struct psmouse *psmouse)
> > >               __set_bit(INPUT_PROP_POINTING_STICK, tp_dev->propbit);
> > >
> > >               error = input_register_device(etd->tp_dev);
> > > -             if (error < 0)
> > > -                     goto init_fail_tp_reg;
> > > +             if (error < 0) {
> > > +                     input_free_device(tp_dev);
> > > +                     goto init_fail_tp_alloc;
> > > +             }
> > >       }
> > >
> > >       psmouse->protocol_handler = elantech_process_byte;
> > > @@ -1735,8 +1737,6 @@ int elantech_init(struct psmouse *psmouse)
> > >       psmouse->pktsize = etd->hw_version > 1 ? 6 : 4;
> > >
> > >       return 0;
> > > - init_fail_tp_reg:
> > > -     input_free_device(tp_dev);
> > >   init_fail_tp_alloc:
> > >       sysfs_remove_group(&psmouse->ps2dev.serio->dev.kobj,
> > >                          &elantech_attr_group);
> > > --
> > > 2.7.4
> > >
> >

Thanks.

-- 
Dmitry

Re: [PATCH] input/mouse/elantech.c: Remove trackpoint related label

[Dmitry Torokhov]

On Mon, Jul 25, 2016 at 11:33:32AM -0700, Dmitry Torokhov wrote:
> On Mon, Jul 25, 2016 at 10:34:28AM +0000, Marcos Souza wrote:
> > Hi,
> > 
> > On Mon, Jul 25, 2016 at 6:54 AM Benjamin Tissoires <
> > benjamin.tissoires@redhat.com> wrote:
> > 
> > > Hi,
> > >
> > > On Jul 23 2016 or thereabouts, Marcos Paulo de Souza wrote:
> > > > by rearranging the code to release the trackpoint resource when some
> > > error
> > > > happens.
> > >
> > > Is there any advantage of doing so beside trying to remove one goto?
> > > Because if not, that's the kernel failure style and this is much better
> > > given that if somebody needs to add other commands after initializing the
> > > trackstick, and if those command fail, we can then remove the
> > > trackstick.
> > >
> > 
> > My only reason was to shrink the code a little, by removing a label used
> 
> Shrink executable code or source code? I do not see either of them
> shrinking, you have the same number of code lines and compiler is going
> to rearrange the instructions the way it sees fit anyway.

By the way, it looks like you are taking the addresses from
git_maitainer.pl but the output is "interesting" to say the least. You
have

"open list:INPUT (KEYBOARD, MOUSE, JOYSTICK, TOUCHSCREEN),
linux-kernel@vger.kernel.org (open list)" <linux-input@vger.kernel.org>

as one of the recipients while.

Thanks.

-- 
Dmitry