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
pgpVJhOMhnouY.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel