[PATCH 1/2 v2] Clean up getValuatorEvents using array loop logic

[Jamey Sharp]

I considered proposing this approach too, as it's a lot cleaner. The
downside is that deviceValuator is a wire-protocol structure, and Xlib
tries to support systems where if you want a packed 32-bit field you
need to declare it as a bitfield. (See the B16 and B32 macros in
protocol headers.) I'm pretty sure the address-of operator won't work
on those systems, let alone assuming that an array packs the same way.

On the other hand, we don't support those systems at all in XCB, and
nobody has complained. There was general consensus that simply nobody
cares any more. Maybe the server is no longer supporting those systems
either, in which case this patch is clearly the right way to go.
Otherwise maybe it's time to decide that the server shouldn't support
those systems going forward, and people can just run an old version of
the server if they have to. But absent either of those decisions, I
guess I'd stick with the switch.

Still, if nobody else vetoes this patch:
Reviewed-by: Jamey Sharp <jamey at minilop.net>

I can't comment on the second patch in the series, but this one is an
improvement regardless.
Jamey

On Mon, Mar 28, 2011 at 1:04 PM, Chase Douglas
<chase.douglas at canonical.com> wrote:
> Signed-off-by: Chase Douglas <chase.douglas at canonical.com>
> ---
>  dix/eventconvert.c |   20 ++++++--------------
>  1 files changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/dix/eventconvert.c b/dix/eventconvert.c
> index 7834d68..9192080 100644
> --- a/dix/eventconvert.c
> +++ b/dix/eventconvert.c
> @@ -378,25 +378,17 @@ getValuatorEvents(DeviceEvent *ev, deviceValuator *xv)
>
>     /* FIXME: non-continuous valuator data in internal events*/
>     for (i = 0; i < num_valuators; i += 6, xv++) {
> +        INT32 *valuators = &xv->valuator0; // Treat all 6 vals as an array
> +        int j;
> +
>         xv->type = DeviceValuator;
>         xv->first_valuator = first_valuator + i;
>         xv->num_valuators = ((num_valuators - i) > 6) ? 6 : (num_valuators - i);
>         xv->deviceid = ev->deviceid;
>         xv->device_state = state;
> -        switch (xv->num_valuators) {
> -        case 6:
> -            xv->valuator5 = ev->valuators.data[xv->first_valuator + 5];
> -        case 5:
> -            xv->valuator4 = ev->valuators.data[xv->first_valuator + 4];
> -        case 4:
> -            xv->valuator3 = ev->valuators.data[xv->first_valuator + 3];
> -        case 3:
> -            xv->valuator2 = ev->valuators.data[xv->first_valuator + 2];
> -        case 2:
> -            xv->valuator1 = ev->valuators.data[xv->first_valuator + 1];
> -        case 1:
> -            xv->valuator0 = ev->valuators.data[xv->first_valuator + 0];
> -        }
> +
> +        for (j = 0; j < xv->num_valuators; j++)
> +            valuators[j] = ev->valuators.data[xv->first_valuator + j];
>
>         if (i + 6 < num_valuators)
>             xv->deviceid |= MORE_EVENTS;
> --
> 1.7.4.1
>
>