On Mon, 23 Jan 2017 11:49:24 -0600 Derek Foreman <der...@osg.samsung.com> wrote:
> Tests that the compositor is prevented from sending a resource id > from one client in an event to another client. > > This tests the (rare in practice) subset of the problem where both > clients magically happen to have the same kind of object with the > same id, so the client has no way to know it's being sent garbage. > > Signed-off-by: Derek Foreman <der...@osg.samsung.com> > --- > tests/display-test.c | 242 > +++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 242 insertions(+) > > This is a follow up to https://patchwork.freedesktop.org/patch/127077/ > (log an error for events with wrong client objects) > The test fails with current git master, but passes with patch 127077 > applied. Hi, display-test.c is getting rather long. Could there be a new category of tests? If it's appropriate here, then no worries. > diff --git a/tests/display-test.c b/tests/display-test.c > index 0c4df16..599db38 100644 > --- a/tests/display-test.c > +++ b/tests/display-test.c > @@ -926,3 +926,245 @@ TEST(error_on_destroyed_object) > display_resume(d); > display_destroy(d); > } > + > +struct mixed_client_server_data { > + struct display *d; > + struct client_info *success_client; > + struct wl_resource *success_mc; > + struct wl_resource *success_cb; > + struct client_info *fail_client; > + struct wl_resource *fail_mc; > + struct wl_resource *failure_cb; > +}; > + > +struct mixed_client_client_data { > + struct wl_proxy *mc; > + struct wl_callback *callback; > + bool done; > + bool should_fail; > +}; > + > +static const struct wl_interface *mixed_client_types[] = { > + &wl_callback_interface, > +}; > + > +static const struct wl_message mixed_client_requests[] = { > + { "callback", "n", mixed_client_types + 0 }, > +}; > + > +static const struct wl_message mixed_client_events[] = { > + { "right_client", "o", mixed_client_types + 0 }, > + { "wrong_client", "o", mixed_client_types + 0 }, > +}; > + > +static const struct wl_interface mixed_client_interface = { > + "mixed_client", 2, > + 1, mixed_client_requests, > + 2, mixed_client_events, > +}; How about just writing the few lines of XML to describe this and run it through the scanner instead? I don't see any reason to have hand-crafted code here. > + > +static void > +mixed_client_cb_done(void *data, struct wl_callback *callback, > + uint32_t unused) > +{ > + wl_callback_destroy(callback); > +} > + > +static const struct wl_callback_listener mixed_client_cb_listener = { > + mixed_client_cb_done > +}; > + > +static void > +mixed_client_success(void *data, struct wl_proxy *mixed_client, > + struct wl_callback *callback) > +{ > + struct mixed_client_client_data *client_data = data; > + > + client_data->done = true; > +} > + > +static void > +mixed_client_failure(void *data, struct wl_proxy *mixed_client, > + struct wl_callback *callback) > +{ > + assert(false); > +} > + > +static const struct { > + void (*right_client)(void *data, struct wl_proxy *mixed_client, > + struct wl_callback *new_callback); > + void (*wrong_client)(void *data, struct wl_proxy *mixed_client, > + struct wl_callback *broken_callback); > +} mixed_client_listener = { > + .right_client = mixed_client_success, > + .wrong_client = mixed_client_failure, > +}; > + > +static void > +mixed_client_registry_global(void *data, struct wl_registry *registry, > + uint32_t id, const char *interface, > + uint32_t version) > +{ > + struct mixed_client_client_data *client_data = data; > + > + if (strcmp(interface, "mixed_client") != 0) > + return; > + > + client_data->mc = wl_registry_bind(registry, id, > + &mixed_client_interface, > + client_data->should_fail + 1); should_fail + 1, err, what? > + wl_proxy_add_listener(client_data->mc, > + (void (**)(void)) &mixed_client_listener, > + data); > +} > + > +static const struct wl_registry_listener mixed_client_registry_listener = { > + mixed_client_registry_global, > + NULL, > +}; > + > +static void > +mixed_client_test(void *data) > +{ > + struct client *client = client_connect(); > + bool should_fail = (bool) (intptr_t) data; > + struct mixed_client_client_data client_data = { > + .mc = NULL, > + .done = false, > + .should_fail = should_fail, > + }; > + struct wl_registry *registry; > + int ret; > + > + registry = wl_display_get_registry(client->wl_display); > + wl_registry_add_listener(registry, &mixed_client_registry_listener, > + &client_data); > + wl_display_roundtrip(client->wl_display); > + wl_registry_destroy(registry); > + assert(client_data.mc); > + > + client_data.callback = > + (struct wl_callback *) > + wl_proxy_marshal_constructor(client_data.mc, 0, > + &wl_callback_interface, > + NULL); > + wl_callback_add_listener(client_data.callback, > + &mixed_client_cb_listener, > + &client_data); > + > + while (!client_data.done && ret >= 0) 'ret' used uninitialized? > + ret = wl_display_roundtrip(client->wl_display); > + > + wl_proxy_destroy(client_data.mc); > + if (should_fail) { > + assert(ret == -1); > + assert(wl_display_get_error(client->wl_display) != 0); > + assert(!client_data.done); > + } else { > + /* We never send a callback to the client that shouldn't > + * fail, so we have to clean it up here */ > + wl_callback_destroy(client_data.callback); > + assert(ret > 0); > + assert(wl_display_get_error(client->wl_display) == 0); > + assert(client_data.done); > + } > + > + client_disconnect_nocheck(client); > +} > + > +static void > +mixed_client_set_callback(struct wl_client *client, > + struct wl_resource *resource, > + uint32_t callback_id) > +{ > + struct mixed_client_server_data *mc = > + wl_resource_get_user_data(resource); > + bool should_fail = (mc->fail_client->wl_client == client); > + > + if (!should_fail) { > + mc->success_cb = wl_resource_create(client, > + &wl_callback_interface, > + 1, > + callback_id); > + assert(mc->success_cb); > + } else { > + mc->failure_cb = wl_resource_create(client, > + &wl_callback_interface, > + 1, > + callback_id); > + assert(mc->failure_cb); > + } > + > + if (!mc->fail_mc || !mc->success_mc) > + return; > + > + wl_resource_post_event(mc->success_mc, 0, mc->success_cb); > + /* This lets the failing client clean up its callback so there's no > + * leaked memory allocations. It also means the mixed-client > + * event will be on a zombie object, but that's still a valid test. > + */ > + wl_callback_send_done(mc->failure_cb, 1); > + wl_resource_post_event(mc->fail_mc, 1, mc->success_cb); > +} > + > +static const struct { > + void (*callback)(struct wl_client *client, > + struct wl_resource *resource, > + uint32_t callback_id); > +} mixed_client_implementation = { > + mixed_client_set_callback, > +}; > + > +static void > +bind_mixed_client(struct wl_client *client, void *data, > + uint32_t vers, uint32_t id) > +{ > + struct mixed_client_server_data *mc = data; > + struct display *d = mc->d; > + struct client_info *ci; > + struct wl_resource *res; > + bool should_fail = (mc->fail_client->wl_client == client); > + > + ci = find_client_info(d, client); > + assert(ci); > + > + res = wl_resource_create(client, &mixed_client_interface, vers, id); > + assert(res); > + wl_resource_set_implementation(res, &mixed_client_implementation, > + mc, NULL); > + > + if (!should_fail) > + mc->success_mc = res; > + else > + mc->fail_mc = res; > +} > + > +TEST(error_on_mixed_client_resource) > +{ > + struct client_info *c1, *c2; > + struct mixed_client_server_data mc; > + struct wl_global *global; > + > + memset(&mc, 0, sizeof(mc)); > + > + mc.d = display_create(); > + assert(mc.d); > + > + global = wl_global_create(mc.d->wl_display, &mixed_client_interface, > + 2, &mc, bind_mixed_client); > + assert(global); > + > + c1 = client_create(mc.d, mixed_client_test, (void *)(intptr_t) false); > + assert(c1); > + mc.success_client = c1; > + c2 = client_create(mc.d, mixed_client_test, (void *)(intptr_t) true); > + assert(c2); > + mc.fail_client = c2; > + > + display_run(mc.d); > + > + assert(mc.success_mc && mc.fail_mc && mc.success_cb && mc.failure_cb); > + > + wl_global_destroy(global); > + display_destroy(mc.d); > +} Ok, I didn't get to the bottom of this today. I find that calling the interface 'mixed_client' is quite confusing. There are 4 times more 'client' than 'server' and I found it hard to keep track what was what. Am I just tired, or do you think the code could be clarified a bit? The underlying idea is good, and I do see in the test logs that the error message added earlier in wayland-server is working. Thanks, pq
pgpSqQ4fYhvNe.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
