On Mon, Oct 27, 2014 at 09:14:40AM +0100, Marek Chalupa wrote:
> The callback returns always with the same serial,
> which is not right (it's serial, not constant...).
> This test highlights the bug.
> 
> Signed-off-by: Marek Chalupa <mchqwe...@gmail.com>
> ---
>  tests/display-test.c | 55 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/tests/display-test.c b/tests/display-test.c
> index a1e45b1..eb4ba1c 100644
> --- a/tests/display-test.c
> +++ b/tests/display-test.c
> @@ -593,3 +593,58 @@ TEST(threading_read_after_error_tst)
>  
>       display_destroy(d);
>  }
> +
> +static void
> +sync_callback(void *data, struct wl_callback *wl_callback, uint32_t serial)
> +{
> +     int *got = data;
> +     static uint32_t last_serial = 0;
> +
> +     /* if this is the first callback, just copy the value of serial */
> +     if (*got == 0)
> +             last_serial = serial;
> +     else
> +             ++last_serial;
> +
> +     /* since we only call display_sync, nothing else can increase the
> +      * serial, so the serails must be sequential */

sp. serials

> +     assert(serial == last_serial
> +            && "Serial is not sequential");
> +
> +     ++(*got);

I'm a bit confused about the intention of 'got', is it just a counter
for how many times the callback is called?  Would sync_callback_count
be a clearer variable name?

> +}
> +
> +static const struct wl_callback_listener sync_listener = {
> +     sync_callback
> +};
> +
> +#define CB_NUM 1000
> +static void
> +callback_serial_tst_main(void)
> +{
> +     int i, got = 0;
> +     struct wl_callback *cb;
> +     struct client *client = client_connect();

'client' maybe not a good choice of variable name since it's also the
type name.  'c' seems to be convention here, so maybe:

        struct client *c = client_connect();

> +     for (i = 0; i < CB_NUM; ++i) {
> +             cb = wl_display_sync(client->wl_display);
> +             wl_callback_add_listener(cb, &sync_listener, &got);
> +     }
> +
> +     wl_display_flush(client->wl_display);
> +     wl_display_roundtrip(client->wl_display);
> +
> +     assert(got == CB_NUM && "Lost some callback");
> +
> +     client_disconnect(client);
> +}
> +
> +TEST(callback_serial_tst)
> +{
> +     struct display *d = display_create();
> +
> +     client_create(d, callback_serial_tst_main);
> +     display_run(d);
> +
> +     display_destroy(d);
> +}
> -- 
> 1.9.3
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Other than the few quibbles, looks quite good.
And cheers for adding a test with the bugfix.  :-)

Reviewed-by: Bryce Harrington <b.harring...@samsung.com>

Bryce
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to