[PATCH synaptics 03/12] Move SHM hardware state update into a separate function.

[Peter Hutterer]

On Fri, May 07, 2010 at 02:31:11PM -0700, Jamey Sharp wrote:
> Reviewed-by: Jamey Sharp <jamey at minilop.net>
> 
> I generally prefer not to use the inline keyword. In this case since
> there's only one caller, it seems pretty clear that the compiler will
> inline the function anyway; and if it doesn't, there's probably a good
> reason not to.

removed, thanks.

> Also, since update_shm only uses local to get priv, which the caller
> already has, the code might be more clear if you pass the
> SynapticsPrivate directly.

Call it lazyness, but I prefer having the local handle, it allows me to get
any data for the device without having to change function parameters. That's
quite handy during debugging too, you can print out the name easier and keep
the diff small.

thanks for the review, I'll push the amended patch.

Cheers,
  Peter

> But whatever, my reviewed-by stands either way. :-)
> 
> Jamey
> 
> On Thu, May 6, 2010 at 9:41 PM, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> > No functional changes, this is just to move a slab of code out of mind when
> > reading.
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > ---
> >  src/synaptics.c |   56 ++++++++++++++++++++++++++++++++----------------------
> >  1 files changed, 33 insertions(+), 23 deletions(-)
> >
> > diff --git a/src/synaptics.c b/src/synaptics.c
> > index 5780be3..f765e30 100644
> > --- a/src/synaptics.c
> > +++ b/src/synaptics.c
> > @@ -2055,6 +2055,38 @@ HandleClickWithFingers(SynapticsParameters *para, struct SynapticsHwState *hw)
> >  }
> >
> >
> > +/* Update the hardware state in shared memory. This is read-only these days,
> > + * nothing in the driver reads back from SHM. SHM configuration is a thing of the past.
> > + */
> > +static inline void
> > +update_shm(const LocalDevicePtr local, const struct SynapticsHwState *hw)
> > +{
> > +    int i;
> > +    SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
> > +    SynapticsSHM *shm = priv->synshm;
> > +
> > +    if (!shm)
> > +           return;
> > +
> > +    shm->x = hw->x;
> > +    shm->y = hw->y;
> > +    shm->z = hw->z;
> > +    shm->numFingers = hw->numFingers;
> > +    shm->fingerWidth = hw->fingerWidth;
> > +    shm->left = hw->left;
> > +    shm->right = hw->right;
> > +    shm->up = hw->up;
> > +    shm->down = hw->down;
> > +    for (i = 0; i < 8; i++)
> > +           shm->multi[i] = hw->multi[i];
> > +    shm->middle = hw->middle;
> > +    shm->guest_left = hw->guest_left;
> > +    shm->guest_mid = hw->guest_mid;
> > +    shm->guest_right = hw->guest_right;
> > +    shm->guest_dx = hw->guest_dx;
> > +    shm->guest_dy = hw->guest_dy;
> > +}
> > +
> >  /*
> >  * React on changes in the hardware state. This function is called every time
> >  * the hardware state changes. The return value is used to specify how many
> > @@ -2065,7 +2097,6 @@ static int
> >  HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
> >  {
> >     SynapticsPrivate *priv = (SynapticsPrivate *) (local->private);
> > -    SynapticsSHM *shm = priv->synshm;
> >     SynapticsParameters *para = &priv->synpara;
> >     int finger;
> >     int dx, dy, buttons, rep_buttons, id;
> > @@ -2075,30 +2106,9 @@ HandleState(LocalDevicePtr local, struct SynapticsHwState *hw)
> >     int double_click, repeat_delay;
> >     int delay = 1000000000;
> >     int timeleft;
> > -    int i;
> >     Bool inside_active_area;
> >
> > -    /* update hardware state in shared memory */
> > -    if (shm)
> > -    {
> > -        shm->x = hw->x;
> > -        shm->y = hw->y;
> > -        shm->z = hw->z;
> > -        shm->numFingers = hw->numFingers;
> > -        shm->fingerWidth = hw->fingerWidth;
> > -        shm->left = hw->left;
> > -        shm->right = hw->right;
> > -        shm->up = hw->up;
> > -        shm->down = hw->down;
> > -        for (i = 0; i < 8; i++)
> > -            shm->multi[i] = hw->multi[i];
> > -        shm->middle = hw->middle;
> > -        shm->guest_left = hw->guest_left;
> > -        shm->guest_mid = hw->guest_mid;
> > -        shm->guest_right = hw->guest_right;
> > -        shm->guest_dx = hw->guest_dx;
> > -        shm->guest_dy = hw->guest_dy;
> > -    }
> > +    update_shm(local, hw);
> >
> >     /* If touchpad is switched off, we skip the whole thing and return delay */
> >     if (para->touchpad_off == 1)
> > --
> > 1.6.6.1
> >
> > _______________________________________________
> > xorg-devel at lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: http://lists.x.org/mailman/listinfo/xorg-devel
> >
>