[PATCH evdev 2/4] Don't pass pointers around to first_v and num_v.

[Peter Hutterer]

On Mon, Oct 11, 2010 at 01:09:50PM +0200, Benjamin Tissoires wrote:
> Hi Peter,
> 
> Le 11/10/2010 01:23, Peter Hutterer a écrit :
> >We only use them as values, no need for the addresses.
> >
> >Signed-off-by: Peter Hutterer<peter.hutterer at who-t.net>
> >---
> >  src/evdev.c |   16 ++++++++--------
> >  src/evdev.h |    4 ++--
> >  2 files changed, 10 insertions(+), 10 deletions(-)
> >
> >diff --git a/src/evdev.c b/src/evdev.c
> >index e5b3065..b634ab4 100644
> >--- a/src/evdev.c
> >+++ b/src/evdev.c
> >@@ -612,13 +612,13 @@ EvdevProcessKeyEvent(InputInfoPtr pInfo, struct input_event *ev)
> >   * Post the relative motion events.
> >   */
> >  void
> >-EvdevPostRelativeMotionEvents(InputInfoPtr pInfo, int *num_v, int *first_v,
> >+EvdevPostRelativeMotionEvents(InputInfoPtr pInfo, int num_v, int first_v,
> >                                int v[MAX_VALUATORS])
> >  {
> >      EvdevPtr pEvdev = pInfo->private;
> >
> >      if (pEvdev->rel) {
> >-        xf86PostMotionEventP(pInfo->dev, FALSE, *first_v, *num_v, v + *first_v);
> >+        xf86PostMotionEventP(pInfo->dev, FALSE, first_v, num_v, v + first_v);
> 
> Wasn't it the job of xf86PostMotionEventP to make the offset? (we
> passed first_v as an argument).

first_v is passed down to GPE and set in the event, that's the main reason
we have it there. the valuator array passed around is always num_v long.

> Also, in the file xserver/hw/xfree86/common/xf86Xinput.c, I've got:
> 	dy = valuators[1 - first_valuator];
> 
> making the offset in the caller will imply that the function
> xf86PostMotionEventP will read outside of the array.

that's a trick to get y from either index 0 or 1, depending on whether
first_valuator is 0 or 1. if you check again, there are checks in place to
ensure that first_valuator <= 1 before we even get here.

> 
> (same comment for the patch 3/4)
> 
> >      }
> >  }
> >
> >@@ -626,7 +626,7 @@ EvdevPostRelativeMotionEvents(InputInfoPtr pInfo, int *num_v, int *first_v,
> >   * Post the absolute motion events.
> >   */
> >  void
> >-EvdevPostAbsoluteMotionEvents(InputInfoPtr pInfo, int *num_v, int *first_v,
> >+EvdevPostAbsoluteMotionEvents(InputInfoPtr pInfo, int num_v, int first_v,
> >                                int v[MAX_VALUATORS])
> >  {
> >      EvdevPtr pEvdev = pInfo->private;
> >@@ -641,14 +641,14 @@ EvdevPostAbsoluteMotionEvents(InputInfoPtr pInfo, int *num_v, int *first_v,
> >       * just work.
> >       */
> >      if (pEvdev->abs&&  pEvdev->tool) {
> >-        xf86PostMotionEventP(pInfo->dev, TRUE, *first_v, *num_v, v);
> >+        xf86PostMotionEventP(pInfo->dev, TRUE, first_v, num_v, v);
> >      }
> >  }
> >
> >  /**
> >   * Post the queued key/button events.
> >   */
> >-static void EvdevPostQueuedEvents(InputInfoPtr pInfo, int *num_v, int *first_v,
> >+static void EvdevPostQueuedEvents(InputInfoPtr pInfo, int num_v, int first_v,
> >                                    int v[MAX_VALUATORS])
> >  {
> >      int i;
> >@@ -684,9 +684,9 @@ EvdevProcessSyncEvent(InputInfoPtr pInfo, struct input_event *ev)
> >
> >      EvdevProcessValuators(pInfo, v,&num_v,&first_v);
> >
> >-    EvdevPostRelativeMotionEvents(pInfo,&num_v,&first_v, v);
> >-    EvdevPostAbsoluteMotionEvents(pInfo,&num_v,&first_v, v);
> >-    EvdevPostQueuedEvents(pInfo,&num_v,&first_v, v);
> 
> I can not remember how this have worked... &num_v was an int**, no?
> I prefer the new approach (avoid int*).

num_v and first_v here are simple ints. I guess the code is there because at
one point during the devel cycle, num_v/first_v may have been modified in
the callee. It still does in EvdevProcessValuators, but not in the others
aymore.

Cheers,
  Peter

> >+    EvdevPostRelativeMotionEvents(pInfo, num_v, first_v, v);
> >+    EvdevPostAbsoluteMotionEvents(pInfo, num_v, first_v, v);
> >+    EvdevPostQueuedEvents(pInfo, num_v, first_v, v);
> >
> >      memset(pEvdev->delta, 0, sizeof(pEvdev->delta));
> >      memset(pEvdev->queue, 0, sizeof(pEvdev->queue));
> >diff --git a/src/evdev.h b/src/evdev.h
> >index 4945140..ce7f5f8 100644
> >--- a/src/evdev.h
> >+++ b/src/evdev.h
> >@@ -200,9 +200,9 @@ void EvdevQueueKbdEvent(InputInfoPtr pInfo, struct input_event *ev, int value);
> >  void EvdevQueueButtonEvent(InputInfoPtr pInfo, int button, int value);
> >  void EvdevPostButtonEvent(InputInfoPtr pInfo, int button, int value);
> >  void EvdevQueueButtonClicks(InputInfoPtr pInfo, int button, int count);
> >-void EvdevPostRelativeMotionEvents(InputInfoPtr pInfo, int *num_v, int *first_v,
> >+void EvdevPostRelativeMotionEvents(InputInfoPtr pInfo, int num_v, int first_v,
> >  				   int v[MAX_VALUATORS]);
> >-void EvdevPostAbsoluteMotionEvents(InputInfoPtr pInfo, int *num_v, int *first_v,
> >+void EvdevPostAbsoluteMotionEvents(InputInfoPtr pInfo, int num_v, int first_v,
> >  				   int v[MAX_VALUATORS]);
> >  unsigned int EvdevUtilButtonEventToButtonNumber(EvdevPtr pEvdev, int code);
> >