[PATCH weston] input: Don't use uninitialized variables in default_grab_pointer_focus()

[Derek Foreman]

On 10/04/15 02:16 AM, Pekka Paalanen wrote:
> On Wed,  8 Apr 2015 15:18:41 -0500
> Derek Foreman <derekf at osg.samsung.com> wrote:
> 
>> If we have no pointer focus and weston_compositor_pick_view() returns NULL
>> default_grab_pointer_focus() will test the unset sx, sy output parameters
>> from weston_compositor_pick_view().
>>
>> Instead, assume that since both pointers are NULL focus hasn't changed and
>> we don't need to update pointer focus.
>>
>> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
>> ---
>>
>> This fixes an error reported by valgrind.  I think the patch is correct but
>> it does stop pointer->focus_signal from being emitted when focus switches
>> from NULL to NULL.  I think that shouldn't have happened anyway?
> 
> If focus switches from NULL to NULL, weston_pointer_set_focus() would
> never be called because of the pointer->focus != view check, right?
> 
> So the case you saw in Valgrind must be a case of !NULL -> NULL switch,
> which has to be processed but does not have valid sx, sy by definition.
> Therefore...
> 
>> Initializing sx, sy to wl_fixed_from_int(0) would silence the warning too,
>> but I think this way is more correct...
> 
> ...isn't initializing the sx, sy the right thing to do, and this patch
> actually fixes nothing?
> 
> Did I miss something?

As did I.

As discussed on irc, the null to null case doesn't short circuit the ||
so the patch does "something"...

However, we probably shouldn't be getting here for a null to null switch
anyway, so I need to investigate why that's happened in the first place.

Initializing sx and sy would really have the same result, so I'm going
to avoid that for now as well.

>>  src/input.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/src/input.c b/src/input.c
>> index 142c670..18d1262 100644
>> --- a/src/input.c
>> +++ b/src/input.c
>> @@ -157,6 +157,9 @@ default_grab_pointer_focus(struct weston_pointer_grab *grab)
>>  					   pointer->x, pointer->y,
>>  					   &sx, &sy);
>>  
>> +	if (!pointer->focus && !view)
>> +		return;
>> +
>>  	if (pointer->focus != view || pointer->sx != sx || pointer->sy != sy)
>>  		weston_pointer_set_focus(pointer, view, sx, sy);
>>  }
> 
> 
> Thanks,
> pq
>