[PATCH weston] tests: Adding simple waycheck validation tool.

Bryce Harrington bryce at osg.samsung.com
Wed Oct 14 14:36:33 PDT 2015


On Wed, Oct 14, 2015 at 04:17:53PM +0300, Pekka Paalanen wrote:
> On Wed,  7 Oct 2015 16:11:43 -0700
> "Jon A. Cruz" <jonc at osg.samsung.com> wrote:
> 
> > Adds an initial implementation of a testing tool that uses the unit
> > test framework to run checks against an arbitrary Wayland compositor.
> > Note that this is not intended for Weston-specific testing, but for
> > generic Wayland testing.
> > 
> > Signed-off-by: Jon A. Cruz <jonc at osg.samsung.com>
> 
> Hi Jon,
> 
> nice to see progress. :-)
> 
> I tested, waycheck runs fine if you run it manually. So this patch is
> not even intended to hook it up to 'make check' at all, but just
> compile a program you can run with any compositor?
> 
> When you later do hook things up into Weston's 'make check', will that
> re-use the waycheck code or only the wayland_fixtures code?
> 
> I think the commit message should say which existing upstream tests
> this patch reimplements. If it doesn't, it should say what we are
> testing instead, in few words. Comments in waycheck.c could explain in
> more detail about what is being tested.

Further, I am not sure we should land this until the test cases have
been reimplemented to use it.  Having two test frameworks in the
codebase increases complexity, and it's already tough to find folks to
work on tests.

> More comments inline.
> 
> > ---
> >  .gitignore                                 |    1 +
> >  Makefile.am                                |   56 +-
> >  doc/doxygen/tooldev.doxygen.in             |    3 +-
> >  doc/doxygen/tools.doxygen.in               |    6 +-
> >  tools/devtools.dox                         |   55 ++
> >  tools/tools.dox                            |   32 +
> >  tools/tools_arch_new.gv                    |   85 ++
> >  tools/tools_arch_old.gv                    |   53 ++
> >  tools/waycheck/rough_draw.c                |  279 +++++++
> >  tools/waycheck/rough_draw.h                |   49 ++
> >  tools/waycheck/waycheck.c                  |  401 ++++++++++
> >  tools/waycheck/waycheck.dox                |   29 +
> >  tools/wayland_fixtures/inc/wtst_fixtures.h |  263 +++++++
> >  tools/wayland_fixtures/src/wtst_fixtures.c | 1178 ++++++++++++++++++++++++++++
> >  14 files changed, 2479 insertions(+), 11 deletions(-)
> >  create mode 100644 tools/devtools.dox
> >  create mode 100644 tools/tools.dox
> >  create mode 100644 tools/tools_arch_new.gv
> >  create mode 100644 tools/tools_arch_old.gv
> >  create mode 100644 tools/waycheck/rough_draw.c
> >  create mode 100644 tools/waycheck/rough_draw.h
> >  create mode 100644 tools/waycheck/waycheck.c
> >  create mode 100644 tools/waycheck/waycheck.dox
> >  create mode 100644 tools/wayland_fixtures/inc/wtst_fixtures.h
> >  create mode 100644 tools/wayland_fixtures/src/wtst_fixtures.c
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 11d23da..7278bcf 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -106,4 +106,5 @@ weston.ini.5
> >  /tests/weston-ivi.ini
> >  internal-screenshot-00.png
> >  
> > +/waycheck
> >  /zuctest
> > diff --git a/Makefile.am b/Makefile.am
> > index 1900390..00cf9c7 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -972,7 +972,7 @@ endif
> >  #
> >  
> >  noinst_LTLIBRARIES += libshared.la libshared-cairo.la \
> > -	libzunitc.la libzunitcmain.la
> > +	libzunitc.la libzunitcmain.la libwayland-fixtures.la
> >  
> >  libshared_la_CFLAGS = $(AM_CFLAGS) $(COMPOSITOR_CFLAGS)
> >  
> > @@ -1011,6 +1011,22 @@ libshared_cairo_la_SOURCES =			\
> >  	shared/frame.c				\
> >  	shared/cairo-util.h
> >  
> > +libwayland_fixtures_la_SOURCES = \
> > +	tools/wayland_fixtures/inc/wtst_fixtures.h	\
> > +	tools/wayland_fixtures/src/wtst_fixtures.c	\
> > +	shared/helpers.h
> > +
> > +libwayland_fixtures_la_CFLAGS = \
> > +	-I$(top_srcdir)/tools/wayland_fixtures/inc	\
> > +	-I$(top_srcdir)/tools/zunitc/inc	\
> > +	$(TEST_CLIENT_CFLAGS)			\
> > +	$(AM_CFLAGS)
> > +
> > +libwayland_fixtures_la_LIBADD = \
> > +	$(TEST_CLIENT_LIBS)	\
> > +	libzunitc.la		\
> > +	libshared.la
> > +
> >  libzunitc_la_SOURCES = \
> >  	tools/zunitc/inc/zunitc/zunitc.h	\
> >  	tools/zunitc/inc/zunitc/zunitc_impl.h	\
> > @@ -1028,8 +1044,8 @@ libzunitc_la_SOURCES = \
> >  	shared/helpers.h
> >  
> >  libzunitc_la_CFLAGS = \
> > -	$(AM_CFLAGS)				\
> > -	-I$(top_srcdir)/tools/zunitc/inc
> > +	-I$(top_srcdir)/tools/zunitc/inc	\
> > +	$(AM_CFLAGS)
> >  
> >  libzunitc_la_LIBADD = \
> >  	libshared.la
> > @@ -1045,8 +1061,8 @@ libzunitcmain_la_SOURCES = \
> >  	tools/zunitc/src/main.c
> >  
> >  libzunitcmain_la_CFLAGS = \
> > -	$(AM_CFLAGS)				\
> > -	-I$(top_srcdir)/tools/zunitc/inc
> > +	-I$(top_srcdir)/tools/zunitc/inc	\
> > +	$(AM_CFLAGS)
> 
> These two above hunks are a bit gratuitous. I suppose you want
> AM_CFLAGS to override specfici CFLAGS? Not there is really anything
> overridable. Or what's the reason for this?
> 
> >  
> >  libzunitcmain_la_LIBADD =	\
> >  	libzunitc.la		\
> > @@ -1305,7 +1321,7 @@ setbacklight_CFLAGS = $(AM_CFLAGS) $(SETBACKLIGHT_CFLAGS)
> >  setbacklight_LDADD = $(SETBACKLIGHT_LIBS)
> >  endif
> >  
> > -all-local: zuctest$(EXEEXT)
> > +all-local: waycheck$(EXEEXT) zuctest$(EXEEXT)
> >  
> >  noinst_PROGRAMS += zuctest$(EXEEXT)
> >  
> > @@ -1313,14 +1329,36 @@ zuctest_LDADD =			\
> >  	libzunitc.la		\
> >  	libzunitcmain.la
> >  
> > -zuctest_CFLAGS =				\
> > -	$(AM_CFLAGS)				\
> > -	-I$(top_srcdir)/tools/zunitc/inc
> > +zuctest_CFLAGS =			\
> > +	-I$(top_srcdir)/tools/zunitc/inc	\
> > +	$(AM_CFLAGS)
> 
> ...and this one, too.
> 
> >  
> >  zuctest_SOURCES =				\
> >  	tools/zunitc/test/fixtures_test.c	\
> >  	tools/zunitc/test/zunitc_test.c
> >  
> > +noinst_PROGRAMS += waycheck$(EXEEXT)
> > +
> > +waycheck_LDADD =			\
> > +	$(TEST_CLIENT_LIBS)		\
> > +	libwayland-fixtures.la		\
> > +	libzunitcmain.la		\
> > +	libshared.la
> > +
> > +waycheck_CFLAGS =				\
> > +	-I$(top_srcdir)/tools/zunitc/inc	\
> > +	-I$(top_srcdir)/tools/wayland_fixtures/inc	\
> > +	-I$(top_builddir)/clients		\
> 
> I don't see any headers in $(top_builddir)/clients, why is this here?
> 
> 
> > +	-I$(top_srcdir)/shared			\
> > +	$(TEST_CLIENT_CFLAGS)			\
> > +	$(AM_CFLAGS)
> > +
> > +waycheck_SOURCES =				\
> > +	tools/waycheck/waycheck.c		\
> > +	tools/waycheck/rough_draw.c		\
> > +	tools/waycheck/rough_draw.h		\
> > +	shared/helpers.h
> > +
> >  EXTRA_DIST +=							\
> >  	tests/weston-tests-env					\
> >  	tests/internal-screenshot.ini				\
> > diff --git a/doc/doxygen/tooldev.doxygen.in b/doc/doxygen/tooldev.doxygen.in
> > index 8b033b3..8998b8b 100644
> > --- a/doc/doxygen/tooldev.doxygen.in
> > +++ b/doc/doxygen/tooldev.doxygen.in
> > @@ -5,7 +5,8 @@ OPTIMIZE_OUTPUT_FOR_C  = YES
> >  EXTRACT_ALL            = YES
> >  INPUT                  = \
> >  		       @top_srcdir@/doc/doxygen/devtools.dox \
> > -		       @top_srcdir@/tools/zunitc
> > +		       @top_srcdir@/tools/zunitc \
> > +		       @top_srcdir@/tools/waycheck
> >  RECURSIVE              = YES
> >  GENERATE_LATEX         = NO
> >  DOTFILE_DIRS           = @top_srcdir@/doc/doxygen
> > diff --git a/doc/doxygen/tools.doxygen.in b/doc/doxygen/tools.doxygen.in
> > index 1b3b078..6947784 100644
> > --- a/doc/doxygen/tools.doxygen.in
> > +++ b/doc/doxygen/tools.doxygen.in
> > @@ -5,6 +5,10 @@ OPTIMIZE_OUTPUT_FOR_C  = YES
> >  INPUT                  = \
> >  		       @top_srcdir@/doc/doxygen/tools.dox \
> >  		       @top_srcdir@/tools/zunitc/doc/zunitc.dox \
> > -		       @top_srcdir@/tools/zunitc/inc/zunitc/zunitc.h
> > +		       @top_srcdir@/tools/zunitc/inc/zunitc/zunitc.h \
> > +		       @top_srcdir@/tools/waycheck/waycheck.dox \
> > +		       @top_srcdir@/tools/wayland_fixtures/inc/wtst_fixtures.h \
> > +		       @top_srcdir@/tools/wayland_fixtures/src/wtst_fixtures.c \
> > +		       @top_srcdir@/tools/waycheck/rough_draw.h
> >  GENERATE_LATEX         = NO
> >  DOTFILE_DIRS           = @top_srcdir@/doc/doxygen
> > diff --git a/tools/devtools.dox b/tools/devtools.dox
> > new file mode 100644
> > index 0000000..08d1572
> > --- /dev/null
> > +++ b/tools/devtools.dox
> > @@ -0,0 +1,55 @@
> > +/*
> > + * Copyright © 2015 Samsung Electronics Co., Ltd
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +/**
> > + at mainpage
> > +
> > +- @ref zunitc - Simple test framework
> > +
> > +- @ref waycheck - Simple integration/acceptance test tool
> > +
> > + at section tools_overview Overview
> > +
> > +The tools area currently consists of two sub-projects (@ref zunitc and
> > + at ref waycheck) that are refined from the prior single weston/tests source
> > +folder.
> > +
> > + at subsection tools_overview_old Old Code Organization
> > +
> > +The original 'tests' folder contained basic weston testing with an
> > +integrated test runner framework. Over time things progressed to the
> > +stage where splitting apart into discrete layers was warranted.
> > +
> > + at dotfile tools_arch_old.gv "Original test code organization"
> > +
> > + at subsection tools_overview_new New Code Organization
> > +
> > +The test code that is not weston-specific gets split out to a separate
> > +folder and/or folders. Then an additional testing tool 'waycheck' that is
> > +compositor-agnostic is added.
> > +
> > + at dotfile tools_arch_new.gv "Refactored test code organization"
> > +
> > +*/
> 
> 
> This file looks a lot like doc/doxygen/devtools.dox.
> Is one of them too much?
> 
> 
> > diff --git a/tools/tools.dox b/tools/tools.dox
> > new file mode 100644
> > index 0000000..0be1372
> > --- /dev/null
> > +++ b/tools/tools.dox
> > @@ -0,0 +1,32 @@
> > +/*
> > + * Copyright © 2015 Samsung Electronics Co., Ltd
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +/**
> > + at mainpage
> > +
> > +- @ref zunitc - Simple test framework
> > +
> > +- @ref waycheck - Simple integration/acceptance test tool
> > +*/
> 
> This file looks a lot like doc/doxygen/tools.dox.
> Should there be only one?
> 
> 
> > diff --git a/tools/tools_arch_new.gv b/tools/tools_arch_new.gv
> > new file mode 100644
> > index 0000000..e4626f3
> > --- /dev/null
> > +++ b/tools/tools_arch_new.gv
> 
> This file is identical to doc/doxygen/tools_arch_new.gv.
> 
> > diff --git a/tools/tools_arch_old.gv b/tools/tools_arch_old.gv
> > new file mode 100644
> > index 0000000..1b38123
> > --- /dev/null
> > +++ b/tools/tools_arch_old.gv
> 
> And this file is identical to doc/doxygen/tools_arch_old.gv.
> 
> One copy should be enough, right?
> 
> 
> > diff --git a/tools/waycheck/rough_draw.c b/tools/waycheck/rough_draw.c
> > new file mode 100644
> > index 0000000..b29a5b4
> > --- /dev/null
> > +++ b/tools/waycheck/rough_draw.c
> > @@ -0,0 +1,279 @@
> > +/*
> > + * Copyright © 2015 Samsung Electronics Co., Ltd
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#include "config.h"
> > +
> > +#include <stddef.h>
> > +#include <string.h>
> > +
> > +#include "rough_draw.h"
> > +#include "shared/helpers.h"
> > +
> > +/**
> > + * Very simple way to track colors.
> > + */
> > +struct colorval {
> > +	float r;
> > +	float g;
> > +	float b;
> > +	float a;
> > +	enum wl_shm_format form;
> > +	int used;
> > +	uint8_t raw[4];
> > +};
> > +
> > +static void
> > +populate_color(struct colorval *color, enum wl_shm_format form)
> > +{
> > +	memset(color->raw, 0, sizeof(color->raw));
> > +	color->form = form;
> > +	switch (form) {
> > +	case WL_SHM_FORMAT_ARGB8888:
> > +		color->used = 4;
> > +		color->raw[3] = (0xff * color->a) + 0.5;
> > +		color->raw[2] = (0xff * color->r) + 0.5;
> > +		color->raw[1] = (0xff * color->g) + 0.5;
> > +		color->raw[0] = (0xff * color->b) + 0.5;
> 
> ARGB8888 is defined like in pixman, as bits of an uint32_t. Therefore
> unconditional assignment to an uint8_t array is not endianess-agnostic.
> 
> > +		break;
> > +	case WL_SHM_FORMAT_RGB565:
> > +		color->used = 2;
> > +		color->raw[1] = (uint8_t)((0x1f * color->r) + 0.5) << 3;
> > +		color->raw[0] = (0x3f * color->g) + 0.5;
> > +		color->raw[1] |= 0x07 & (color->raw[0] >> 3);
> > +		color->raw[0] <<= 5;
> > +		color->raw[0] |= (uint8_t)((0x1f * color->b) + 0.5);
> 
> I suppose this has the same endianess issue.
> 
> > +		break;
> > +	default:
> > +		color->used = 0;
> > +	}
> > +}
> > +
> > +static void
> > +fill_rect(uint8_t *raw, struct colorval *color,
> > +	  int width, int height, int stride)
> > +{
> > +	int y = 0;
> > +	for (y = 0; y < height; ++y) {
> > +		int x = 0;
> > +		uint8_t *ptr = raw + (y * stride);
> > +		for (x = 0; x < width; ++x) {
> > +			memcpy(ptr, &color->raw[0], color->used);
> > +			ptr += color->used;
> > +		}
> > +	}
> > +}
> > +
> > +static void
> > +blend(struct colorval *dest,
> > +      struct colorval const *val1, struct colorval const *val2,
> > +      double factor)
> > +{
> > +	dest->r = (val1->r * (1.0 - factor)) + (val2->r * factor);
> > +	dest->g = (val1->g * (1.0 - factor)) + (val2->g * factor);
> > +	dest->b = (val1->b * (1.0 - factor)) + (val2->b * factor);
> > +	dest->a = (val1->a * (1.0 - factor)) + (val2->a * factor);
> > +	populate_color(dest, val1->form);
> > +}
> > +
> > +static void
> > +hgrad_rect(uint8_t *buf,
> > +	   struct colorval const *val1, struct colorval const *val2,
> > +	   int width, int height, int stride)
> > +{
> > +	int x = 0;
> > +	int y = 0;
> > +	int pw = val1->used;
> > +	struct colorval color = {.a=1.0};
> > +	double dwidth = (double)width;
> > +	for (x = 0; x < width; ++x) {
> > +		uint8_t *ptr = buf + x * pw;
> > +		blend(&color, val1, val2, x / dwidth);
> > +		for (y = 0; y < height; ++y) {
> > +			memcpy(ptr, &color.raw[0], color.used);
> > +			ptr += stride;
> > +		}
> > +	}
> > +}
> > +
> > +/*
> > + * Approximates common color bars with black at 16 and white at 235.
> > + */
> > +static const double lvblk = (16 + 0.00 * (235 - 16)) / 255.0;
> > +static const double lvwht = (16 + 1.00 * (235 - 16)) / 255.0;
> > +static const double lvupr = (16 + 0.75 * (235 - 16)) / 255.0;
> > +static const double lvmid = (16 + 0.40 * (235 - 16)) / 255.0;
> > +static const double lvb15 = (16 + 0.15 * (235 - 16)) / 255.0;
> > +static const double lvb_2 = (16 - 0.02 * (235 - 16)) / 255.0;
> > +static const double lvb02 = (16 + 0.02 * (235 - 16)) / 255.0;
> > +static const double lvb04 = (16 + 0.04 * (235 - 16)) / 255.0;
> > +
> > +/**
> > + * Draws a simple color-bars test pattern to the specified buffer.
> > + */
> > +void
> > +rd_draw_bars(uint8_t *buf, enum wl_shm_format form, int stride,
> > +	     int width, int height)
> > +{
> > +	size_t i;
> > +	int y0, y1, y2;
> > +	int h1, h2;
> > +	int w1, w2, w3, w4, hw;
> > +	int xr;
> > +	int pw;
> > +	int xoff;
> > +	double bar_span;
> > +	size_t bar_count;
> > +	struct colorval bars[] = {
> > +		{.r=lvmid, .g=lvmid, .b=lvmid, .a=1.0},
> > +		{.r=lvupr, .g=lvupr, .b=lvupr, .a=1.0},
> > +		{.r=lvupr, .g=lvupr, .b=lvblk, .a=1.0},
> > +		{.r=lvblk, .g=lvupr, .b=lvupr, .a=1.0},
> > +		{.r=lvblk, .g=lvupr, .b=lvblk, .a=1.0},
> > +		{.r=lvupr, .g=lvblk, .b=lvupr, .a=1.0},
> > +		{.r=lvupr, .g=lvblk, .b=lvblk, .a=1.0},
> > +		{.r=lvblk, .g=lvblk, .b=lvupr, .a=1.0},
> > +		{.r=lvmid, .g=lvmid, .b=lvmid, .a=1.0},
> > +		{.r=lvb15, .g=lvb15, .b=lvb15, .a=1.0},
> > +		{.r=lvwht, .g=lvwht, .b=lvwht, .a=1.0},
> > +		{.r=lvblk, .g=lvblk, .b=lvblk, .a=1.0},
> > +		{.r=lvb_2, .g=lvb_2, .b=lvb_2, .a=1.0},
> > +		{.r=lvb02, .g=lvb02, .b=lvb02, .a=1.0},
> > +		{.r=lvb04, .g=lvb04, .b=lvb04, .a=1.0},
> > +	};
> > +
> > +	if (!buf)
> > +		return;
> > +
> > +	for (i = 0; i < ARRAY_LENGTH(bars); ++i)
> > +		populate_color(&bars[i], form);
> > +
> > +	pw = bars[0].used;
> > +	y0 = ((height * 1.75) / 3);
> > +	y1 = ((height * 2) / 3);
> > +	y2 = ((height * 3) / 4);
> > +	h1 = y1 - y0;
> > +	h2 = height - y2;
> > +	w1 = width / 8;
> > +	w2 = (width * 0.75) / 7;
> > +	xr = pw * ((width * 7) / 8);
> > +
> > +	/* left gray: */
> > +	fill_rect(buf, &bars[0], w1, y0, stride);
> > +	fill_rect(buf + (y0 * stride), &bars[3], w1, h1, stride);
> > +	fill_rect(buf + (y1 * stride), &bars[2], w1, h1, stride);
> > +	fill_rect(buf + (y2 * stride), &bars[9], w1, h2, stride);
> > +
> > +	/* right gray */
> > +	fill_rect(buf + xr, &bars[0], w1, y0, stride);
> > +	fill_rect(buf + xr + (y0 * stride), &bars[7], w1, h1, stride);
> > +	fill_rect(buf + xr + (y1 * stride), &bars[6], w1, h1, stride);
> > +	fill_rect(buf + xr + (y2 * stride), &bars[9], w1, h2, stride);
> > +
> > +	xoff = w1 * pw;
> > +	fill_rect(buf + xoff + (y0 * stride), &bars[10], w2, h1, stride);
> > +	fill_rect(buf + xoff + (y1 * stride), &bars[11], w2, h1, stride);
> > +
> > +	bar_span = (width * 0.75);
> > +	bar_count = 7;
> > +	for (i = 0; i < bar_count; ++i)
> > +	{
> > +		int ww = (bar_span * (i + 1)) / bar_count;
> > +		xoff = (bar_span * i) / bar_count;
> > +		ww -= xoff;
> > +		xoff += w1;
> > +		fill_rect(buf + xoff * pw, &bars[i + 1], ww, y0, stride);
> > +	}
> > +
> > +
> > +	hw = width - (w1 + w1 + w2);
> > +	/* Gray horiz: */
> > +	xoff = w1 + w2;
> > +	fill_rect(buf + xoff * pw + (y0 * stride), &bars[1], hw, h1, stride);
> > +	hgrad_rect(buf + xoff * pw + (y1 * stride), &bars[11], &bars[10],
> > +		   hw, h1, stride);
> > +
> > +	/* note - horizontal positions of lower bars are not simple values */
> > +
> > +	xoff = (width * 0.85625) / 3;
> > +	w3 = (width * 0.64375) / 3;
> > +	fill_rect(buf + w1 * pw + (y2 * stride), &bars[11], width - (w1 * 2),
> > +		  h2, stride);
> > +	fill_rect(buf + xoff * pw + (y2 * stride), &bars[10], w3,
> > +		  h2, stride);
> > +
> > +	w4 = (width * 0.10625) / 3;
> > +	xoff = (width * 1.765625) / 3;
> > +	fill_rect(buf + xoff * pw + (y2 * stride), &bars[12], w4, h2, stride);
> > +
> > +	xoff = (width * 1.98125) / 3;
> > +	fill_rect(buf + xoff * pw + (y2 * stride), &bars[13], w4, h2, stride);
> > +
> > +	xoff = (width * 2.196875) / 3;
> > +	fill_rect(buf + xoff * pw + (y2 * stride), &bars[14], w4, h2, stride);
> > +}
> > +
> > +/**
> > + * Draws a simple cross-hairs image suitable for a cursor to the specified
> > + * buffer.
> > + */
> > +void
> > +rd_draw_crosshairs(uint8_t *buf, enum wl_shm_format form, int stride,
> > +		   int width, int height)
> > +{
> > +	int pw;
> > +	int x = (width + 1) / 2;
> > +	int y = (height + 1) / 2;
> > +	struct colorval fg = {.r=0, .g=0, .b=0, .a=0.5};
> > +	struct colorval bg = {.r=0.5, .g=0.5, .b=0.5, .a=0.5};
> > +
> > +	if (!buf)
> > +		return;
> > +
> > +	populate_color(&fg, form);
> > +	populate_color(&bg, form);
> > +	pw = fg.used;
> > +
> > +	fill_rect(buf + ((y - 1) * stride), &bg, width, 3, stride);
> > +	fill_rect(buf + (y * stride), &fg, width, 1, stride);
> > +
> > +	fill_rect(buf + ((y - 2) * stride), &bg, width / 3, 5, stride);
> > +	fill_rect(buf + ((y - 1) * stride), &fg, width / 3, 3, stride);
> > +	fill_rect(buf + ((y - 2) * stride + (width * 2 / 3) * pw), &bg,
> > +		  width / 3, 5, stride);
> > +	fill_rect(buf + ((y - 1) * stride + (width * 2 / 3) * pw), &fg,
> > +		  width / 3, 3, stride);
> > +
> > +
> > +	fill_rect(buf + (x - 1) * pw, &bg, 3, height, stride);
> > +	fill_rect(buf + x * pw, &fg, 1, height, stride);
> > +
> > +	fill_rect(buf + (x - 2) * pw, &bg, 5, height / 3, stride);
> > +	fill_rect(buf + (x - 1) * pw, &fg, 3, height / 3, stride);
> > +
> > +	fill_rect(buf + (x - 2) * pw + ((height * 2 / 3) * stride), &bg,
> > +		  5, height / 3, stride);
> > +	fill_rect(buf + (x - 1) * pw + ((height * 2 / 3) * stride), &fg,
> > +		  3, height / 3, stride);
> > +}
> 
> 
> Ookay...
> 
> It might not seem much now to hand-code all that, but in the future you
> may want more, and it will become unmaintainable if it isn't already. I
> see three better alternatives:
> - Pixman
> - Cairo
> - a ready-made image file
> or any combination thereof.
> 
> 
> > diff --git a/tools/waycheck/rough_draw.h b/tools/waycheck/rough_draw.h
> > new file mode 100644
> > index 0000000..18e05b3
> > --- /dev/null
> > +++ b/tools/waycheck/rough_draw.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * Copyright © 2015 Samsung Electronics Co., Ltd
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#ifndef ROUGH_DRAW_H
> > +#define ROUGH_DRAW_H
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include <stdint.h>
> > +
> > +#include <wayland-client-protocol.h>
> > +
> > +void
> > +rd_draw_bars(uint8_t *buf, enum wl_shm_format form, int stride,
> > +	     int width, int height);
> > +
> > +void
> > +rd_draw_crosshairs(uint8_t *buf, enum wl_shm_format form, int stride,
> > +		   int width, int height);
> > +
> > +#ifdef __cplusplus
> > +} /* extern "C" */
> > +#endif
> > +
> > +#endif /* ROUGH_DRAW_H */
> > diff --git a/tools/waycheck/waycheck.c b/tools/waycheck/waycheck.c
> > new file mode 100644
> > index 0000000..5572b77
> > --- /dev/null
> > +++ b/tools/waycheck/waycheck.c
> > @@ -0,0 +1,401 @@
> > +/*
> > + * Copyright © 2015 Samsung Electronics Co., Ltd
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#include "config.h"
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <unistd.h>
> > +#include <stdbool.h>
> > +
> > +#include <sys/types.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +
> > +#include <sys/mman.h>
> > +
> > +#include <linux/input.h>
> > +#include <wayland-client.h>
> > +
> > +
> > +#include "os-compatibility.h"
> > +#include "rough_draw.h"
> > +#include "shared/config-parser.h"
> > +#include "shared/helpers.h"
> > +#include "shared/zalloc.h"
> > +#include "wtst_fixtures.h"
> > +#include "zunitc/zunitc.h"
> > +
> > +#define ARGB32_SIZE 4
> > +
> > +static const uint32_t PIXEL_FORMAT = WL_SHM_FORMAT_ARGB8888;
> > +
> > +static void *
> > +setup_test_config(void *data)
> > +{
> > +	struct wtst_ctx *ctx = wtst_ctx_create(NULL);
> > +	ZUC_ASSERTG_NOT_NULL(ctx, out);
> > +
> > +out:
> > +	return ctx;
> > +}
> > +
> > +static void
> > +cleanup_test_config(void *data)
> > +{
> > +	struct wtst_ctx *ctx = data;
> > +	ZUC_ASSERT_NOT_NULL(ctx);
> > +	wtst_ctx_destroy(ctx);
> > +}
> > +
> > +static struct zuc_fixture base_test_f = {
> > +	.set_up = setup_test_config,
> > +	.tear_down = cleanup_test_config
> > +};
> > +
> > +/**
> > + * Baseline test to see if a connection can be made to the Wayland server.
> > + */
> > +ZUC_TEST(base_test, can_connect_display)
> > +{
> > +	struct wl_display *display = wl_display_connect(NULL);
> > +
> > +	ZUC_ASSERT_NOT_NULL(display);
> > +
> > +	wl_display_disconnect(display);
> > +}
> > +
> > +/* A very simple case that only exercises the test fixture. */
> > +ZUC_TEST(base_test, hello_wayland)
> > +{
> > +	struct wtst_ctx *ctx = wtst_ctx_create(NULL);
> > +	ZUC_ASSERT_NOT_NULL(ctx);
> > +
> > +	wtst_ctx_destroy(ctx);
> > +}
> > +
> > +ZUC_TEST_F(base_test_f, advertises_required)
> > +{
> > +	int num_required = 0;
> > +	int i = 0;
> > +	char const *required[] = {
> > +		"wl_compositor",
> > +		"wl_data_device_manager",
> > +		"wl_output",
> > +		"wl_seat",
> > +		"wl_shell",
> > +		"wl_shm",
> > +		"wl_subcompositor",
> > +	};
> > +	struct wtst_ctx *ctx = data;
> > +
> > +	num_required = ARRAY_LENGTH(required);
> > +	for (i = 0; i < num_required; i++) {
> > +		ZUC_TRACEPOINT("Check %s\n", required[i]);
> > +		ZUC_ASSERT_TRUE(wtst_is_global_advertised(ctx, required[i]));
> 
> I actually had to go and read the docs on ZUC_ASSERT_TRUE() to recall
> how it works. That's ok, but what I found confusing is that the doc
> says "Verifies that the specified expression is true, marks the test as
> failed and terminates the test if it is not." However, here "terminate"
> actually means return, not abort() or such.
> 
> I think the ZUC documentation should clearly say the macro does a
> return on failure.
> 
> > +	}
> > +}
> > +
> > +typedef void (*button_cb)(uint32_t serial, uint32_t time,
> > +			  uint32_t button, uint32_t state);
> > +
> > +struct surface_extras
> > +{
> 
> On struct definitions the opening braces belongs to the previous line.
> 
> > +	void (*button_cb)(uint32_t serial, uint32_t time,
> > +			  uint32_t button, uint32_t state);
> 
> Should you rather use the typedef'd type?
> 
> > +
> > +	uint8_t *membuf;
> > +	struct wl_buffer *buffer;
> > +	struct wl_surface *cursor;
> > +	int32_t hit_x;
> > +	int32_t hit_y;
> > +};
> > +
> > +static void
> > +setup_surface_extras(struct wtst_surface *wsurf)
> > +{
> > +	ZUC_ASSERT_NOT_NULL(wsurf);
> > +	ZUC_ASSERT_NULL(wsurf->data);
> > +	struct surface_extras *extras = zalloc(sizeof(*extras));
> 
> Declarations should be in the beginning of a function/block.
> 
> > +	ZUC_ASSERT_NOT_NULL(extras);
> > +	wsurf->data = extras;
> > +}
> > +
> > +/**
> > + * @todo refactor to be more generically helpful then move this along with
> > + * the corresponding helper functions to fixtures lib
> > + */
> > +struct wtst_shm_pool {
> > +	struct wl_shm_pool *pool;
> > +
> > +	int fd;
> > +	uint8_t *membuf;
> > +	size_t flen;
> > +	size_t used;
> > +};
> > +
> > +/* Simple testing helpers: */
> > +static uint8_t *
> > +wtst_shm_pool_get_membuf(struct wtst_shm_pool *pool)
> > +{
> > +	ZUC_ASSERTG_NOT_NULL(pool, err);
> > +	return pool->membuf;
> > +err:
> > +	return NULL;
> > +}
> > +
> > +static struct wl_shm_pool *
> > +wtst_shm_pool_get_pool(struct wtst_shm_pool *pool)
> > +{
> > +	ZUC_ASSERTG_NOT_NULL(pool, err);
> > +	return pool->pool;
> > +err:
> > +	return NULL;
> > +}
> > +
> > +static void
> > +wtst_shm_pool_consume(struct wtst_shm_pool *pool, size_t mem_used)
> > +{
> > +	ZUC_ASSERT_NOT_NULL(pool);
> > +	pool->used += mem_used;
> > +}
> > +
> > +static struct wtst_shm_pool *
> > +wtst_create_mempool(struct wl_shm *shm, int fd, size_t flen)
> > +{
> > +	struct wtst_shm_pool *wtst_pool = zalloc(sizeof(struct wtst_shm_pool));
> > +	ZUC_ASSERTG_NOT_NULL(wtst_pool, err);
> > +
> > +	wtst_pool->fd = fd;
> > +	wtst_pool->flen = flen;
> > +	wtst_pool->membuf = mmap(NULL, flen, PROT_READ | PROT_WRITE,
> > +				 MAP_SHARED, fd, 0);
> > +	ZUC_ASSERTG_TRUE(wtst_pool->membuf != MAP_FAILED, err_free);
> > +
> > +	wtst_pool->pool = wl_shm_create_pool(shm, fd, flen);
> > +	ZUC_ASSERTG_NOT_NULL(wtst_pool->pool, err_unmap);
> > +
> > +	return wtst_pool;
> > +err_unmap:
> > +	munmap(wtst_pool->membuf, flen);
> > +err_free:
> > +	free(wtst_pool);
> > +err:
> > +	return NULL;
> > +}
> > +
> > +static void
> > +setup_cursor(struct wtst_shm_pool *pool,
> > +	     unsigned int width, unsigned int height,
> > +	     int32_t hit_x, int32_t hit_y,
> > +	     struct wl_compositor *compositor,
> > +	     struct surface_extras *extras)
> > +{
> > +	int32_t stride = width * ARGB32_SIZE;
> > +	ZUC_ASSERT_NOT_NULL(extras);
> > +	extras->hit_x = hit_x;
> > +	extras->hit_y = hit_y;
> > +
> > +	extras->cursor = wl_compositor_create_surface(compositor);
> > +	ZUC_ASSERT_NOT_NULL(extras->cursor);
> > +
> > +	extras->membuf = pool->membuf;
> > +	extras->membuf += pool->used / sizeof(*extras->membuf);
> > +	rd_draw_crosshairs(extras->membuf, WL_SHM_FORMAT_ARGB8888,
> > +			   width * ARGB32_SIZE,
> > +			   width, height);
> > +
> > +	extras->buffer = wl_shm_pool_create_buffer(pool->pool,
> > +						   pool->used,
> > +						   width,
> > +						   height,
> > +						   stride,
> > +						   PIXEL_FORMAT);
> > +	pool->used += width * height * ARGB32_SIZE;
> > +	ZUC_ASSERT_NOT_NULL(extras->buffer);
> > +
> > +	rd_draw_crosshairs(extras->membuf, WL_SHM_FORMAT_ARGB8888,
> > +			   width * ARGB32_SIZE,
> > +			   width, height);
> 
> Draw twice?
> 
> > +}
> > +
> > +static void
> > +set_button_cb(struct wtst_shell_surface *shell_surface, button_cb callback)
> > +{
> > +	struct wtst_surface *wsurf = NULL;
> > +	ZUC_ASSERT_NOT_NULL(shell_surface);
> > +	ZUC_ASSERT_NOT_NULL(shell_surface->surface);
> > +	ZUC_ASSERT_NOT_NULL(shell_surface->surface->wl_surface);
> > +
> > +	wsurf = shell_surface->surface;
> > +	ZUC_ASSERT_NOT_NULL(wsurf);
> > +	ZUC_ASSERT_NOT_NULL(wsurf->data);
> > +
> > +	((struct surface_extras *)wsurf->data)->button_cb = callback;
> > +}
> > +
> > +static bool keep_alive = true;
> > +
> > +static void
> > +handle_button(uint32_t serial, uint32_t time,
> > +	      uint32_t button, uint32_t state)
> > +{
> > +	keep_alive = false;
> > +}
> > +
> > +ZUC_TEST(base_test, DISABLED_sleeper)
> > +{
> > +/* Simple disabled test */
> > +}
> 
> Remove that?
> 
> > +
> > +static void
> > +example_test_enter(void *data,
> > +		   struct wl_pointer *wl_pointer,
> > +		   uint32_t serial, struct wl_surface *surface,
> > +		   wl_fixed_t surface_x, wl_fixed_t surface_y)
> > +{
> > +	struct wtst_pointer *pointer = data;
> > +	if (pointer && pointer->focus && pointer->focus->data) {
> > +		struct surface_extras *extras = pointer->focus->data;
> > +		wl_surface_attach(extras->cursor, extras->buffer, 0, 0);
> > +		wl_surface_commit(extras->cursor);
> > +		wl_pointer_set_cursor(pointer->wl_pointer, serial,
> > +				      extras->cursor,
> > +				      extras->hit_x, extras->hit_y);
> > +	}
> > +}
> > +
> > +static void
> > +example_test_leave(void *data,
> > +		   struct wl_pointer *wl_pointer, uint32_t serial,
> > +		   struct wl_surface *wl_surface)
> > +{
> > +}
> > +
> > +static void
> > +example_test_motion(void *data,
> > +		    struct wl_pointer *wl_pointer, uint32_t time,
> > +		    wl_fixed_t surface_x, wl_fixed_t surface_y)
> > +{
> > +}
> > +
> > +static void
> > +example_test_button(void *data,
> > +		    struct wl_pointer *wl_pointer, uint32_t serial,
> > +		    uint32_t time, uint32_t button, uint32_t state)
> > +{
> > +	struct wtst_pointer *pointer = data;
> > +	if (pointer && pointer->focus && pointer->focus->data) {
> > +		struct surface_extras *extras = pointer->focus->data;
> > +		if (extras->button_cb)
> > +			extras->button_cb(serial, time, button, state);
> > +	}
> > +}
> > +
> > +static void
> > +example_test_axis(void *data,
> > +		  struct wl_pointer *wl_pointer, uint32_t time,
> > +		  uint32_t axis, wl_fixed_t value)
> > +{
> > +}
> > +
> > +static const struct wl_pointer_listener pointer_listener = {
> > +	.enter = example_test_enter,
> > +	.leave = example_test_leave,
> > +	.motion = example_test_motion,
> > +	.button = example_test_button,
> > +	.axis = example_test_axis
> > +};
> > +
> > +ZUC_TEST_F(base_test_f, simple_setup)
> > +{
> > +	struct wtst_ctx *ctx = data;
> 
> Uuh, magic plain-named variable 'data'...
> 
> > +	size_t needed = 0;
> > +	int anon = 0;
> > +	const int surf_width = 355;
> > +	const int surf_height = 200;
> > +	const int cursor_size = 45;
> > +	const int mid_hit = cursor_size / 2;
> > +	struct wtst_shm_pool *pool = NULL;
> > +	struct wl_buffer *buffer = NULL;
> > +	struct wtst_shell_surface *sh_surface = NULL;
> > +
> > +	keep_alive = false; /* Don't run the main loop for normal testing */
> > +
> > +	wtst_set_dump_pointer_events(WTST_DBG_POINTER_ALL
> > +				     & ~WTST_DBG_POINTER_MOTION);
> > +
> > +	needed = ARGB32_SIZE * surf_width * surf_height;
> > +	needed += ARGB32_SIZE * cursor_size * cursor_size;
> > +
> > +	anon = os_create_anonymous_file(needed);
> > +	buffer = NULL;
> > +	pool = wtst_create_mempool(ctx->shm, anon, needed);
> > +	if (!pool) {
> > +		close(anon);
> > +		ZUC_ASSERT_NOT_NULL(pool);
> > +	} else {
> > +		uint8_t *mem = wtst_shm_pool_get_membuf(pool);
> > +		rd_draw_bars(mem, WL_SHM_FORMAT_ARGB8888,
> > +			     surf_width * ARGB32_SIZE,
> > +			     surf_width, surf_height);
> > +
> > +		struct wl_shm_pool *shpool =
> > +			wtst_shm_pool_get_pool(pool);
> 
> Mixed code and decls.
> 
> > +		buffer = wl_shm_pool_create_buffer(shpool,
> > +						 0,
> > +						 surf_width,
> > +						 surf_height,
> > +						 surf_width * ARGB32_SIZE,
> > +						 PIXEL_FORMAT);
> > +		wtst_shm_pool_consume(pool,
> > +				      surf_width * surf_height * ARGB32_SIZE);
> > +		ZUC_ASSERT_NOT_NULL(buffer);
> > +	}
> > +
> > +	sh_surface = wtst_create_shell_surface(ctx->compositor, ctx->shell);
> > +	setup_surface_extras(sh_surface->surface);
> > +
> > +	if (buffer) {
> 
> Cannot get here without a buffer.
> 
> > +		wl_surface_attach(sh_surface->surface->wl_surface, buffer,
> > +				  0, 0);
> > +		wl_surface_commit(sh_surface->surface->wl_surface);
> > +	}
> > +
> > +	setup_cursor(pool, cursor_size, cursor_size, mid_hit, mid_hit,
> > +		     ctx->compositor, sh_surface->surface->data);
> > +
> > +	set_button_cb(sh_surface, handle_button);
> > +	wtst_pointer_add_listener(ctx->input->pointer,
> > +				  &pointer_listener, ctx->input->pointer);
> > +
> > +	while (keep_alive) {
> > +		if (wl_display_dispatch(ctx->display) < 0)
> > +			keep_alive = false;
> > +	}
> > +
> 
> Should probably destroy the shell_surface and wl_surface first, unless
> you are explicitly testing what happens on unusual ordering.
> 
> > +	wl_buffer_destroy(buffer);
> > +}
> > diff --git a/tools/waycheck/waycheck.dox b/tools/waycheck/waycheck.dox
> > new file mode 100644
> > index 0000000..66ae65b
> > --- /dev/null
> > +++ b/tools/waycheck/waycheck.dox
> > @@ -0,0 +1,29 @@
> > +/*
> > + * Copyright © 2015 Samsung Electronics Co., Ltd
> > + *
> > + * Permission to use, copy, modify, distribute, and sell this software and
> > + * its documentation for any purpose is hereby granted without fee, provided
> > + * that the above copyright notice appear in all copies and that both that
> > + * copyright notice and this permission notice appear in supporting
> > + * documentation, and that the name of the copyright holders not be used in
> > + * advertising or publicity pertaining to distribution of the software
> > + * without specific, written prior permission.  The copyright holders make
> > + * no representations about the suitability of this software for any
> > + * purpose.  It is provided "as is" without express or implied warranty.
> > + *
> > + * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS
> > + * SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND
> > + * FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY
> > + * SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER
> > + * RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF
> > + * CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
> > + * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> > +
> > +/**
> > + at page waycheck
> > +
> > +A simple integration/acceptance tool to exercise Wayland compositors.
> > +
> > +Tests use @ref zunitc for their infrastructure, and most include use of a common wtst_ctx test fixture.
> > +*/
> > \ No newline at end of file
> > diff --git a/tools/wayland_fixtures/inc/wtst_fixtures.h b/tools/wayland_fixtures/inc/wtst_fixtures.h
> > new file mode 100644
> > index 0000000..f37c370
> > --- /dev/null
> > +++ b/tools/wayland_fixtures/inc/wtst_fixtures.h
> > @@ -0,0 +1,263 @@
> > +/*
> > + * Copyright © 2015 Samsung Electronics Co., Ltd
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#ifndef WTST_FIXTURES_H
> > +#define WTST_FIXTURES_H
> > +
> > +/**
> > + * @file
> > + * Common helpers for Wayland tests.
> > + */
> > +
> > +#include <stdint.h>
> > +#include <stdbool.h>
> > +
> > +#include <wayland-client.h>
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +/**
> > + * Test fixture that holds the basics for simple Wayland client testing.
> > + *
> > + * @see wtst_ctx_create()
> > + * @see wtst_ctx_destroy()
> > + */
> > +struct wtst_ctx
> > +{
> > +	/** The display the client is connected to. */
> > +	struct wl_display *display;
> > +
> > +	/** The registry the client is connected to. */
> > +	struct wl_registry *registry;
> > +
> > +	/** Main compositor as advertised and updated by the registry. */
> > +	struct wl_compositor *compositor;
> > +
> > +	/**
> > +	 * Main shared memory object as advertised and updated by the
> > +	 * registry.
> > +	 */
> > +	struct wl_shm *shm;
> > +
> > +	/** The seat that is actually used for input events. */
> > +	struct wtst_input *input;
> > +
> > +	/**
> > +	 * Server can have more wl_seats. We need keep them all until we
> > +	 * find the one that we need. After that, the others
> > +	 * will be destroyed, so this list will have the length of 1.
> > +	 * If some day in the future we will need the other seats,
> > +	 * we can just keep them here.
> > +	 */
> > +	struct wl_list inputs;
> > +
> > +	/**
> > +	 * The output used during testing.
> > +	 */
> > +	struct wtst_output *output;
> > +
> > +	/**
> > +	 * The surface used during testing.
> > +	 */
> > +	struct wtst_surface *surface;
> > +
> > +	/**
> > +	 * Flag indicating the presence of DRM.
> > +	 */
> > +	bool has_wl_drm;
> > +
> > +	/** Main shell as advertised and updated by the registry. */
> > +	struct wl_shell *shell;
> > +
> > +	/** Listener registered with the registry. */
> > +	struct wl_registry_listener *reg_listener;
> > +
> > +	/** Context for internal implementation details. */
> > +	struct wtst_ctx_private *private;
> > +};
> > +
> > +/**
> > + * Structure for tracking Wayland registry globals.
> > + */
> > +struct wtst_global {
> > +	uint32_t name;		/**< name of the global object. */
> > +	char *interface;	/**< interface the global object implements. */
> > +	uint32_t version;	/**< given version of the interface
> > +				   implemented by the the global object. */
> > +	struct wl_list link;	/**< list implementation hook. */
> > +};
> > +
> > +/**
> > + * Structure for tracking Wayland input instances.
> 
> More accurately "... wl_seat instances for input".
> 
> > + */
> > +struct wtst_input {
> > +	struct wl_seat *wl_seat;	/**< The target Wayland seat. */
> > +	struct wtst_pointer *pointer;	/**< Main pointer for the seat. */
> > +	struct wtst_keyboard *keyboard; /**< Main keyboard for the seat. */
> > +	struct wtst_touch *touch;	/**< Main touch device for the seat. */
> 
> Strike "Main", there is only one per wl_seat, and each may be an
> aggregate of physical input devices.
> 
> > +	char *seat_name;		/**< Name of the seat. */
> > +	enum wl_seat_capability caps;	/**< Base seat capabilities. */
> 
> Should this be uint32_t?
> I see the old upstream code has the same.
> 
> > +	struct wl_list link;		/**< list implementation hook. */
> > +};
> > +
> > +/**
> > + * Structure for tracking Wayland pointers.
> > + */
> > +struct wtst_pointer {
> > +	struct wl_pointer *wl_pointer;	/**< The target Wayland pointer. */
> > +	struct wtst_surface *focus;	/**< Tracked focused surface. */
> > +	int x;				/**< Tracked x. */
> > +	int y;				/**< Tracked y. */
> 
> Might mention these are coordinates on the surface 'focus'.
> 
> > +	uint32_t button;		/**< Tracked button. */
> > +	uint32_t state;		/**< Tracked state. */
> 
> These comments don't actually help in understanding what these members
> are, and I have no idea.
> 
> Similar comments for all wtst_* struct members below.
> 
> IMHO, if you can't write a helpful explanation now, leave it out.
> 
> Your commit message could explain that these are mostly copied from the
> old code in upstream.
> 
> > +	struct wtst_pointer_private *private; /**< private instance data. */
> > +};
> > +
> > +/**
> > + * Structure for tracking Wayland keyboards.
> > + */
> > +struct wtst_keyboard {
> > +	struct wl_keyboard *wl_keyboard;	/**< The target Wayland
> > +						   keyboard. */
> > +	struct wtst_surface *focus;		/**< Last tracked focus. */
> > +	uint32_t key;				/**< Tracked key. */
> > +	uint32_t state;			/**< Tracked state. */
> > +	uint32_t mods_depressed;		/**< Tracked mods. */
> > +	uint32_t mods_latched;			/**< Tracked mods latched. */
> > +	uint32_t mods_locked;			/**< Tracked mods locked. */
> > +	uint32_t group;			/**< Tracked group. */
> > +	struct {
> > +		int rate;			/**< Repeat rate. */
> > +		int delay;			/**< Repeat delay. */
> > +	} repeat_info;				/**< Tracked repeat. */
> > +};
> > +
> > +/**
> > + * Structure for tracking Wayland touch instances.
> > + */
> > +struct wtst_touch {
> > +	struct wl_touch *wl_touch;	/**< The target Wayland touch. */
> > +	int down_x;			/**< Tracked down x position. */
> > +	int down_y;			/**< Tracked down y position. */
> > +	int x;				/**< Tracked x position. */
> > +	int y;				/**< Tracked y potision. */
> > +	int id;			/**< Tracked ID. */
> > +	int up_id;			/**< Id of last wl_touch.up event */
> > +	int frame_no;			/**< Count of frame events. */
> > +	int cancel_no;			/**< Count of cancel events. */
> > +};
> > +
> > +/**
> > + * Structure for tracking output instances.
> > + */
> > +struct wtst_output {
> > +	struct wl_output *wl_output;	/**< The target Wayland output. */
> > +	int x;				/**< Tracked x. */
> > +	int y;				/**< Tracked y. */
> > +	int width;			/**< Tracked width. */
> > +	int height;			/**< Tracked height. */
> > +	int scale;			/**< Tracked scale. */
> > +	bool initialized;		/**< Tracked initialized flag. */
> > +};
> > +
> > +/**
> > + * Strcuture for tracking Wayland surfaces.
> > + */
> > +struct wtst_surface {
> > +	struct wl_surface *wl_surface;	/**< The target Wayland surface. */
> > +	struct wl_buffer *wl_buffer;	/**< Buffer created for working with
> > +					   this surface. */
> > +	struct wtst_output *output;	/**< Tracked output for this surface. */
> > +	int x;				/**< Tracked x. */
> > +	int y;				/**< Tracked y. */
> > +	int width;			/**< Tracked width. */
> > +	int height;			/**< Tracked height. */
> > +	void *data;			/**< private instance data. */
> > +};
> > +
> > +/**
> > + * Structure for tracking Wayland shell surfaces.
> > + */
> > +struct wtst_shell_surface {
> > +	struct wl_shell_surface *wl_shell_surface;	/**< The target Wayland
> > +							   shell surface. */
> > +	struct wtst_surface *surface; /**< surface that this is wrapping. */
> > +};
> > +
> > +/**
> > + * Enum to control what debugging information is dumped during tests.
> > + */
> > +enum wtst_dbgevents_pointer {
> > +	WTST_DBG_POINTER_NONE = 0x00,
> > +	WTST_DBG_POINTER_ENTER = 0x01,
> > +	WTST_DBG_POINTER_LEAVE = 0x02,
> > +	WTST_DBG_POINTER_MOTION = 0x04,
> > +	WTST_DBG_POINTER_BUTTON = 0x08,
> > +	WTST_DBG_POINTER_AXIS = 0x10,
> > +	WTST_DBG_POINTER_ALL = 0x1f
> > +};
> > +
> > +struct wtst_ctx *
> > +wtst_ctx_create(const char *display);
> > +
> > +struct wtst_ctx *
> > +wtst_ctx_create_with_surface(const char *display, int width, int height);
> > +
> > +struct wtst_ctx *
> > +wtst_ctx_create_chained(const char *display,
> > +			struct wl_registry_listener *reg_listener,
> > +			void *reg_data);
> > +
> > +struct wtst_ctx *
> > +wtst_ctx_create_with_surface_chained(const char *display,
> > +				     int width, int height,
> > +				     struct wl_registry_listener *reg_listener,
> > +				     void *reg_data);
> > +
> > +void
> > +wtst_ctx_destroy(struct wtst_ctx *ctx);
> > +
> > +int
> > +wtst_is_global_advertised(struct wtst_ctx *ctx, char const *interface);
> > +
> > +int
> > +wtst_pointer_add_listener(struct wtst_pointer *pointer,
> > +			  const struct wl_pointer_listener *listener,
> > +			  void *data);
> > +
> > +void
> > +wtst_set_dump_pointer_events(enum wtst_dbgevents_pointer mask);
> > +
> > +struct wtst_shell_surface *
> > +wtst_create_shell_surface(struct wl_compositor *compositor,
> > +			  struct wl_shell *shell);
> > +
> > +#ifdef __cplusplus
> > +} /* extern "C" */
> > +#endif
> > +
> > +#endif /* WTST_FIXTURES_H */
> > diff --git a/tools/wayland_fixtures/src/wtst_fixtures.c b/tools/wayland_fixtures/src/wtst_fixtures.c
> > new file mode 100644
> > index 0000000..7041f71
> > --- /dev/null
> > +++ b/tools/wayland_fixtures/src/wtst_fixtures.c
> > @@ -0,0 +1,1178 @@
> > +/*
> > + * Copyright © 2015 Samsung Electronics Co., Ltd
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#include "config.h"
> > +
> > +#include "wtst_fixtures.h"
> > +
> > +#include <stdint.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +#include <sys/mman.h>
> > +#include <unistd.h>
> > +
> > +#include "shared/helpers.h"
> > +#include "shared/os-compatibility.h"
> > +#include "shared/zalloc.h"
> > +#include "zunitc/zunitc.h"
> > +
> > +/**
> > + * Private data for implementation of test fixture context.
> > + */
> > +struct wtst_ctx_private {
> > +
> > +	/** List of all known global advertised by the registry. */
> > +	struct wl_list global_list;
> > +
> > +	/** List of formats supported by the wl_shm. */
> > +	struct wl_list format_list;
> > +
> > +	/** Chained registry listener to invoke. */
> > +	struct wl_registry_listener *chained;
> > +
> > +	/** Data to pass back to chained registry listener. */
> > +	void *chained_data;
> > +
> > +	/** Current main compositor name advertised by the registry. */
> > +	uint32_t compositor_name;
> > +
> > +	/** Current shared memory object name advertised by the registry. */
> > +	uint32_t shm_name;
> > +
> > +	/** Current shell name advertised by the registry. */
> > +	uint32_t shell_name;
> 
> Took a moment to realize these refer to wl_compositor, wl_shm, and
> wl_shell/xdg_shell. You also need to store the version of each. Perhaps
> it's easier to just look up from the global_list as needed? You only do
> it when you bind, anyway.
> 
> > +};
> > +
> > +/**
> > + * Find the test-seat and set it in client while destroying other inputs.
> > + * @todo probably need to defer input selection to higher layers and not
> > + * hardcode to "test-seat" here.
> > + */
> > +static void
> > +wtst_ctx_set_input(struct wtst_ctx *ctx);
> > +
> > +static void
> > +input_update_devices(struct wtst_input *input);
> > +
> > +static enum wtst_dbgevents_pointer g_pointer_dbg_mask = WTST_DBG_POINTER_ALL;
> > +
> > +/**
> > + * Sets what events will be dumped during testing.
> > + *
> > + * @param mask a mask of which events to dump out.
> > + */
> > +void
> > +wtst_set_dump_pointer_events(enum wtst_dbgevents_pointer mask)
> > +{
> > +	g_pointer_dbg_mask = mask;
> > +}
> > +
> > +static void
> > +input_destroy(struct wtst_input *inp)
> > +{
> > +	wl_list_remove(&inp->link);
> > +	wl_seat_destroy(inp->wl_seat);
> 
> Check wl_seat version, and use wl_seat_release() instead when
> appropriate.
> 
> > +	free(inp);
> > +}
> > +
> > +/**
> > + * Private data for implementation of pointer tracking.
> > + */
> > +struct
> > +wtst_pointer_private
> > +{
> > +	/** opaque data. */
> > +	void *data;
> > +
> > +	/** connected listener. */
> > +	const struct wl_pointer_listener *listener;
> > +
> > +	/** list implementation hook. */
> > +	struct wl_list link;
> 
> From the code below, it looks like this should be a list, not link.
> Hmm, but you use the same struct as both owning the list and the items
> in the list.
> 
> Seem like there should be two different structs: the pointer private,
> and the item in the list.
> 
> It would be good to document for both list and link members which
> struct is the list item and, if unique, the list head owner owner.
> 
> > +};
> > +
> > +static void
> > +output_handle_geometry(void *data,
> > +		       struct wl_output *wl_output,
> > +		       int x, int y,
> > +		       int physical_width, int physical_height,
> > +		       int subpixel,
> > +		       const char *make, const char *model,
> > +		       int32_t transform)
> > +{
> > +	struct wtst_output *output = data;
> > +
> > +	output->x = x;
> > +	output->y = y;
> > +}
> > +
> > +static void
> > +output_handle_mode(void *data,
> > +		   struct wl_output *wl_output,
> > +		   uint32_t flags,
> > +		   int width, int height,
> > +		   int refresh)
> > +{
> > +	struct wtst_output *output = data;
> > +
> > +	if (flags & WL_OUTPUT_MODE_CURRENT) {
> > +		output->width = width;
> > +		output->height = height;
> > +	}
> > +}
> > +
> > +static void
> > +output_handle_scale(void *data,
> > +		    struct wl_output *wl_output,
> > +		    int scale)
> > +{
> > +	struct wtst_output *output = data;
> > +
> > +	output->scale = scale;
> > +}
> > +
> > +static void
> > +output_handle_done(void *data,
> > +		   struct wl_output *wl_output)
> > +{
> > +	struct wtst_output *output = data;
> > +
> > +	output->initialized = true;
> > +}
> > +
> > +static void
> > +seat_handle_capabilities(void *data, struct wl_seat *seat,
> > +			 enum wl_seat_capability caps)
> > +{
> > +	struct wtst_input *input = data;
> > +
> > +	input->caps = caps;
> > +
> > +	/* we will create/update the devices only with the right (test) seat.
> > +	 * If we haven't discovered which seat is the test seat, just
> > +	 * store capabilities and bail out */
> > +	if (input->seat_name && strcmp(input->seat_name, "test-seat") == 0)
> > +		input_update_devices(input);
> > +
> > +	fprintf(stderr, "test-client: got seat %p capabilities: %x\n",
> > +		input, caps);
> > +}
> > +
> > +static void
> > +seat_handle_name(void *data, struct wl_seat *seat, const char *name)
> > +{
> > +	struct wtst_input *input = data;
> > +
> > +	input->seat_name = strdup(name);
> > +	ZUC_ASSERT_NOT_NULL(input->seat_name);
> > +
> > +	fprintf(stderr, "test-client: got seat %p name: \'%s\'\n",
> > +		input, name);
> > +}
> > +
> > +static void
> > +shell_surface_ping(void *data,
> > +		   struct wl_shell_surface *shell_surface,
> > +		   uint32_t serial)
> > +{
> > +    wl_shell_surface_pong(shell_surface, serial);
> > +}
> > +
> > +static void
> > +shell_surface_configure(void *data,
> > +			struct wl_shell_surface *shell_surface,
> > +			uint32_t edges,
> > +			int32_t width, int32_t height)
> > +{
> > +}
> > +
> > +static void
> > +pointer_handle_enter(void *data, struct wl_pointer *wl_pointer,
> > +		     uint32_t serial, struct wl_surface *wl_surface,
> > +		     wl_fixed_t x, wl_fixed_t y)
> > +{
> > +	struct wtst_pointer *pointer = data;
> > +	struct wtst_pointer_private *entry = NULL;
> > +	struct wtst_pointer_private *tmp = NULL;
> > +
> > +	pointer->focus = wl_surface_get_user_data(wl_surface);
> > +	pointer->x = wl_fixed_to_int(x);
> > +	pointer->y = wl_fixed_to_int(y);
> > +
> > +	if (g_pointer_dbg_mask & WTST_DBG_POINTER_ENTER)
> > +		fprintf(stderr, "test-client: "
> > +			"got pointer enter %d %d, surface %p\n",
> > +			pointer->x, pointer->y, pointer->focus);
> > +
> > +	wl_list_for_each_safe(entry, tmp, &pointer->private->link, link)
> > +		if (entry->listener && entry->listener->enter)
> > +			entry->listener->enter(entry->data, wl_pointer,
> > +					       serial, wl_surface, x, y);
> > +}
> 
> <chop>
> 
> > +/**
> > + * Item for listing supported memory formats.
> > + */
> > +struct shm_format {
> > +	uint32_t format;	/**< Format type. */
> > +
> > +	struct wl_list link;	/**< list implementation hook. */
> > +};
> > +
> > +static bool
> > +has_shm_format(struct wtst_ctx *ctx, uint32_t format)
> > +{
> > +	bool status = false;
> > +	struct shm_format *format_info = NULL;
> > +
> > +	wl_list_for_each(format_info, &ctx->private->format_list, link)
> > +		status |= (format == format_info->format);
> > +
> > +	return status;
> > +}
> > +
> > +static void
> > +shm_handle_format(void *data, struct wl_shm *wl_shm, uint32_t format)
> > +{
> > +	struct wtst_ctx *ctx = data;
> > +	struct shm_format *shm_format = zalloc(sizeof(*shm_format));
> > +
> > +	ZUC_ASSERT_NOT_NULL(shm_format);
> > +	shm_format->format = format;
> > +	wl_list_insert(&ctx->private->format_list, &shm_format->link);
> > +}
> > +
> > +struct wl_shm_listener shm_listener = {
> > +	shm_handle_format
> > +};
> > +
> > +static const struct wl_shell_surface_listener
> > +shell_surface_listener = {
> > +	.ping = shell_surface_ping,
> > +	.configure = shell_surface_configure,
> 
> Missing popup_done.
> 
> > +};
> > +
> > +static const struct wl_output_listener output_listener = {
> > +	output_handle_geometry,
> > +	output_handle_mode,
> > +	output_handle_done,
> > +	output_handle_scale,
> > +};
> > +
> > +static const struct wl_seat_listener seat_listener = {
> > +	seat_handle_capabilities,
> > +	seat_handle_name,
> > +};
> > +
> > +static const struct wl_pointer_listener pointer_listener = {
> > +	pointer_handle_enter,
> > +	pointer_handle_leave,
> > +	pointer_handle_motion,
> > +	pointer_handle_button,
> > +	pointer_handle_axis,
> > +};
> > +
> > +static const struct wl_keyboard_listener keyboard_listener = {
> > +	keyboard_handle_keymap,
> > +	keyboard_handle_enter,
> > +	keyboard_handle_leave,
> > +	keyboard_handle_key,
> > +	keyboard_handle_modifiers,
> > +	keyboard_handle_repeat_info
> > +};
> > +
> > +static const struct wl_touch_listener touch_listener = {
> > +	touch_handle_down,
> > +	touch_handle_up,
> > +	touch_handle_motion,
> > +	touch_handle_frame,
> > +	touch_handle_cancel,
> > +};
> > +
> > +static const struct wl_surface_listener surface_listener = {
> > +	surface_handle_enter,
> > +	surface_handle_leave
> > +};
> > +
> > +static bool is_singleton_interface(const char *interface)
> > +{
> > +	 size_t i = 0;
> > +	const char *known_singletons[] = {
> > +		"wl_compositor",
> > +		"wl_display",
> > +		"wl_compositor",
> > +		"wl_shm",
> > +		"wl_data_device_manager",
> > +	};
> > +
> > +	for (i = 0; i < ARRAY_LENGTH(known_singletons); ++i)
> > +		if (strcmp(known_singletons[i], interface) == 0)
> > +			return true;
> > +
> > +	return false;
> > +}
> > +
> > +static void
> > +add_global_advert(struct wtst_ctx *ctx,
> > +		  uint32_t name,
> > +		  const char *interface,
> > +		  uint32_t version)
> > +{
> > +	if (!ctx || !ctx->private)
> > +		return;
> > +
> > +	/* See if we've got this interface already */
> > +	struct wtst_global *curr = NULL;
> > +	struct wtst_global *tmp = NULL;
> 
> Mixed code and decls.
> 
> > +	wl_list_for_each(tmp, &ctx->private->global_list, link)
> > +		if (strcmp(tmp->interface, interface) == 0) {
> > +			curr = tmp;
> > +			break;
> > +		}
> > +
> > +	if (curr) {
> > +		/* These are not supposed to change. */
> > +		if (is_singleton_interface(interface)) {
> > +			ZUC_FATAL("Singleton advertised more than once.");
> > +
> > +			ZUC_ASSERT_EQ(curr->name, name);
> > +			ZUC_ASSERT_EQ(curr->version, version);
> > +		}
> > +	} else {
> > +		struct wtst_global *glbl = zalloc(sizeof(*glbl));
> > +		ZUC_ASSERT_NOT_NULL(glbl);
> > +		glbl->name = name;
> > +		glbl->interface = strdup(interface);
> > +		glbl->version = version;
> > +		wl_list_insert(&ctx->private->global_list, &glbl->link);
> > +	}
> > +}
> > +
> > +static void
> > +reg_global(void *data,
> > +	   struct wl_registry *registry,
> > +	   uint32_t name,
> > +	   const char *interface,
> > +	   uint32_t version)
> > +{
> > +	struct wtst_ctx *ctx = (struct wtst_ctx *)data;
> > +	struct wtst_input *input = NULL;
> > +
> > +	ZUC_ASSERT_STREQ("wl_compositor", wl_compositor_interface.name);
> > +
> > +	add_global_advert(ctx, name, interface, version);
> > +
> > +	if (strcmp(interface, wl_compositor_interface.name) == 0) {
> > +		ctx->compositor = wl_registry_bind(registry, name,
> > +						   &wl_compositor_interface,
> > +						   version);
> > +		ctx->private->compositor_name = name;
> > +	} else if (strcmp(interface, wl_seat_interface.name) == 0) {
> > +		input = zalloc(sizeof(*input));
> > +		ZUC_ASSERT_NOT_NULL(input);
> > +		input->wl_seat = wl_registry_bind(registry, name,
> > +						  &wl_seat_interface, version);
> > +		wl_seat_add_listener(input->wl_seat, &seat_listener, input);
> > +		wl_list_insert(&ctx->inputs, &input->link);
> > +	} else if (strcmp(interface, wl_shm_interface.name) == 0) {
> > +		ZUC_ASSERT_NULL(ctx->shm);
> > +		ctx->shm = wl_registry_bind(registry, name,
> > +					    &wl_shm_interface, version);
> > +		ctx->private->shm_name = name;
> > +		wl_shm_add_listener(ctx->shm, &shm_listener, ctx);
> > +	} else if (strcmp(interface, wl_output_interface.name) == 0) {
> > +		struct wtst_output *output = zalloc(sizeof(*output));
> > +		ZUC_ASSERT_NOT_NULL(output);
> > +		output->wl_output = wl_registry_bind(registry, name,
> > +						     &wl_output_interface,
> > +						     version);
> > +		wl_output_add_listener(output->wl_output,
> > +				       &output_listener, output);
> > +		ctx->output = output;
> > +	} else if (strcmp(interface, wl_shell_interface.name) == 0) {
> > +		ZUC_ASSERT_NULL(ctx->shell);
> > +		ctx->shell = wl_registry_bind(registry, name,
> > +					      &wl_shell_interface, version);
> > +		ctx->private->shell_name = name;
> 
> The problem with all of the above is that they bind with whatever
> version the compositor advertises. If you happen to run this code
> against a compositor that advertises an interface version this code
> does not yet handle, things will likely break for no reason.
> 
> The version number you pass to wl_registry_bind() should be
> MIN(compositor_advertised, my_supported). You cannot increase
> my_supported without actually writing the code to deal with the
> protocol additions.
> 
> I see the old upstream code has the same mistake, but if this code is
> supposed to be usable against any Wayland compositor, not just Weston,
> then it would be good to bind the right version.
> 
> You might want to print a warning that the test suite is outdated if a
> compositor advertises a version we do not support here.
> 
> > +	} else if (strcmp(interface, "wl_drm") == 0) {
> > +		ctx->has_wl_drm = true;
> > +	}
> > +
> > +	if (ctx->private->chained && ctx->private->chained->global) {
> > +		ctx->private->chained->global(ctx->private->chained_data,
> > +					      registry, name,
> > +					      interface, version);
> > +	}
> > +}
> > +
> > +static void
> > +reg_global_remove(void *data,
> > +		  struct wl_registry *registry,
> > +		  uint32_t name)
> > +{
> > +	struct wtst_ctx *ctx = (struct wtst_ctx *)data;
> > +
> > +	if (name == ctx->private->compositor_name) {
> > +		if (ctx->compositor)
> > +			wl_compositor_destroy(ctx->compositor);
> > +		ctx->compositor = NULL;
> > +		ctx->private->compositor_name = 0;
> > +	} else if (name == ctx->private->shm_name) {
> > +		if (ctx->shm)
> > +			wl_shm_destroy(ctx->shm);
> > +		ctx->shm = NULL;
> > +		ctx->private->shm_name = 0;
> > +	} else if (name == ctx->private->shell_name) {
> > +		if (ctx->shell)
> > +			wl_shell_destroy(ctx->shell);
> > +		ctx->shell = NULL;
> > +		ctx->private->shell_name = 0;
> > +	}
> 
> I think the above is basically dead code. Removal of wl_compositor,
> wl_shm, or wl_shell likely never happens.
> 
> > +
> > +	if (ctx->private->chained && ctx->private->chained->global_remove) {
> > +		ctx->private->chained->global_remove(ctx->private->chained_data,
> > +						     registry, name);
> > +	}
> > +}
> > +
> > +static struct wtst_surface *
> > +wrap_wl_surface(struct wl_surface *wl_surface)
> > +{
> > +	struct wtst_surface *wsurf = NULL;
> > +	ZUC_ASSERTG_NOT_NULL(wl_surface, out);
> > +	wsurf = zalloc(sizeof(*wsurf));
> > +	ZUC_ASSERTG_NOT_NULL(wsurf, out);
> > +	wl_surface_set_user_data(wl_surface, wsurf);
> 
> wl_surface_set_user_data() is overwritten by wl_surface_add_listener().
> 
> > +
> > +	wsurf->wl_surface = wl_surface;
> > +	wl_surface_add_listener(wl_surface, &surface_listener, wsurf);
> > +
> > +out:
> > +	return wsurf;
> > +}
> > +
> > +static struct wl_buffer *
> > +create_shm_buffer(struct wl_shm *shm, int width, int height, void **pixels)
> > +{
> > +	int stride = width * 4;
> > +	int size = stride * height;
> > +	struct wl_shm_pool *pool = NULL;
> > +	struct wl_buffer *buffer = NULL;
> > +	int fd = -1;
> > +	void *data = NULL;
> > +
> > +	if (pixels)
> > +		*pixels = NULL;
> > +
> > +	fd = os_create_anonymous_file(size);
> > +	ZUC_ASSERTG_TRUE(fd >= 0, out);
> > +
> > +	data = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> > +	ZUC_ASSERTG_NE(MAP_FAILED, data, out_close);
> > +
> > +	pool = wl_shm_create_pool(shm, fd, size);
> > +	ZUC_ASSERTG_NOT_NULL(pool, out_close);
> > +	buffer = wl_shm_pool_create_buffer(pool, 0, width, height, stride,
> > +					   WL_SHM_FORMAT_ARGB8888);
> > +	ZUC_ASSERTG_NOT_NULL(buffer, out_free);
> > +
> > +	if (pixels)
> > +		*pixels = data;
> > +
> > +out_free:
> > +	wl_shm_pool_destroy(pool);
> > +out_close:
> > +	close(fd);
> > +out:
> > +	return buffer;
> > +}
> 
> I see waycheck.c also had shm buffer stuff, maybe it should be using
> these exported in a nice API?
> 
> > +
> > +static void
> > +wtst_surface_destroy(struct wtst_surface *surface)
> > +{
> > +	ZUC_ASSERT_NOT_NULL(surface);
> > +	wl_surface_destroy(surface->wl_surface);
> > +	free(surface);
> > +}
> > +
> > +static struct wtst_ctx_private *
> > +wtst_ctx_private_create(void)
> > +{
> > +	struct wtst_ctx_private *private = zalloc(sizeof(*private));
> > +
> > +	ZUC_ASSERTG_NOT_NULL(private, out);
> > +
> > +	wl_list_init(&private->global_list);
> > +	wl_list_init(&private->format_list);
> > +
> > +out:
> > +	return private;
> > +}
> > +
> > +static void
> > +wtst_ctx_private_destroy(struct wtst_ctx_private *private)
> > +{
> > +	if (!private)
> > +		return;
> > +
> > +	if (!wl_list_empty(&private->global_list)) {
> > +		struct wtst_global *entry = NULL;
> > +		struct wtst_global *tmp = NULL;
> > +		wl_list_for_each_reverse_safe(entry, tmp,
> > +					      &private->global_list, link) {
> > +			wl_list_remove(&entry->link);
> > +			free(entry);
> > +		}
> > +	}
> > +
> > +	if (!wl_list_empty(&private->format_list)) {
> > +		struct shm_format *entry = NULL;
> > +		struct shm_format *tmp = NULL;
> > +		wl_list_for_each_reverse_safe(entry, tmp,
> > +					      &private->format_list, link) {
> > +			wl_list_remove(&entry->link);
> > +			free(entry);
> > +		}
> > +	}
> > +
> > +	free(private);
> > +}
> > +
> > +/**
> > + * Creates an instance of the test fixture.
> > + *
> > + * @param display name of the wayland display to connect to or NULL for
> > + * the default.
> > + * @return a pointer to a test fixture upon success, NULL otherwise.
> > + * @see wtst_ctx_destroy()
> > + */
> > +struct wtst_ctx *
> > +wtst_ctx_create(const char *display)
> > +{
> > +	return wtst_ctx_create_chained(display, NULL, NULL);
> > +}
> > +
> > +/**
> > + * Creates an instance of the test fixture with a corresponding surface.
> > + *
> > + * @param display name of the wayland display to connect to or NULL for
> > + * the default.
> > + * @param width the width for the surface.
> > + * @param height the height for the surface.
> > + * @return a pointer to a test fixture upon success, NULL otherwise.
> > + * @see wtst_ctx_destroy()
> > + */
> > +struct wtst_ctx *
> > +wtst_ctx_create_with_surface(const char *display, int width, int height)
> > +{
> > +	struct wtst_ctx *ctx =
> > +		wtst_ctx_create_with_surface_chained(display, width, height,
> > +						     NULL, NULL);
> > +	return ctx;
> > +}
> > +
> > +/**
> > + * Creates an instance of the test fixture that is chained and with a
> > + * corresponding surface.
> > + * Additional registry advertisements will be passed on.
> > + *
> > + * @param display name for the Wayland display to connect to.
> > + * @param width the width for the surface.
> > + * @param height the height for the surface.
> > + * @param reg_listener the registry listener to chain in.
> > + * @param reg_data pointer to pass back to registry listener.
> > + * @return a pointer to a test fixture upon success, NULL otherwise.
> > + * @see wtst_ctx_destroy()
> > + */
> > +struct wtst_ctx *
> > +wtst_ctx_create_with_surface_chained(const char *display,
> > +				     int width, int height,
> > +				     struct wl_registry_listener *reg_listener,
> > +				     void *reg_data)
> > +{
> > +	struct wl_surface *wl_surface = NULL;
> > +	struct wtst_surface *surface = NULL;
> > +	struct wtst_ctx *ctx = wtst_ctx_create_chained(display,
> > +						       reg_listener, reg_data);
> > +	ZUC_ASSERTG_NOT_NULL(ctx, err);
> > +
> > +	wl_surface = wl_compositor_create_surface(ctx->compositor);
> > +	ZUC_ASSERTG_NOT_NULL(wl_surface, err_free);
> > +
> > +	surface = wrap_wl_surface(wl_surface);
> > +	ZUC_ASSERTG_NOT_NULL(surface, err_destroy);
> > +
> > +	ctx->surface = surface;
> > +
> > +	surface->width = width;
> > +	surface->height = height;
> > +
> > +	surface->wl_buffer = create_shm_buffer(ctx->shm, width, height,
> > +					       &surface->data);
> > +
> > +	memset(surface->data, 64, width * height * 4);
> > +
> > +	return ctx;
> > +err_destroy:
> > +	wl_surface_destroy(wl_surface);
> > +err_free:
> > +	wtst_ctx_destroy(ctx);
> > +err:
> > +	return NULL;
> > +}
> > +
> > +/**
> > + * Creates an instance of the test fixture that is chained.
> > + * Additional registry advertisements will be passed on.
> > + *
> > + * @param display name of the wayland display to connect to or NULL for
> > + * the default.
> > + * @param reg_listener the registry listener to chain in.
> > + * @param reg_data pointer to pass back to registry listener.
> > + * @return a pointer to a test fixture upon success, NULL otherwise.
> > + * @see wtst_ctx_destroy()
> > + */
> > +struct wtst_ctx *
> > +wtst_ctx_create_chained(const char *display,
> > +			struct wl_registry_listener *reg_listener,
> > +			void *reg_data)
> > +{
> > +	struct wtst_ctx *ctx = zalloc(sizeof(*ctx));
> > +	ZUC_ASSERTG_NOT_NULL(ctx, err);
> > +
> > +	wl_list_init(&ctx->inputs);
> > +	ctx->reg_listener = zalloc(sizeof(*(ctx->reg_listener)));
> 
> Why is reg_listener dynamically allocated?
> 
> I was expecting to find a static const wl_registry_listener and got
> puzzled at first for not finding it.
> 
> > +	ZUC_ASSERTG_NOT_NULL(ctx->reg_listener, err_free);
> > +
> > +	/* connect to display. */
> > +	ctx->display = wl_display_connect(display);
> > +	ZUC_ASSERTG_NOT_NULL(ctx->display, err_free);
> > +
> > +	ctx->private = wtst_ctx_private_create();
> > +	ZUC_ASSERTG_NOT_NULL(ctx->private, err_free);
> > +
> > +	ctx->private->chained = reg_listener;
> > +	ctx->private->chained_data = reg_data;
> > +
> > +	/* setup registry so we can bind to interfaces. */
> > +	ctx->registry = wl_display_get_registry(ctx->display);
> > +	ctx->reg_listener->global = reg_global;
> > +	ctx->reg_listener->global_remove = reg_global_remove;
> > +	wl_registry_add_listener(ctx->registry,
> > +				 ctx->reg_listener,
> > +				 ctx);
> > +
> > +	/* this roundtrip makes sure we have all globals
> > +	 * and we have bound to them: */
> > +	wl_display_roundtrip(ctx->display);
> > +
> > +	/* this roundtrip makes sure we got all wl_shm
> > +	 * format and wl_seat.* events: */
> 
> And also wl_output events.
> 
> > +	wl_display_roundtrip(ctx->display);
> > +
> > +	/* find the right input for us */
> > +	wtst_ctx_set_input(ctx);
> > +
> > +	/* must have WL_SHM_FORMAT_ARGB32 */
> > +	ZUC_ASSERTG_TRUE(has_shm_format(ctx, WL_SHM_FORMAT_ARGB8888), err_free);
> > +
> > +	/* must have an output */
> > +	ZUC_ASSERTG_NOT_NULL(ctx->output, err_free);
> > +
> > +	/* the output must be initialized */
> > +	ZUC_ASSERTG_TRUE(ctx->output->initialized, err_free);
> > +
> > +	/* must have seat set */
> > +	ZUC_ASSERTG_NOT_NULL(ctx->input, err_free);
> > +
> > +	return ctx;
> > +
> > +err_free:
> > +	wtst_ctx_destroy(ctx);
> > +err:
> > +	return NULL;
> > +}
> > +
> > +/**
> > + * Destroys a test fixture.
> > + *
> > + * @param ctx pointer to a test fixture to destroy.
> > + * It should have previously been created via wtst_ctx_create().
> > + * @see wtst_ctx_create()
> > + */
> > +void
> > +wtst_ctx_destroy(struct wtst_ctx *ctx)
> > +{
> > +	if (!ctx)
> > +		return;
> > +
> > +	if (ctx->surface)
> > +		wtst_surface_destroy(ctx->surface);
> > +
> > +	if (ctx->registry)
> > +		wl_registry_destroy(ctx->registry);
> > +
> > +	if (ctx->reg_listener)
> > +		free(ctx->reg_listener);
> > +
> > +	if (ctx->shell)
> > +		wl_shell_destroy(ctx->shell);
> > +
> > +	if (ctx->shm)
> > +		wl_shm_destroy(ctx->shm);
> > +
> > +	if (ctx->compositor)
> > +		wl_compositor_destroy(ctx->compositor);
> > +
> > +	if (ctx->display)
> > +		wl_display_disconnect(ctx->display);
> > +
> > +	wtst_ctx_private_destroy(ctx->private);
> > +	free(ctx);
> > +}
> > +
> > +static struct wtst_pointer *
> > +wtst_pointer_create(void)
> > +{
> > +	struct wtst_pointer *pointer = zalloc(sizeof(*pointer));
> > +	ZUC_ASSERTG_NOT_NULL(pointer, err);
> > +
> > +	pointer->private = zalloc(sizeof(*pointer->private));
> > +	ZUC_ASSERTG_NOT_NULL(pointer->private, err_free);
> > +	wl_list_init(&pointer->private->link);
> > +
> > +	return pointer;
> > +err_free:
> > +	free(pointer);
> > +err:
> > +	return NULL;
> > +}
> > +
> > +/**
> > + * Adds a listener to a specified pointer.
> > + *
> > + * @param pointer the pointer instance to add a listener to.
> > + * @param listener the listener to add.
> > + * @param data callback data.
> > + */
> > +int
> > +wtst_pointer_add_listener(struct wtst_pointer *pointer,
> > +			  const struct wl_pointer_listener *listener,
> > +			  void *data)
> > +{
> > +	struct wtst_pointer_private *priv = zalloc(sizeof(*priv));
> > +	ZUC_ASSERTG_NOT_NULL(priv, err);
> > +	priv->data = data;
> > +	priv->listener = listener;
> > +	wl_list_insert(&pointer->private->link, &priv->link);
> > +	return 0;
> > +err:
> > +	return -1;
> > +}
> > +
> > +void
> > +input_update_devices(struct wtst_input *input)
> > +{
> > +	struct wl_seat *seat = input->wl_seat;
> > +	enum wl_seat_capability caps = input->caps;
> > +
> > +	if ((caps & WL_SEAT_CAPABILITY_POINTER) && !input->pointer) {
> > +		struct wtst_pointer *pointer = wtst_pointer_create();
> > +		pointer->wl_pointer = wl_seat_get_pointer(seat);
> > +		wl_pointer_set_user_data(pointer->wl_pointer, pointer);
> > +		wl_pointer_add_listener(pointer->wl_pointer, &pointer_listener,
> > +					pointer);
> 
> add_listener overwrites set_user_data; set_user_data is unnecessary.
> 
> > +		input->pointer = pointer;
> > +	} else if (!(caps & WL_SEAT_CAPABILITY_POINTER) && input->pointer) {
> > +		wl_pointer_destroy(input->pointer->wl_pointer);
> > +		free(input->pointer);
> > +		input->pointer = NULL;
> > +	}
> > +
> > +	if ((caps & WL_SEAT_CAPABILITY_KEYBOARD) && !input->keyboard) {
> > +		struct wtst_keyboard *keyboard = zalloc(sizeof(*keyboard));
> > +		ZUC_ASSERT_NOT_NULL(keyboard);
> > +		keyboard->wl_keyboard = wl_seat_get_keyboard(seat);
> > +		wl_keyboard_set_user_data(keyboard->wl_keyboard, keyboard);
> > +		wl_keyboard_add_listener(keyboard->wl_keyboard,
> > +					 &keyboard_listener, keyboard);
> 
> and here.
> 
> > +		input->keyboard = keyboard;
> > +	} else if (!(caps & WL_SEAT_CAPABILITY_KEYBOARD) && input->keyboard) {
> > +		wl_keyboard_destroy(input->keyboard->wl_keyboard);
> > +		free(input->keyboard);
> > +		input->keyboard = NULL;
> > +	}
> > +
> > +	if ((caps & WL_SEAT_CAPABILITY_TOUCH) && !input->touch) {
> > +		struct wtst_touch *touch = zalloc(sizeof(*touch));
> > +		ZUC_ASSERT_NOT_NULL(touch);
> > +		touch->wl_touch = wl_seat_get_touch(seat);
> > +		wl_touch_set_user_data(touch->wl_touch, touch);
> > +		wl_touch_add_listener(touch->wl_touch, &touch_listener,
> > +					 touch);
> 
> and here.
> 
> > +		input->touch = touch;
> > +	} else if (!(caps & WL_SEAT_CAPABILITY_TOUCH) && input->touch) {
> > +		wl_touch_destroy(input->touch->wl_touch);
> > +		free(input->touch);
> > +		input->touch = NULL;
> > +	}
> > +}
> > +
> > +void
> > +wtst_ctx_set_input(struct wtst_ctx *ctx)
> 
> Should be static and preferably before its use in the file.
> 
> > +{
> > +	/* For now prune all but first. Later support looking for named seats */
> > +	struct wtst_input *inp, *inptmp;
> > +	wl_list_for_each_safe(inp, inptmp, &ctx->inputs, link) {
> > +		ZUC_ASSERT_TRUE(inp->seat_name && "BUG: input with no name");
> > +		if (!ctx->input) {
> > +			ctx->input = inp;
> > +			input_update_devices(inp);
> > +		} else {
> > +			input_destroy(inp);
> > +		}
> > +	}
> > +
> > +	/* we keep only one input */
> > +	ZUC_ASSERT_EQ(1, wl_list_length(&ctx->inputs));
> > +}
> > +
> > +/**
> > + * Determines of an interface has been advertized by the Wayland registry
> > + * as being present.
> > + *
> > + * @param ctx the test fixture in use.
> > + * @param interface the name of the interface to query.
> > + * @return true if the interface is supported, false otherwise.
> > + */
> > +int
> > +wtst_is_global_advertised(struct wtst_ctx *ctx, char const *interface)
> > +{
> > +	int found = 0;
> > +	if (ctx && ctx->private && interface) {
> > +		struct wtst_global *tmp = NULL;
> > +		wl_list_for_each(tmp, &ctx->private->global_list, link)
> > +			if (strcmp(interface, tmp->interface) == 0) {
> > +				found = true;
> > +				break;
> > +			}
> > +	}
> > +	return found;
> > +}
> > +
> > +/**
> > + * Creates a wl_shell_surface and packages it in a wtst_shell_surface wrapper.
> > + */
> > +struct wtst_shell_surface *
> > +wtst_create_shell_surface(struct wl_compositor *compositor,
> > +			  struct wl_shell *shell)
> > +{
> > +	struct wl_surface *wl_surface = NULL;
> > +	struct wtst_shell_surface *wss = zalloc(sizeof(*wss));
> > +	ZUC_ASSERTG_NOT_NULL(wss, err);
> > +
> > +	wl_surface = wl_compositor_create_surface(compositor);
> > +	ZUC_ASSERTG_NOT_NULL(wl_surface, err_free);
> > +
> > +	wss->surface = wrap_wl_surface(wl_surface);
> > +	ZUC_ASSERTG_NOT_NULL(wss->surface, err_destroy);
> > +	wl_surface = NULL; /* ownership was transferred. */
> > +
> > +	wss->wl_shell_surface =
> > +		wl_shell_get_shell_surface(shell, wss->surface->wl_surface);
> > +	ZUC_ASSERTG_NOT_NULL(wss->wl_shell_surface, err_destroy);
> > +
> > +	wl_shell_surface_add_listener(wss->wl_shell_surface,
> > +				      &shell_surface_listener,
> > +				      wss);
> > +	wl_shell_surface_set_toplevel(wss->wl_shell_surface);
> > +	wl_shell_surface_set_user_data(wss->wl_shell_surface, wss);
> 
> set_user_data overwrites add_listener's data.
> 
> > +
> > +	return wss;
> > +
> > +err_destroy:
> > +	if (wl_surface)
> > +		wl_surface_destroy(wl_surface);
> > +
> > +	if (wss->surface) {
> > +		if (wss->surface->wl_surface)
> > +			wl_surface_destroy(wss->surface->wl_surface);
> > +		free(wss->surface);
> > +	}
> > +err_free:
> > +	free(wss);
> > +err:
> > +	return NULL;
> > +}
> 
> I didn't check that all the protocol actions, especially wl_surface vs.
> wl_shell and cursor, happen in the usual order. That needs verifying at
> some point.
> 
> 
> Thanks,
> pq

> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v2
> 
> iQIVAwUBVh5WCSNf5bQRqqqnAQigmw/+PejgTCvBADZqx+vDKe9zME2LPOJZAId6
> //VxpsEwvK4PGNsioaKYB8/+JxiQA6Rybb0q1R1CKSabQdFVg6Jzg11R+TUQFHZk
> xZ7waolbQ66vx4tMs55n1CWGWKbk3SXNz8JVCIzddRlr80Qt75qlz8fY9gh2n3Xz
> npqJlTzqbXzFkGQIs6gVYe0dI2BwbxHGcqWbirj1PUHqHvr9uh+OUYAscWAJ5kqh
> lS5lXEs6fLP7svKIOZZCLBssxjHj6TZ+2d7sRbaJpgfikO4Wzo0TZRIj+lRso2sf
> 9CtmD6jYleFC5CWll13D+hVsnUFvyFqdq9wY7OHE1H9rldBSBTw3Qm1nz+iNwEgq
> lqsUngKg77d7dtcQR/9mnkzcUUSor8clLzTP6xwuHvDQJjls/dSp0by9rjrHveus
> QeTFbRwi+s4r9e62eO6Czn6P8XjDZQ/KuXRbDBRB/PGUoW59PbMx+kiI2QW34hJb
> gSU6PiwMAQo1RCrkXo60rkhODaeqbTsLDdiWgOY8num+PwuFsDZTOALF0PY59jD/
> 3BdY5YiaSDxG5QPueq4vDL5zWPKFP7rhoUPRpeCZoovJ5aBrI0aBHvp55tUXZMI0
> ncJJmCNuCcbFjAx1kGjnHOg6CW9HhmQ0kkLWnMCkxYlhEuyRH+631NXMqqVeHsWI
> +p+yerKOna8=
> =veYL
> -----END PGP SIGNATURE-----


> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Bryce


More information about the wayland-devel mailing list