Pushed
Thanks,
Derek
On 15/10/15 02:30 AM, Pekka Paalanen wrote:
> On Wed, 14 Oct 2015 20:57:35 +0200
> Hardening <rdp.eff...@gmail.com> wrote:
>
>> Le 14/10/2015 16:39, Derek Foreman a écrit :
>>> Apparently it's possible for a compositor to advertise seats with
>>> different versions on the same connection, so this makes us more robust
>>> against that dubious behaviour.
>>>
>>> This also tracks the seat version we requested instead of the advertised
>>> maximum.
>>>
>>> Signed-off-by: Derek Foreman <der...@osg.samsung.com>
>>> ---
>>>
>>> As penance for my sass, I've gone ahead and "fixed" this - I think
>>> tracking the version we requested instead of the one advertised is
>>> more robust against catching a failure to increment the requested
>>> version though. However, it does seem like the correct thing to do.
>>>
>>> clients/window.c | 20 +++++++++++---------
>>> 1 file changed, 11 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/clients/window.c b/clients/window.c
>>> index 6d3e944..d8f2ee2 100644
>>> --- a/clients/window.c
>>> +++ b/clients/window.c
>>> @@ -140,7 +140,6 @@ struct display {
>>> void *dummy_surface_data;
>>>
>>> int has_rgb565;
>>> - int seat_version;
>>> int data_device_manager_version;
>>> };
>>>
>>> @@ -362,6 +361,7 @@ struct input {
>>> uint32_t repeat_sym;
>>> uint32_t repeat_key;
>>> uint32_t repeat_time;
>>> + int seat_version;
>>> };
>>>
>>> struct output {
>>> @@ -3256,7 +3256,7 @@ seat_handle_capabilities(void *data, struct wl_seat
>>> *seat,
>>> wl_pointer_add_listener(input->pointer, &pointer_listener,
>>> input);
>>> } else if (!(caps & WL_SEAT_CAPABILITY_POINTER) && input->pointer) {
>>> - if (input->display->seat_version >=
>>> WL_POINTER_RELEASE_SINCE_VERSION)
>>> + if (input->seat_version >= WL_POINTER_RELEASE_SINCE_VERSION)
>>> wl_pointer_release(input->pointer);
>>> else
>>> wl_pointer_destroy(input->pointer);
>>> @@ -3269,7 +3269,7 @@ seat_handle_capabilities(void *data, struct wl_seat
>>> *seat,
>>> wl_keyboard_add_listener(input->keyboard, &keyboard_listener,
>>> input);
>>> } else if (!(caps & WL_SEAT_CAPABILITY_KEYBOARD) && input->keyboard) {
>>> - if (input->display->seat_version >=
>>> WL_KEYBOARD_RELEASE_SINCE_VERSION)
>>> + if (input->seat_version >= WL_KEYBOARD_RELEASE_SINCE_VERSION)
>>> wl_keyboard_release(input->keyboard);
>>> else
>>> wl_keyboard_destroy(input->keyboard);
>>> @@ -3281,7 +3281,7 @@ seat_handle_capabilities(void *data, struct wl_seat
>>> *seat,
>>> wl_touch_set_user_data(input->touch, input);
>>> wl_touch_add_listener(input->touch, &touch_listener, input);
>>> } else if (!(caps & WL_SEAT_CAPABILITY_TOUCH) && input->touch) {
>>> - if (input->display->seat_version >=
>>> WL_TOUCH_RELEASE_SINCE_VERSION)
>>> + if (input->seat_version >= WL_TOUCH_RELEASE_SINCE_VERSION)
>>> wl_touch_release(input->touch);
>>> else
>>> wl_touch_destroy(input->touch);
>>> @@ -5218,17 +5218,20 @@ fini_xkb(struct input *input)
>>> }
>>>
>>> static void
>>> -display_add_input(struct display *d, uint32_t id)
>>> +display_add_input(struct display *d, uint32_t id, int display_seat_version)
>>> {
>>> struct input *input;
>>> + int seat_version = MIN(display_seat_version, 4);
>>>
>>> input = xzalloc(sizeof *input);
>>> input->display = d;
>>> input->seat = wl_registry_bind(d->registry, id, &wl_seat_interface,
>>> - MIN(d->seat_version, 4));
>>> + seat_version);
>>> input->touch_focus = NULL;
>>> input->pointer_focus = NULL;
>>> input->keyboard_focus = NULL;
>>> + input->seat_version = seat_version;
>>> +
>>> wl_list_init(&input->touch_point_list);
>>> wl_list_insert(d->input_list.prev, &input->link);
>>>
>>> @@ -5278,7 +5281,7 @@ input_destroy(struct input *input)
>>> else
>>> wl_data_device_destroy(input->data_device);
>>> }
>>> - if (input->display->seat_version >= WL_POINTER_RELEASE_SINCE_VERSION) {
>>> + if (input->seat_version >= WL_POINTER_RELEASE_SINCE_VERSION) {
>>> if (input->touch)
>>> wl_touch_release(input->touch);
>>> if (input->pointer)
>>> @@ -5365,8 +5368,7 @@ registry_handle_global(void *data, struct wl_registry
>>> *registry, uint32_t id,
>>> } else if (strcmp(interface, "wl_output") == 0) {
>>> display_add_output(d, id);
>>> } else if (strcmp(interface, "wl_seat") == 0) {
>>> - d->seat_version = version;
>>> - display_add_input(d, id);
>>> + display_add_input(d, id, version);
>>> } else if (strcmp(interface, "wl_shm") == 0) {
>>> d->shm = wl_registry_bind(registry, id, &wl_shm_interface, 1);
>>> wl_shm_add_listener(d->shm, &shm_listener, d);
>>>
>>
>> I had noticed that too, I can't see in which situation this could
>> happen, perhaps a subcompositor advertising its own seats ?
>
> There is no such thing. :-)
>
> I think this is more a correctness thing than anything that actually
> happens.
>
>> Anyway,
>>
>> Reviewed-by: David FORT <cont...@hardening-consulting.com>
>>
>
> Yeah, looks good to me too.
> Reviewed-by: Pekka Paalanen <pekka.paala...@collabora.co.uk>
>
>
> Thanks,
> pq
>
>
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
