Weston test suite (Re: [PATCH weston 1/4] ivi-shell: change layer visibility to bool)

2018-02-15 Thread Pekka Paalanen 
On Thu, 15 Feb 2018 15:30:55 +
Emil Velikov  wrote:

> On 13 February 2018 at 17:46, Daniel Stone  wrote:
> > Hi Emil,
> >
> > On 13 February 2018 at 16:37, Emil Velikov  
> > wrote:  
> >> The following two questions come to mind:
> >>  - app bugs - using threads? ivi-shell-user-interface.c mentions
> >> pthread, but haven't looked closely  
> >
> > Hm, I couldn't find any threading-related bugs at all: everything I
> > found was clear just looking at the inter-process flows. Did you have
> > anything in particular in mind?
> >  
> >>  - missing dependencies  
> >
> > Again, did you have anything in particular in mind? Looking at the
> > logs, it loads the exact same set of plugins/helpers/configs in both
> > cases (working/broken). Dumping out /proc/self/smaps didn't show any
> > discrepancies either. Everything was fresh and up-to-date: it's seems
> > like it's only the test execution environment which influences this.
> >  
> >> Having a look reveals:
> >>  - extremely convoluted test runner  
> >
> > Sure. We had a few requirements which led to the runner being the way
> > it was, mainly based on it being relatively easy to write tests, and
> > assertion failures / segfaults not destroying the whole testsuite.
> > Personally I like igt, and I guess others like gtest or maybe Piglit,
> > but none of them are anything like simple. Do you have a good
> > replacement in mind?
> >
> > (Whilst we're mentioning test runners and threads, I really wouldn't
> > mind one which ran everything in threads rather than forking
> > children.)
> >  
> >>  - dummy BACKEND variable - always headless-backend.so  
> >
> > It's not a dummy: you can run 'make check BACKEND=foo-backend.so' as a
> > developer, if you'd like to test a particular backend. Most of them
> > are totally unsuitable for general-purpose use (e.g. wayland-backend
> > or x11-backend will pop up a billion windows as it goes about its
> > work, and that rapid de/focus can in fact break some tests), so we
> > default to headless-backend for mere mortals. But it is there as an
> > option for people who know what they're doing, or want to try tests in
> > a different context.
> >  
> >>  - the --backend, --config, --shell and/or --modules file is not part
> >> of the respective dependency chain  
> >
> > The checks here are only run as part of the 'check' target; check
> > depends on check-am, which in turn depends on the all-am target, which
> > in turn depends on all libraries/programs/built-sources, which covers
> > all of the items you've mentioned. Is there some subtlety I'm missing
> > here?
> >  
> Pardon for the delay, Dan.
> While Emre has already resolved the issue, let me add a couple words
> more to my earlier, cryptic, email:
> 
>  - threading
> I could not spot any, although the behaviour that you described
> sounded like an in-weston race condition.
> We now know that wasn't a race, but a layering confusion/violation(?).

Hi Emil

It was a race. Loading a plugin that was not supposed be loaded
(hmi-controller.so) forked a helper process, which raced against the
actual tests (the test has a weston process and a test process, but they
run synchronized via Wayland).

The racing helper process triggered actions in the plugin that was not
supposed to be loaded, changing the weston process state which confused
the weston process part of the test.

I cannot see it as a layering thing. It was a matter of loading two
mutually exclusive plugins, both hooking up to the same ivi-layout API.
The API does not protect against this, because the original idea was
that it would be useful to be able to load multiple plugins with the
caveat that they must be carefully designed to avoid assuming that they
are the only user of the API. However, a test plugin using this API
must really be the only user of the API, otherwise the tests become
unpredictable.

We don't use threads in Weston code base, but we do fork helpers.

> On the testrunner side - I've used cmocka and it seemed rather nice.
> I liked the exception handling for signals, it has no templates plus
> valgrind was always happy.

The Weston test suite is mostly about integration(?) testing rather
than unit testing. There are some tests you could probably classify as
unit tests, but most of them need almost a full-blown compositor. I
would assume that the lack of small contained interfaces in libweston
makes writing meaningful mocks a big effort with prohibitive
maintenance costs. Hence we tend to test the whole compositor with the
public interfaces it has: input devices and rendered output vs. client
Wayland interface operation, or only the Wayland interface.

That said, there are several mock-like components: headless-backend,
the test clients, the IVI controller test plugin, the desktop test
shell plugin. These are used by having libweston load them instead of
what one uses in production.

There have been floating ideas of

Re: [PATCH libinput 1/2] meson: Fix absolute libdir case in install script

2018-02-15 Thread Peter Hutterer 
On Thu, Nov 30, 2017 at 09:23:38AM +0100, Quentin Glidic wrote:
> From: Quentin Glidic 
> 
> If libdir is an absolute path (which means it’s outside of prefix) we
> would wrongly add the prefix to it in the install script. Just pass the
> correct libdir from Meson directly thanks to join_paths() magic.
> 
> Signed-off-by: Quentin Glidic 
> ---
> Just found this one while doing patch 2, I didn’t actually test it.

sorry for the delay, I missed this one and only noticed it now when the
other one showed up in bug https://bugs.freedesktop.org/show_bug.cgi?id=105080

Both applied now, expect them to be pushed as once the test runs finish.

Cheers,
   Peter
 
>  meson.build | 2 +-
>  src/libinput-restore-selinux-context.sh | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index 9b0fa246..6c5b9a84 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -225,7 +225,7 @@ pkgconfig.generate(
>  # Restore the SELinux context for the libinput.so.a.b.c on install
>  # meson bug https://github.com/mesonbuild/meson/issues/1967
>  meson.add_install_script('src/libinput-restore-selinux-context.sh',
> -  get_option('libdir'),
> +  join_paths(get_option('prefix'), get_option('libdir')),
>lib_libinput.full_path())
>  
>   documentation 
> diff --git a/src/libinput-restore-selinux-context.sh 
> b/src/libinput-restore-selinux-context.sh
> index 3b1c555b..606b5b08 100644
> --- a/src/libinput-restore-selinux-context.sh
> +++ b/src/libinput-restore-selinux-context.sh
> @@ -7,6 +7,6 @@ libdir="$1"
>  sofile=$(basename "$2")
>  
>  if command -v restorecon >/dev/null; then
> - echo "Restoring SELinux context on 
> $MESON_INSTALL_DESTDIR_PREFIX/$libdir/$sofile"
> - restorecon "$MESON_INSTALL_DESTDIR_PREFIX/$libdir/$sofile"
> + echo "Restoring SELinux context on ${DESTDIR}${libdir}/${sofile}"
> + restorecon "${DESTDIR}${libdir}/${sofile}"
>  fi
> -- 
> 2.15.0
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> 
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 2/6] wayland-egl: make wayland-egl-backend.h C++ safe

2018-02-15 Thread Derek Foreman 

On 2018-02-15 12:50 PM, Emil Velikov wrote:

From: Emil Velikov 

private is a reserved keyworkd in C++. Thus to make things work, we
should use something else - wl_egl_window::driver_private.

Make sure to do so only for C++ contexts, or otherwise it will break the
API - leading to build failures when used alongside existing Mesa.

Cc: Daniel Stone 
Cc: Arnaud Vrac 
Cc: Miguel A . Vico 
Signed-off-by: Emil Velikov 
---
  egl/wayland-egl-backend.h | 8 
  1 file changed, 8 insertions(+)

diff --git a/egl/wayland-egl-backend.h b/egl/wayland-egl-backend.h
index 3c23a56..b16ff45 100644
--- a/egl/wayland-egl-backend.h
+++ b/egl/wayland-egl-backend.h
@@ -53,7 +53,15 @@ struct wl_egl_window {
int attached_width;
int attached_height;
  
+	/*

+* private is a reserved word in C++ - use driver_private instead.
+* Don't change the name otherwise, in order to preserve the existing C 
API.
+*/
+#ifdef __cplusplus
+   void *driver_private;
+#else
void *private;
+#endif


Maybe it's just me, but having different names for the same variable for 
C vs C++ compilation seems really nasty and potentially surprising.


Shouldn't old versions of Mesa already have internal versions of this 
header anyway?  I didn't think it was publicly installed until now...


Thanks,
Derek


void (*resize_callback)(struct wl_egl_window *, void *);
void (*destroy_window_callback)(void *);
  



___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH wayland 4/6] wayland-egl: fail the symbol check if lib is missing

2018-02-15 Thread Eric Engestrom 


On February 15, 2018 6:55:14 PM UTC, Emil Velikov  
wrote:
> From: Emil Velikov 
> 
> Based on a similar patch (in Mesa) by Eric Engestrom.
> 
> Cc: Eric Engestrom 

Reviewed-by: Eric Engestrom 

> Signed-off-by: Emil Velikov 
> ---
>  egl/wayland-egl-symbols-check | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/egl/wayland-egl-symbols-check
> b/egl/wayland-egl-symbols-check
> index e7105ea..93a3361 100755
> --- a/egl/wayland-egl-symbols-check
> +++ b/egl/wayland-egl-symbols-check
> @@ -1,6 +1,14 @@
>  #!/bin/sh
> +set -eu
>  
> -FUNCS=$(nm -D --defined-only ${1-.libs/libwayland-egl.so} | grep -o
> "T .*" | cut -c 3- | while read func; do
> +LIB=${1-.libs/libwayland-egl.so}
> +
> +if [ ! -f "$LIB" ]; then
> + echo "The test binary \"$LIB\" does no exist"
> + exit 1
> +fi
> +
> +FUNCS=$(nm -D --defined-only $LIB | grep -o "T .*" | cut -c 3- |
> while read func; do
>  ( grep -q "^$func$" || echo $func )  <  wl_egl_window_resize
>  wl_egl_window_create
> -- 
> 2.16.0
> 
> ___
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland 6/6] wayland-egl: bump the version number to 18.1.0

2018-02-15 Thread Emil Velikov 
From: Emil Velikov 

Seems like I was overoptimistic with my earlier assumption, namely:

"... 17.3.x should be the last version that ships the library."

Mesa 18.0.0 and its wayland-egl is about to be released any time soon,
so bump the number since it must no be smaller. As soon as we get
a wayland release I'll drop the Mesa copy but for now.

Signed-off-by: Emil Velikov 
---
 egl/wayland-egl.pc.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/egl/wayland-egl.pc.in b/egl/wayland-egl.pc.in
index 943442e..2e2d4c4 100644
--- a/egl/wayland-egl.pc.in
+++ b/egl/wayland-egl.pc.in
@@ -5,7 +5,7 @@ includedir=@includedir@
 
 Name: wayland-egl
 Description: Frontend wayland-egl library
-Version: 17.4.0
+Version: 18.1.0
 Requires: wayland-client
 Libs: -L${libdir} -lwayland-egl
 Cflags: -I${includedir}
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland 4/6] wayland-egl: fail the symbol check if lib is missing

2018-02-15 Thread Emil Velikov 
From: Emil Velikov 

Based on a similar patch (in Mesa) by Eric Engestrom.

Cc: Eric Engestrom 
Signed-off-by: Emil Velikov 
---
 egl/wayland-egl-symbols-check | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index e7105ea..93a3361 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,6 +1,14 @@
 #!/bin/sh
+set -eu
 
-FUNCS=$(nm -D --defined-only ${1-.libs/libwayland-egl.so} | grep -o "T .*" | 
cut -c 3- | while read func; do
+LIB=${1-.libs/libwayland-egl.so}
+
+if [ ! -f "$LIB" ]; then
+   echo "The test binary \"$LIB\" does no exist"
+   exit 1
+fi
+
+FUNCS=$(nm -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while read 
func; do
 ( grep -q "^$func$" || echo $func )

[PATCH wayland 2/6] wayland-egl: make wayland-egl-backend.h C++ safe

2018-02-15 Thread Emil Velikov 
From: Emil Velikov 

private is a reserved keyworkd in C++. Thus to make things work, we
should use something else - wl_egl_window::driver_private.

Make sure to do so only for C++ contexts, or otherwise it will break the
API - leading to build failures when used alongside existing Mesa.

Cc: Daniel Stone 
Cc: Arnaud Vrac 
Cc: Miguel A . Vico 
Signed-off-by: Emil Velikov 
---
 egl/wayland-egl-backend.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/egl/wayland-egl-backend.h b/egl/wayland-egl-backend.h
index 3c23a56..b16ff45 100644
--- a/egl/wayland-egl-backend.h
+++ b/egl/wayland-egl-backend.h
@@ -53,7 +53,15 @@ struct wl_egl_window {
int attached_width;
int attached_height;
 
+   /*
+* private is a reserved word in C++ - use driver_private instead.
+* Don't change the name otherwise, in order to preserve the existing C 
API.
+*/
+#ifdef __cplusplus
+   void *driver_private;
+#else
void *private;
+#endif
void (*resize_callback)(struct wl_egl_window *, void *);
void (*destroy_window_callback)(void *);
 
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland 5/6] wayland-egl: enhance the symbol test

2018-02-15 Thread Emil Velikov 
From: Emil Velikov 

The current test had a few fall-outs:
 - it was checking only for T (.text) symbols
 - did not consider symbol removal

Fix that by fetching all the symbols and doing a bidirectional check -
for added and removed symbols. Error out with informative message for
each case.

Signed-off-by: Emil Velikov 
---
 egl/wayland-egl-symbols-check | 36 
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index 93a3361..c1239c1 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -8,17 +8,37 @@ if [ ! -f "$LIB" ]; then
exit 1
 fi
 
-FUNCS=$(nm -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while read 
func; do
-( grep -q "^$func$" || echo $func )

[PATCH wayland 3/6] Revert "wayland-egl-symbols-check: pass the DSO name via the build system"

2018-02-15 Thread Emil Velikov 
This reverts commit 85cb5ed64aa8246f4da93fc5b76dfc34096bf803.

It seems like we've misread the existing code - the DSO name can be
propagated via the build-system. The one available in the script was a
simple fall-back.

Cc: Daniel Stone 
---
 Makefile.am   | 1 -
 egl/wayland-egl-symbols-check | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 322d6b8..7f24ca5 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -192,7 +192,6 @@ AM_TESTS_ENVIRONMENT =  
\
export WAYLAND_SCANNER='$(top_builddir)/wayland-scanner'\
TEST_DATA_DIR='$(top_srcdir)/tests/data'\
TEST_OUTPUT_DIR='$(top_builddir)/tests/output'  \
-   WAYLAND_EGL_LIB='$(top_builddir)/egl/.libs/libwayland-egl.so'   \
SED=$(SED)  \
;
 
diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index e107362..e7105ea 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-FUNCS=$(nm -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut -c 3- | 
while read func; do
+FUNCS=$(nm -D --defined-only ${1-.libs/libwayland-egl.so} | grep -o "T .*" | 
cut -c 3- | while read func; do
 ( grep -q "^$func$" || echo $func )

[PATCH wayland 1/6] Revert "wayland-egl: rename wl_egl_window::private to driver_private"

2018-02-15 Thread Emil Velikov 
This reverts commit 9fa60983b5799be62b9d88a4059f4d0038d7c80d.

The commit preserves the ABI, although breaks the API. This is fine for
most cases - building up-to date components, or shipping binary-only
drivers.

Yet it breaks when using old, released, Mesa alongside new wayland-egl.
A simpler and compatible way to handle the C++ inclusion of the header
is coming in shortly. But for now revert this change.

https://bugs.freedesktop.org/show_bug.cgi?id=105103
Cc: Daniel Stone 
Cc: Arnaud Vrac 
Cc: Miguel A . Vico 
---
 egl/wayland-egl-abi-check.c | 6 +++---
 egl/wayland-egl-backend.h   | 2 +-
 egl/wayland-egl.c   | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/egl/wayland-egl-abi-check.c b/egl/wayland-egl-abi-check.c
index faa6b57..62c51a2 100644
--- a/egl/wayland-egl-abi-check.c
+++ b/egl/wayland-egl-abi-check.c
@@ -91,7 +91,7 @@ struct wl_egl_window_v3 {
 int attached_width;
 int attached_height;
 
-void *driver_private;
+void *private;
 void (*resize_callback)(struct wl_egl_window *, void *);
 void (*destroy_window_callback)(void *);
 
@@ -208,7 +208,7 @@ int main(int argc, char **argv)
 CHECK_MEMBER(_v2, _v3, dy);
 CHECK_MEMBER(_v2, _v3, attached_width);
 CHECK_MEMBER(_v2, _v3, attached_height);
-CHECK_RENAMED_MEMBER(_v2, _v3, private, driver_private);
+CHECK_MEMBER(_v2, _v3, private);
 CHECK_MEMBER(_v2, _v3, resize_callback);
 CHECK_MEMBER(_v2, _v3, destroy_window_callback);
 
@@ -223,7 +223,7 @@ int main(int argc, char **argv)
 CHECK_MEMBER_CURRENT(_v3, dy);
 CHECK_MEMBER_CURRENT(_v3, attached_width);
 CHECK_MEMBER_CURRENT(_v3, attached_height);
-CHECK_MEMBER_CURRENT(_v3, driver_private);
+CHECK_MEMBER_CURRENT(_v3, private);
 CHECK_MEMBER_CURRENT(_v3, resize_callback);
 CHECK_MEMBER_CURRENT(_v3, destroy_window_callback);
 CHECK_MEMBER_CURRENT(_v3, surface);
diff --git a/egl/wayland-egl-backend.h b/egl/wayland-egl-backend.h
index 869c86f..3c23a56 100644
--- a/egl/wayland-egl-backend.h
+++ b/egl/wayland-egl-backend.h
@@ -53,7 +53,7 @@ struct wl_egl_window {
int attached_width;
int attached_height;
 
-   void *driver_private;
+   void *private;
void (*resize_callback)(struct wl_egl_window *, void *);
void (*destroy_window_callback)(void *);
 
diff --git a/egl/wayland-egl.c b/egl/wayland-egl.c
index a60f899..02ac04e 100644
--- a/egl/wayland-egl.c
+++ b/egl/wayland-egl.c
@@ -49,7 +49,7 @@ wl_egl_window_resize(struct wl_egl_window *egl_window,
egl_window->dy = dy;
 
if (egl_window->resize_callback)
-   egl_window->resize_callback(egl_window, 
egl_window->driver_private);
+   egl_window->resize_callback(egl_window, egl_window->private);
 }
 
 WL_EXPORT struct wl_egl_window *
@@ -89,7 +89,7 @@ WL_EXPORT void
 wl_egl_window_destroy(struct wl_egl_window *egl_window)
 {
if (egl_window->destroy_window_callback)
-   egl_window->destroy_window_callback(egl_window->driver_private);
+   egl_window->destroy_window_callback(egl_window->private);
free(egl_window);
 }
 
-- 
2.16.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston 1/4] ivi-shell: change layer visibility to bool

2018-02-15 Thread Emil Velikov 
On 13 February 2018 at 17:46, Daniel Stone  wrote:
> Hi Emil,
>
> On 13 February 2018 at 16:37, Emil Velikov  wrote:
>> The following two questions come to mind:
>>  - app bugs - using threads? ivi-shell-user-interface.c mentions
>> pthread, but haven't looked closely
>
> Hm, I couldn't find any threading-related bugs at all: everything I
> found was clear just looking at the inter-process flows. Did you have
> anything in particular in mind?
>
>>  - missing dependencies
>
> Again, did you have anything in particular in mind? Looking at the
> logs, it loads the exact same set of plugins/helpers/configs in both
> cases (working/broken). Dumping out /proc/self/smaps didn't show any
> discrepancies either. Everything was fresh and up-to-date: it's seems
> like it's only the test execution environment which influences this.
>
>> Having a look reveals:
>>  - extremely convoluted test runner
>
> Sure. We had a few requirements which led to the runner being the way
> it was, mainly based on it being relatively easy to write tests, and
> assertion failures / segfaults not destroying the whole testsuite.
> Personally I like igt, and I guess others like gtest or maybe Piglit,
> but none of them are anything like simple. Do you have a good
> replacement in mind?
>
> (Whilst we're mentioning test runners and threads, I really wouldn't
> mind one which ran everything in threads rather than forking
> children.)
>
>>  - dummy BACKEND variable - always headless-backend.so
>
> It's not a dummy: you can run 'make check BACKEND=foo-backend.so' as a
> developer, if you'd like to test a particular backend. Most of them
> are totally unsuitable for general-purpose use (e.g. wayland-backend
> or x11-backend will pop up a billion windows as it goes about its
> work, and that rapid de/focus can in fact break some tests), so we
> default to headless-backend for mere mortals. But it is there as an
> option for people who know what they're doing, or want to try tests in
> a different context.
>
>>  - the --backend, --config, --shell and/or --modules file is not part
>> of the respective dependency chain
>
> The checks here are only run as part of the 'check' target; check
> depends on check-am, which in turn depends on the all-am target, which
> in turn depends on all libraries/programs/built-sources, which covers
> all of the items you've mentioned. Is there some subtlety I'm missing
> here?
>
Pardon for the delay, Dan.
While Emre has already resolved the issue, let me add a couple words
more to my earlier, cryptic, email:

 - threading
I could not spot any, although the behaviour that you described
sounded like an in-weston race condition.
We now know that wasn't a race, but a layering confusion/violation(?).
 - missing dependencies [in the Makefile.am]
I didn't explicitly check if all the files (binaries and configs) are
correctly annotated, hence present before the tests were run.


On the testrunner side - I've used cmocka and it seemed rather nice.
I liked the exception handling for signals, it has no templates plus
valgrind was always happy.

HTH
Emil

[1] https://api.cmocka.org/
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH wayland] client: remove definition of wl_global

2018-02-15 Thread Pekka Paalanen 
From: Pekka Paalanen 

Nothing on the client side uses it since
9fe75537ad207c1496e6d9be41a8f5af4b876506 which was just before the 0.99
release.

Signed-off-by: Pekka Paalanen 
---
 src/wayland-client.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/src/wayland-client.c b/src/wayland-client.c
index c1369b8..c66e0e5 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -71,13 +71,6 @@ struct wl_proxy {
uint32_t version;
 };
 
-struct wl_global {
-   uint32_t id;
-   char *interface;
-   uint32_t version;
-   struct wl_list link;
-};
-
 struct wl_event_queue {
struct wl_list event_list;
struct wl_display *display;
-- 
2.13.6

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Re: [PATCH weston v4 3/7] libweston: Make weston_seat release safe

2018-02-15 Thread Pekka Paalanen 
On Thu, 15 Feb 2018 13:07:09 +0200
Alexandros Frantzis  wrote:

> Ensure the server can safely handle client requests for wl_seat resource
> that have become inert due to weston_seat object release and subsequent
> destruction.
> 
> The clean-up involves, among other things, unsetting the destroyed
> weston_seat object from the user data of wl_seat resources, and handling
> this NULL user data case where required.
> 
> The list of sites extracting and using weston_seat object from wl_seat
> resources which were audited for this patch are:
> 
> Legend:
> N/A = Not Applicable (not implemented by weston)
> FIXED = Fixed in the commit
> OK = Already works correctly
> 
> == keyboard_shortcuts_inhibit_unstable_v1 ==
> [N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
> == tablet_input_unstable_v{1,2} ==
> [N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
> == text_input_unstable_v1 ==
> [FIXED] zwp_text_input_v1.activate
> [FIXED] zwp_text_input_v1.deactivate
> == wl_data_device ==
> [FIXED] wl_data_device_manager.get_data_device
> [OK] wl_data_device.start_drag
> [FIXED] wl_data_device.set_selection
> [OK] wl_data_device.release
> == wl_shell ==
> [FIXED] wl_shell_surface.move
> [FIXED] wl_shell_surface.resize
> [FIXED] wl_shell_surface.set_popup
> == xdg_shell and xdg_shell_unstable_v6 ==
> [FIXED] xdg_toplevel.show_window_menu
> [FIXED] xdg_toplevel.move
> [FIXED] xdg_toplevel.resize
> [FIXED] xdg_popup.grab
> == xdg_shell_unstable_v5 ==
> [FIXED] xdg_shell.get_xdg_popup
> [FIXED] xdg_surface.show_window_menu
> [FIXED] xdg_surface.move
> [FIXED] xdg_surface.resize
> 
> Signed-off-by: Alexandros Frantzis 
> Reviewed-by: Pekka Paalanen 
> Reviewed-by: Quentin Glidic 
> ---
> Changes in v4:
>  - Add seat == NULL in assert in weston_desktop_seat_popup_grab_start.
>  - Move check for NULL seat after checking for toplevel configured in
>weston_desktop_xdg_toplevel_protocol_{show_window_menu, move, resize}
> Changes in v3:
>  - Drop xdg_shell v5 changes since support for v5 has been removed.
>  - Handle inert seats more transparently in libweston-desktop, by ensuring
>that the weston_desktop_seat_from_seat,
>weston_desktop_seat_popup_grab_get_topmost_surface, and
>weston desktop_seat_popup_grab_start functions gracefully handle NULL
>seat pointer arguments internally.
>
> Changes in v2:
>  - Properly create inert resources in seat_get_pointer/touch/keyboard.
>  - Ensure all sites which have a wl_seat input resource can deal
>with inert resources.
> 
>  compositor/text-backend.c|  8 --
>  libweston-desktop/seat.c | 18 
>  libweston-desktop/wl-shell.c |  9 +-
>  libweston-desktop/xdg-shell-v6.c | 24 
>  libweston/data-device.c  | 15 ++
>  libweston/input.c| 61 
> 
>  6 files changed, 103 insertions(+), 32 deletions(-)
> 

Very good, pushed the whole series:
   c6e2942f..8b964bca  master -> master


Thanks,
pq


pgpRevNnJP_Gz.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

[PATCH weston v4 3/7] libweston: Make weston_seat release safe

2018-02-15 Thread Alexandros Frantzis 
Ensure the server can safely handle client requests for wl_seat resource
that have become inert due to weston_seat object release and subsequent
destruction.

The clean-up involves, among other things, unsetting the destroyed
weston_seat object from the user data of wl_seat resources, and handling
this NULL user data case where required.

The list of sites extracting and using weston_seat object from wl_seat
resources which were audited for this patch are:

Legend:
N/A = Not Applicable (not implemented by weston)
FIXED = Fixed in the commit
OK = Already works correctly

== keyboard_shortcuts_inhibit_unstable_v1 ==
[N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
== tablet_input_unstable_v{1,2} ==
[N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
== text_input_unstable_v1 ==
[FIXED] zwp_text_input_v1.activate
[FIXED] zwp_text_input_v1.deactivate
== wl_data_device ==
[FIXED] wl_data_device_manager.get_data_device
[OK] wl_data_device.start_drag
[FIXED] wl_data_device.set_selection
[OK] wl_data_device.release
== wl_shell ==
[FIXED] wl_shell_surface.move
[FIXED] wl_shell_surface.resize
[FIXED] wl_shell_surface.set_popup
== xdg_shell and xdg_shell_unstable_v6 ==
[FIXED] xdg_toplevel.show_window_menu
[FIXED] xdg_toplevel.move
[FIXED] xdg_toplevel.resize
[FIXED] xdg_popup.grab
== xdg_shell_unstable_v5 ==
[FIXED] xdg_shell.get_xdg_popup
[FIXED] xdg_surface.show_window_menu
[FIXED] xdg_surface.move
[FIXED] xdg_surface.resize

Signed-off-by: Alexandros Frantzis 
Reviewed-by: Pekka Paalanen 
Reviewed-by: Quentin Glidic 
---
Changes in v4:
 - Add seat == NULL in assert in weston_desktop_seat_popup_grab_start.
 - Move check for NULL seat after checking for toplevel configured in
   weston_desktop_xdg_toplevel_protocol_{show_window_menu, move, resize}
Changes in v3:
 - Drop xdg_shell v5 changes since support for v5 has been removed.
 - Handle inert seats more transparently in libweston-desktop, by ensuring
   that the weston_desktop_seat_from_seat,
   weston_desktop_seat_popup_grab_get_topmost_surface, and
   weston desktop_seat_popup_grab_start functions gracefully handle NULL
   seat pointer arguments internally.
   
Changes in v2:
 - Properly create inert resources in seat_get_pointer/touch/keyboard.
 - Ensure all sites which have a wl_seat input resource can deal
   with inert resources.

 compositor/text-backend.c|  8 --
 libweston-desktop/seat.c | 18 
 libweston-desktop/wl-shell.c |  9 +-
 libweston-desktop/xdg-shell-v6.c | 24 
 libweston/data-device.c  | 15 ++
 libweston/input.c| 61 
 6 files changed, 103 insertions(+), 32 deletions(-)

diff --git a/compositor/text-backend.c b/compositor/text-backend.c
index e6ee249c..4d8c085b 100644
--- a/compositor/text-backend.c
+++ b/compositor/text-backend.c
@@ -193,10 +193,14 @@ text_input_activate(struct wl_client *client,
 {
struct text_input *text_input = wl_resource_get_user_data(resource);
struct weston_seat *weston_seat = wl_resource_get_user_data(seat);
-   struct input_method *input_method = weston_seat->input_method;
+   struct input_method *input_method;
struct weston_compositor *ec = text_input->ec;
struct text_input *current;
 
+   if (!weston_seat)
+   return;
+
+   input_method = weston_seat->input_method;
if (input_method->input == text_input)
return;
 
@@ -237,7 +241,7 @@ text_input_deactivate(struct wl_client *client,
 {
struct weston_seat *weston_seat = wl_resource_get_user_data(seat);
 
-   if (weston_seat->input_method->input)
+   if (weston_seat && weston_seat->input_method->input)
deactivate_input_method(weston_seat->input_method);
 }
 
diff --git a/libweston-desktop/seat.c b/libweston-desktop/seat.c
index 382b9e41..ae1c5e9f 100644
--- a/libweston-desktop/seat.c
+++ b/libweston-desktop/seat.c
@@ -242,6 +242,9 @@ weston_desktop_seat_from_seat(struct weston_seat *wseat)
struct wl_listener *listener;
struct weston_desktop_seat *seat;
 
+   if (wseat == NULL)
+   return NULL;
+
listener = wl_signal_get(>destroy_signal,
 weston_desktop_seat_destroy);
if (listener != NULL)
@@ -270,7 +273,7 @@ weston_desktop_seat_from_seat(struct weston_seat *wseat)
 struct weston_desktop_surface *
 weston_desktop_seat_popup_grab_get_topmost_surface(struct weston_desktop_seat 
*seat)
 {
-   if (wl_list_empty(>popup_grab.surfaces))
+   if (seat == NULL || wl_list_empty(>popup_grab.surfaces))
return NULL;
 
struct wl_list *grab_link = seat->popup_grab.surfaces.next;
@@ -282,11 +285,14 @@ bool
 weston_desktop_seat_popup_grab_start(struct weston_desktop_seat *seat,
 struct wl_client

Re: [PATCH weston v3 3/7] libweston: Make weston_seat release safe

2018-02-15 Thread Quentin Glidic 

On 2/14/18 2:05 PM, Alexandros Frantzis wrote:

Ensure the server can safely handle client requests for wl_seat resource
that have become inert due to weston_seat object release and subsequent
destruction.

The clean-up involves, among other things, unsetting the destroyed
weston_seat object from the user data of wl_seat resources, and handling
this NULL user data case where required.

The list of sites extracting and using weston_seat object from wl_seat
resources which were audited for this patch are:

Legend:
N/A = Not Applicable (not implemented by weston)
FIXED = Fixed in the commit
OK = Already works correctly

== keyboard_shortcuts_inhibit_unstable_v1 ==
[N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
== tablet_input_unstable_v{1,2} ==
[N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
== text_input_unstable_v1 ==
[FIXED] zwp_text_input_v1.activate
[FIXED] zwp_text_input_v1.deactivate
== wl_data_device ==
[FIXED] wl_data_device_manager.get_data_device
[OK] wl_data_device.start_drag
[FIXED] wl_data_device.set_selection
[OK] wl_data_device.release
== wl_shell ==
[FIXED] wl_shell_surface.move
[FIXED] wl_shell_surface.resize
[FIXED] wl_shell_surface.set_popup
== xdg_shell and xdg_shell_unstable_v6 ==
[FIXED] xdg_toplevel.show_window_menu
[FIXED] xdg_toplevel.move
[FIXED] xdg_toplevel.resize
[FIXED] xdg_popup.grab
== xdg_shell_unstable_v5 ==
[FIXED] xdg_shell.get_xdg_popup
[FIXED] xdg_surface.show_window_menu
[FIXED] xdg_surface.move
[FIXED] xdg_surface.resize

Signed-off-by: Alexandros Frantzis 
Reviewed-by: Pekka Paalanen 
---
Changes in v3:
  - Drop xdg_shell v5 changes since support for v5 has been removed.
  - Handle inert seats more transparently in libweston-desktop, by ensuring that
the weston_desktop_seat_from_seat,
weston_desktop_seat_popup_grab_get_topmost_surface, and
weston desktop_seat_popup_grab_start functions gracefully handle NULL
seat pointer arguments internally.

Changes in v2:
  - Properly create inert resources in seat_get_pointer/touch/keyboard.
  - Ensure all sites which have a wl_seat input resource can deal
with inert resources.

  compositor/text-backend.c|  8 --
  libweston-desktop/seat.c | 13 ++---
  libweston-desktop/wl-shell.c |  9 +-
  libweston-desktop/xdg-shell-v6.c | 24 
  libweston/data-device.c  | 15 ++
  libweston/input.c| 61 
  6 files changed, 100 insertions(+), 30 deletions(-)

diff --git a/compositor/text-backend.c b/compositor/text-backend.c
index e6ee249c..4d8c085b 100644
--- a/compositor/text-backend.c
+++ b/compositor/text-backend.c
@@ -193,10 +193,14 @@ text_input_activate(struct wl_client *client,
  {
struct text_input *text_input = wl_resource_get_user_data(resource);
struct weston_seat *weston_seat = wl_resource_get_user_data(seat);
-   struct input_method *input_method = weston_seat->input_method;
+   struct input_method *input_method;
struct weston_compositor *ec = text_input->ec;
struct text_input *current;
  
+	if (!weston_seat)

+   return;
+
+   input_method = weston_seat->input_method;
if (input_method->input == text_input)
return;
  
@@ -237,7 +241,7 @@ text_input_deactivate(struct wl_client *client,

  {
struct weston_seat *weston_seat = wl_resource_get_user_data(seat);
  
-	if (weston_seat->input_method->input)

+   if (weston_seat && weston_seat->input_method->input)
deactivate_input_method(weston_seat->input_method);
  }
  
diff --git a/libweston-desktop/seat.c b/libweston-desktop/seat.c

index 382b9e41..4e51ee0f 100644
--- a/libweston-desktop/seat.c
+++ b/libweston-desktop/seat.c
@@ -242,6 +242,9 @@ weston_desktop_seat_from_seat(struct weston_seat *wseat)
struct wl_listener *listener;
struct weston_desktop_seat *seat;
  
+	if (wseat == NULL)

+   return NULL;
+
listener = wl_signal_get(>destroy_signal,
 weston_desktop_seat_destroy);
if (listener != NULL)
@@ -270,7 +273,7 @@ weston_desktop_seat_from_seat(struct weston_seat *wseat)
  struct weston_desktop_surface *
  weston_desktop_seat_popup_grab_get_topmost_surface(struct weston_desktop_seat 
*seat)
  {
-   if (wl_list_empty(>popup_grab.surfaces))
+   if (seat == NULL || wl_list_empty(>popup_grab.surfaces))
return NULL;
  
  	struct wl_list *grab_link = seat->popup_grab.surfaces.next;

@@ -284,9 +287,11 @@ weston_desktop_seat_popup_grab_start(struct 
weston_desktop_seat *seat,
  {
assert(seat->popup_grab.client == NULL || seat->popup_grab.client == 
client);
  


Here we should have "seat == NULL || " added, or the assert would 
segfault, and this patch makes it legal to call grab_start() with NULL.




-   struct weston_keyboard *keyboard =