<div dir="ltr"><div>The issues I pointed out are not anything big. I'll send a patch that removes wl_display_dispatch_pending from both test-cases and I noticed that there are more leaks it the queue-test, so I'll get rid of them all in another patch if it's OK :) </div>Thanks, Marek </div><div class="gmail_extra"> <div class="gmail_quote">On 21 August 2014 10:57, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Thu, 21 Aug 2014 10:46:55 +0200 Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>> wrote: > Hi, > > > On 20 August 2014 18:29, Giulio Camuffo <<a href="mailto:giuliocamuffo@gmail.com">giuliocamuffo@gmail.com</a>> wrote: > > > --- > > tests/queue-test.c | 62 > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 62 insertions(+) > > > > diff --git a/tests/queue-test.c b/tests/queue-test.c > > index a4b165d..fc8a920 100644 > > --- a/tests/queue-test.c > > +++ b/tests/queue-test.c > > @@ -158,6 +158,63 @@ client_test_multiple_queues(void) > > } > > > > static void > > +sync_callback_roundtrip(void *data, struct wl_callback *callback, > > uint32_t serial) > > +{ > > + bool *done = data; > > + *done = true; > > +} > > + > > +static const struct wl_callback_listener sync_listener_roundtrip = { > > + sync_callback_roundtrip > > +}; > > + > > +/* Test that doing a roundtrip on a queue only the events on that > > + * queue get dispatched. */ > > +static int > > +client_test_queue_roundtrip(void) > > +{ > > + struct wl_event_queue *queue; > > + struct wl_callback *callback1; > > + struct wl_callback *callback2; > > + struct wl_display *display; > > + > > + display = wl_display_connect(SOCKET_NAME); > > + client_assert(display); > > + > > + /* Make the current thread the display thread. This is because > > + * wl_display_dispatch_queue() will only read the display fd if > > + * the main display thread has been set. */ > > + wl_display_dispatch_pending(display); > > > > I don't think this is necessary, this behavior was introduced in > 385fe30e8b144a968aa88c6546c2ef247771b3d7 and > 78cfa967681c965d23f6cbf76e080bbb0b564ff6 > and it was removed > in 3c7e8bfbb4745315b7bcbf69fa746c3d6718c305 (now there's only unused > variable display_thread in wl_display structure, we were discussing it back > on the list IIRC). > > > > > + queue = wl_display_create_queue(display); > > + client_assert(queue); > > + > > + bool done1 = false; > > + callback1 = wl_display_sync(display); > > + assert(callback1 != NULL); > > + wl_callback_add_listener(callback1, &sync_listener_roundtrip, > > &done1); > > + > > + bool done2 = false; > > + callback2 = wl_display_sync(display); > > + assert(callback2 != NULL); > > + wl_callback_add_listener(callback2, &sync_listener_roundtrip, > > &done2); > > + wl_proxy_set_queue((struct wl_proxy *) callback2, queue); > > + > > + wl_display_roundtrip(display); > > + client_assert(done1 == true); > > + client_assert(done2 == false); > > + > > + wl_display_roundtrip_queue(display, queue); > > + client_assert(done2 == true); > > + > > + wl_callback_destroy(callback1); > > + wl_callback_destroy(callback2); > > > > You're leaking the queue here. > > > > + wl_display_disconnect(display); > > + > > + return 0; > > +} > > + > > +static void > > client_alarm_handler(int sig) > > { > > fprintf(stderr, "Received SIGALRM signal, aborting.\n"); > > @@ -199,6 +256,11 @@ client_main(int fd) > > return EXIT_FAILURE; > > } > > > > + if (client_test_queue_roundtrip() != 0) { > > + fprintf(stderr, "queue rountrip test failed\n"); > > + return EXIT_FAILURE; > > + } > > + > > return EXIT_SUCCESS; > > } > > > > -- > > 2.0.4 > > > > _______________________________________________ > > wayland-devel mailing list > > <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a> > > <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a> > > > > Otherwise: > > Reviewed-by: Marek Chalupa <<a href="mailto:mchqwerty@gmail.com">mchqwerty@gmail.com</a>> </div></div>Oops, just pushed it already. Anyone care to follow up? :-) Thanks, pq </blockquote></div> </div>