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

[Benjamin Tissoires]

Le 12/10/2010 03:20, Peter Hutterer a écrit :
> 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.

Arg, I miss the very first test.

so, for what it means, for this series of 4 patches:

Reviewed by: Benjamin Tissoires tissoire at cena.fr

I tried to find bugs, but one day I will!!!! ;-)

Cheers,
Benjamin

>
>>
>> (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);
>>>