[PATCH v2] client: Fix handling display-reader_count if poll fails

2013-09-25 Thread Neil Roberts
In wl_display_dispatch_queue, if poll fails then it would previously
return immediately and leak a reference in display-reader_count. Then
if the application ignores the error and tries to read again it will
block forever. This can happen for example if the poll fails with
EINTR which the application might consider to be a recoverable error.
This patch makes it cancel the read so the reader_count will be
decremented when poll fails.
---

Ok, here is a second version of the patch which calls
wl_display_cancel_read as you suggested. Thanks for the review.

 src/wayland-client.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/wayland-client.c b/src/wayland-client.c
index cad587d..a20a71e 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -1224,8 +1224,10 @@ wl_display_dispatch_queue(struct wl_display *display,
 
pfd[0].fd = display-fd;
pfd[0].events = POLLIN;
-   if (poll(pfd, 1, -1) == -1)
+   if (poll(pfd, 1, -1) == -1) {
+   wl_display_cancel_read(display);
return -1;
+   }
 
pthread_mutex_lock(display-mutex);
 
-- 
1.8.3.1

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


[PATCH] config: verify that the config file has been parsed

2013-09-25 Thread Alex DAMIAN
From: Alexandru DAMIAN alexandru.dam...@intel.com

weston_config_parse may return NULL, leading
to an ungraceful exit via SIGSEGV.

Adding a nice check that gracefully exits the compositor
when the config.ini parsing failed.

Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
---
 src/compositor.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/compositor.c b/src/compositor.c
index f619f82..9ba6e88 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3489,6 +3489,10 @@ int main(int argc, char *argv[])
}
 
config = weston_config_parse(weston.ini);
+   if (config == NULL) {
+   weston_log(Could not parse config file weston.ini);
+   exit(EXIT_FAILURE);
+   }
weston_log(Using config file '%s'\n,
   weston_config_get_full_path(config));
section = weston_config_get_section(config, core, NULL, NULL);
-- 
1.8.1.2

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


[PATCH v2 1/2] config: verify that the config file has been parsed

2013-09-25 Thread Alex DAMIAN
From: Alexandru DAMIAN alexandru.dam...@intel.com

weston_config_parse may return NULL, leading
to an ungraceful exit via SIGSEGV.

Adding a nice check that gracefully exits the compositor
when the config.ini parsing failed.

Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
---
 src/compositor.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/src/compositor.c b/src/compositor.c
index f619f82..bc4837f 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -3489,6 +3489,10 @@ int main(int argc, char *argv[])
}
 
config = weston_config_parse(weston.ini);
+   if (config == NULL) {
+   weston_log(Could not parse config file weston.ini\n);
+   exit(EXIT_FAILURE);
+   }
weston_log(Using config file '%s'\n,
   weston_config_get_full_path(config));
section = weston_config_get_section(config, core, NULL, NULL);
-- 
1.8.1.2

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


[PATCH v2 2/2] compositor: check if seteuid worked

2013-09-25 Thread Alex DAMIAN
From: Alexandru DAMIAN alexandru.dam...@intel.com

Checking the return value from seteuid in
order to not launch clients with the wrong effective uid.

Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
---
 src/compositor.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index bc4837f..2a16f52 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -247,8 +247,11 @@ child_client_exec(int sockfd, const char *path)
sigfillset(allsigs);
sigprocmask(SIG_UNBLOCK, allsigs, NULL);
 
-   /* Launch clients as the user. */
-   seteuid(getuid());
+   /* Launch clients as the user. Do not lauch clients with wrong euid.*/
+   if (seteuid(getuid()) -1) {
+   weston_log(compositor: failed seteuid\n);
+   return;
+   }
 
/* SOCK_CLOEXEC closes both ends, so we dup the fd to get a
 * non-CLOEXEC fd to pass through exec. */
-- 
1.8.1.2

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


Re: [PATCH v2 2/2] compositor: check if seteuid worked

2013-09-25 Thread Emilio Pozuelo Monfort
Hi,

On 25/09/13 14:48, Alex DAMIAN wrote:
 From: Alexandru DAMIAN alexandru.dam...@intel.com
 
 Checking the return value from seteuid in
 order to not launch clients with the wrong effective uid.
 
 Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
 ---
  src/compositor.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/src/compositor.c b/src/compositor.c
 index bc4837f..2a16f52 100644
 --- a/src/compositor.c
 +++ b/src/compositor.c
 @@ -247,8 +247,11 @@ child_client_exec(int sockfd, const char *path)
   sigfillset(allsigs);
   sigprocmask(SIG_UNBLOCK, allsigs, NULL);
  
 - /* Launch clients as the user. */
 - seteuid(getuid());
 + /* Launch clients as the user. Do not lauch clients with wrong euid.*/
 + if (seteuid(getuid()) -1) {

Missing == operator; this code won't build as is.

Emilio

 + weston_log(compositor: failed seteuid\n);
 + return;
 + }
  
   /* SOCK_CLOEXEC closes both ends, so we dup the fd to get a
* non-CLOEXEC fd to pass through exec. */
 

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


[[PATCH v3 2/2]] compositor: check if seteuid worked

2013-09-25 Thread Alex DAMIAN
From: Alexandru DAMIAN alexandru.dam...@intel.com

Checking the return value from seteuid in
order to not launch clients with the wrong effective uid.

Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
---
 src/compositor.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/compositor.c b/src/compositor.c
index bc4837f..1a85693 100644
--- a/src/compositor.c
+++ b/src/compositor.c
@@ -247,8 +247,11 @@ child_client_exec(int sockfd, const char *path)
sigfillset(allsigs);
sigprocmask(SIG_UNBLOCK, allsigs, NULL);
 
-   /* Launch clients as the user. */
-   seteuid(getuid());
+   /* Launch clients as the user. Do not lauch clients with wrong euid.*/
+   if (seteuid(getuid()) == -1) {
+   weston_log(compositor: failed seteuid\n);
+   return;
+   }
 
/* SOCK_CLOEXEC closes both ends, so we dup the fd to get a
 * non-CLOEXEC fd to pass through exec. */
-- 
1.8.1.2

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


Re: [PATCH v2 2/2] compositor: check if seteuid worked

2013-09-25 Thread Damian, Alexandru
Funky, I tested the correct patch and then submitted this garbage.

Thanks for spotting this, new patch in the mail.

Alex


On Wed, Sep 25, 2013 at 2:36 PM, Emilio Pozuelo Monfort
poch...@gmail.comwrote:

 Hi,

 On 25/09/13 14:48, Alex DAMIAN wrote:
  From: Alexandru DAMIAN alexandru.dam...@intel.com
 
  Checking the return value from seteuid in
  order to not launch clients with the wrong effective uid.
 
  Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
  ---
   src/compositor.c | 7 +--
   1 file changed, 5 insertions(+), 2 deletions(-)
 
  diff --git a/src/compositor.c b/src/compositor.c
  index bc4837f..2a16f52 100644
  --- a/src/compositor.c
  +++ b/src/compositor.c
  @@ -247,8 +247,11 @@ child_client_exec(int sockfd, const char *path)
sigfillset(allsigs);
sigprocmask(SIG_UNBLOCK, allsigs, NULL);
 
  - /* Launch clients as the user. */
  - seteuid(getuid());
  + /* Launch clients as the user. Do not lauch clients with wrong
 euid.*/
  + if (seteuid(getuid()) -1) {

 Missing == operator; this code won't build as is.

 Emilio

  + weston_log(compositor: failed seteuid\n);
  + return;
  + }
 
/* SOCK_CLOEXEC closes both ends, so we dup the fd to get a
 * non-CLOEXEC fd to pass through exec. */
 




-- 
Alex Damian
Yocto Project
SSG / OTC
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v2] Post buffer release events instead of queue when no frame callback

2013-09-25 Thread Tomeu Vizoso
On 16 September 2013 17:28, Neil Roberts n...@linux.intel.com wrote:


 Here is a second version of the patch to do the posting. It makes the
 patch for doing the queue disabling redundant.


Have done some testing on the RPi and this patch allows me not having to
sync in a loop while waiting for buffer releases in SwapBuffers.

I'm a bit concerned though about applications that install frame callbacks
for reasons other than knowing when to start to draw, such as gathering
timing statistics. I'm also concerned about making things more complicated
and thus harder to debug.

I have seen a flag proposed to enable or disable the queueing of release
events, but I'm not sure what would make most sense to be the default value
of it.

Regards,

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


Re: [PATCH v2] client: Fix handling display-reader_count if poll fails

2013-09-25 Thread Kristian Høgsberg
On Wed, Sep 25, 2013 at 10:39:12AM +0100, Neil Roberts wrote:
 In wl_display_dispatch_queue, if poll fails then it would previously
 return immediately and leak a reference in display-reader_count. Then
 if the application ignores the error and tries to read again it will
 block forever. This can happen for example if the poll fails with
 EINTR which the application might consider to be a recoverable error.
 This patch makes it cancel the read so the reader_count will be
 decremented when poll fails.
 ---
 
 Ok, here is a second version of the patch which calls
 wl_display_cancel_read as you suggested. Thanks for the review.

Thanks for the reminder, we need that in the release.

Kristian

  src/wayland-client.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
 
 diff --git a/src/wayland-client.c b/src/wayland-client.c
 index cad587d..a20a71e 100644
 --- a/src/wayland-client.c
 +++ b/src/wayland-client.c
 @@ -1224,8 +1224,10 @@ wl_display_dispatch_queue(struct wl_display *display,
  
   pfd[0].fd = display-fd;
   pfd[0].events = POLLIN;
 - if (poll(pfd, 1, -1) == -1)
 + if (poll(pfd, 1, -1) == -1) {
 + wl_display_cancel_read(display);
   return -1;
 + }
  
   pthread_mutex_lock(display-mutex);
  
 -- 
 1.8.3.1
 
 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH] config: verify that the config file has been parsed

2013-09-25 Thread Kristian Høgsberg
On Wed, Sep 25, 2013 at 01:34:17PM +0100, Alex DAMIAN wrote:
 From: Alexandru DAMIAN alexandru.dam...@intel.com
 
 weston_config_parse may return NULL, leading
 to an ungraceful exit via SIGSEGV.
 
 Adding a nice check that gracefully exits the compositor
 when the config.ini parsing failed.

Ah, the problem is actually that weston_config_get_full_path()
crashes.  Everything else is designed to work with a NULL config by
returning default values.  Weston should be able to start without a
config file.

I think we just want to log that we didn't find a config file if
weston_config_parse() returns NULL, but when it does return a config
we log the filename like we do now.

Kristian

 Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
 ---
  src/compositor.c | 4 
  1 file changed, 4 insertions(+)
 
 diff --git a/src/compositor.c b/src/compositor.c
 index f619f82..9ba6e88 100644
 --- a/src/compositor.c
 +++ b/src/compositor.c
 @@ -3489,6 +3489,10 @@ int main(int argc, char *argv[])
   }
  
   config = weston_config_parse(weston.ini);
 + if (config == NULL) {
 + weston_log(Could not parse config file weston.ini);
 + exit(EXIT_FAILURE);
 + }
   weston_log(Using config file '%s'\n,
  weston_config_get_full_path(config));
   section = weston_config_get_section(config, core, NULL, NULL);
 -- 
 1.8.1.2
 
 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH 1/3 wayland-fits] core/pointer: Keep track of the focus serial

2013-09-25 Thread Neil Roberts
The focus serial is needed to correctly send a cursor so it is useful
to be able to verify that this is working correctly.
---
 src/test/core/pointer.cpp | 2 ++
 src/test/core/pointer.h   | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/src/test/core/pointer.cpp b/src/test/core/pointer.cpp
index be6f9a9..eb94fd3 100644
--- a/src/test/core/pointer.cpp
+++ b/src/test/core/pointer.cpp
@@ -29,6 +29,7 @@ namespace core {
 Pointer::Pointer(const Seat seat)
: seat_(seat)
, focus_(NULL)
+   , focusSerial_(0)
, x_(-1)
, y_(-1)
, button_(0)
@@ -73,6 +74,7 @@ bool Pointer::hasFocus(wl_surface* surface)
 //  wl_fixed_to_int(y)  std::endl;

pointer-focus_ = wl_surface;
+   pointer-focusSerial_ = serial;
pointer-x_ = wl_fixed_to_int(x);
pointer-y_ = wl_fixed_to_int(y);
 }
diff --git a/src/test/core/pointer.h b/src/test/core/pointer.h
index e891142..3607861 100644
--- a/src/test/core/pointer.h
+++ b/src/test/core/pointer.h
@@ -57,6 +57,7 @@ public:
const uint32_t button() const { return button_; }
const uint32_t buttonState() const { return buttonState_; }
wl_surface* focus() const { return focus_; }
+   const uint32_t focusSerial() const { return focusSerial_; }
 
bool hasFocus(wl_surface*);
 
@@ -76,6 +77,7 @@ private:
 
const Seat seat_;
wl_surface* focus_;
+   uint32_tfocusSerial_;
int32_t x_;
int32_t y_;
uint32_tbutton_;
-- 
1.8.3.1

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


[PATCH 2/3 wayland-fits] core: Add a wrapper for the keyboard

2013-09-25 Thread Neil Roberts
This adds a wrapper for a wl_keyboard in a similar way to the pointer
wrapper. It keeps track of the keys that are pressed so that they can
be quickly verified. wayland-fits now depends on libxkbcommon so that
the keyboard wrapper can pass the keymap to it and get the modifier
indices.
---
 configure.ac   |   2 +
 src/test/Makefile.am   |   2 +
 src/test/core/Makefile.am  |   3 +
 src/test/core/display.cpp  |  13 +++
 src/test/core/display.h|   3 +
 src/test/core/keyboard.cpp | 213 +
 src/test/core/keyboard.h   | 107 +++
 src/test/efl/Makefile.am   |   2 +
 src/test/gtk+/Makefile.am  |   2 +
 9 files changed, 347 insertions(+)
 create mode 100644 src/test/core/keyboard.cpp
 create mode 100644 src/test/core/keyboard.h

diff --git a/configure.ac b/configure.ac
index de84adf..5ad59ca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,6 +61,8 @@ PKG_CHECK_MODULES([WAYLAND],
 )
 PKG_CHECK_MODULES(WAYLAND_SERVER, [wayland-server = wayland_req_version])
 
+PKG_CHECK_MODULES([XKBCOMMON], xkbcommon)
+
 have_weston=no
 want_weston_extensions=auto
 AC_ARG_ENABLE([weston-extensions],
diff --git a/src/test/Makefile.am b/src/test/Makefile.am
index ba5e5ce..e50c0fc 100644
--- a/src/test/Makefile.am
+++ b/src/test/Makefile.am
@@ -14,6 +14,7 @@ wfits_SOURCES =   \
 wfits_LDADD =  \
core/libwfits-core.la   \
$(CHECK_LIBS)   \
+   $(XKBCOMMON_LIBS)   \
$(WAYLAND_LIBS) \
$(BOOST_LIBS)
 
@@ -23,6 +24,7 @@ wfits_LDFLAGS =   \
 wfits_CPPFLAGS =   \
-I$(top_srcdir)/src \
$(CHECK_CFLAGS) \
+   $(XKBCOMMON_CFLAGS) \
$(WAYLAND_CFLAGS)   \
$(BOOST_CPPFLAGS)
 
diff --git a/src/test/core/Makefile.am b/src/test/core/Makefile.am
index bff1c5a..3a93a67 100644
--- a/src/test/core/Makefile.am
+++ b/src/test/core/Makefile.am
@@ -3,6 +3,7 @@ noinst_LTLIBRARIES = libwfits-core.la
 libwfits_core_la_SOURCES = \
display.cpp \
compositor.cpp  \
+   keyboard.cpp\
shell.cpp   \
seat.cpp\
pointer.cpp \
@@ -23,11 +24,13 @@ libwfits_core_la_SOURCES =  \
 
 libwfits_core_la_LIBADD =  \
$(WAYLAND_LIBS) \
+   $(XKBCOMMON_LIBS)   \
$(CHECK_LIBS)
 
 libwfits_core_la_CPPFLAGS =\
-I$(top_srcdir)/src \
$(WAYLAND_CFLAGS)   \
+   $(XKBCOMMON_CFLAGS) \
$(CHECK_CFLAGS)
 
 AM_CXXFLAGS =  \
diff --git a/src/test/core/display.cpp b/src/test/core/display.cpp
index 1c29dbf..5c9a459 100644
--- a/src/test/core/display.cpp
+++ b/src/test/core/display.cpp
@@ -30,6 +30,7 @@ Display::Display()
: wl_display_(wl_display_connect(0))
, wl_registry_(NULL)
, globals_()
+   , xkbContext_(NULL)
 {
ASSERT(wl_display_ != NULL);
 
@@ -49,6 +50,8 @@ Display::Display()
 
 /*virtual*/ Display::~Display()
 {
+   if (xkbContext_)
+   xkb_context_unref(xkbContext_);
wl_registry_destroy(wl_registry_);
wl_display_disconnect(*this);
 }
@@ -69,6 +72,16 @@ void Display::yield(const unsigned time) const
usleep(time);
 }
 
+struct xkb_context *Display::xkbContext() const
+{
+   if (xkbContext_ == NULL) {
+   xkbContext_ = xkb_context_new((enum xkb_context_flags) 0);
+   ASSERT(xkbContext_ != NULL);
+   }
+
+   return xkbContext_;
+}
+
 /*static*/ void Display::global(
void *data, struct wl_registry *wl_registry, uint32_t id,
const char* interface, uint32_t version)
diff --git a/src/test/core/display.h b/src/test/core/display.h
index 0a4c10a..bf09057 100644
--- a/src/test/core/display.h
+++ b/src/test/core/display.h
@@ -25,6 +25,7 @@
 
 #include map
 #include wayland-client.h
+#include xkbcommon/xkbcommon.h
 #include test/tools.h
 
 namespace wfits {
@@ -63,6 +64,7 @@ public:
void roundtrip() const;
void dispatch() const;
void yield(const unsigned = 0.001 * 1e6) const;
+   struct xkb_context *xkbContext() const;
 
operator wl_display*() const { return wl_display_; }
operator wl_registry*() const { return wl_registry_; }
@@ -74,6 +76,7 @@ private:
wl_display  *wl_display_;
wl_registry *wl_registry_;
Globals globals_;
+   mutable struct xkb_context *xkbContext_;
 };
 
 } // namespace core
diff --git a/src/test/core/keyboard.cpp b/src/test/core/keyboard.cpp
new file mode 100644
index 000..aa49413
--- /dev/null
+++ b/src/test/core/keyboard.cpp
@@ -0,0 +1,213 @@
+/*
+ * Copyright © 2013 Intel 

[PATCH 3/3 wayland-fits] core: Add a test for multiple pointer and keyboard resources

2013-09-25 Thread Neil Roberts
This adds a test which creates multiple pointer and keyboard resources
for the same client and verifies that they all receive events. It also
tests various combiniations of pointer and keyboard focus and ensures
that for example a keyboard created while the surface already has
focus will correctly get updated about the state.
---
 src/test/core/Makefile.am |   1 +
 src/test/core/test_multi_resource.cpp | 259 ++
 2 files changed, 260 insertions(+)
 create mode 100644 src/test/core/test_multi_resource.cpp

diff --git a/src/test/core/Makefile.am b/src/test/core/Makefile.am
index 3a93a67..ec7901b 100644
--- a/src/test/core/Makefile.am
+++ b/src/test/core/Makefile.am
@@ -19,6 +19,7 @@ libwfits_core_la_SOURCES =\
test_bind_interface.cpp \
test_cursor.cpp \
test_data.cpp   \
+   test_multi_resource.cpp \
test_surface.cpp\
test_dnd.cpp
 
diff --git a/src/test/core/test_multi_resource.cpp 
b/src/test/core/test_multi_resource.cpp
new file mode 100644
index 000..d3fb003
--- /dev/null
+++ b/src/test/core/test_multi_resource.cpp
@@ -0,0 +1,259 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * 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.
+ */
+
+#include harness.h
+#include compositor.h
+#include pointer.h
+#include keyboard.h
+#include seat.h
+#include surface.h
+#include shell.h
+#include shell_surface.h
+#include shm.h
+
+namespace wfits {
+namespace test {
+namespace core {
+namespace input {
+
+class DummyClient
+{
+public:
+   DummyClient()
+   : display_()
+   , compositor_(display_)
+   , shell_(display_)
+   , seat_(display_)
+   , surface_(compositor_)
+   , shellSurface_(shell_, surface_)
+   , shm_(display_)
+   , buffer_(shm_, 128, 128)
+   {
+   wl_surface_attach(surface_, buffer_, 0, 0);
+   wl_surface_damage(surface_, 0, 0,
+ buffer_.width(), buffer_.height());
+   surface_.commit();
+   }
+
+private:
+   Display display_;
+   Compositor compositor_;
+   Shell shell_;
+   Seat seat_;
+   Surface surface_;
+   ShellSurface shellSurface_;
+   SharedMemory shm_;
+   SharedMemoryBuffer buffer_;
+};
+
+class MultiResourceTest : public Harness
+{
+public:
+   MultiResourceTest()
+   : Harness::Harness()
+   , compositor_(display())
+   , shell_(display())
+   , seat_(display())
+   , surface_(compositor_)
+   , shellSurface_(shell_, surface_)
+   , shm_(display())
+   , buffer_(shm_, 128, 128)
+   {
+   }
+
+   void setup();
+   void testPointer();
+   void testKeyboard();
+
+   void movePointer(const int32_t x, const int32_t y)
+   {
+   Geometry geometry(getSurfaceGeometry(surface_));
+   setGlobalPointerPosition(geometry.x + x, geometry.y + y);
+   }
+
+   void checkPointer(Pointer *pointer, const int32_t x, const int32_t y)
+   {
+   YIELD_UNTIL(pointer-x() == x and pointer-y() == y);
+   }
+
+private:
+   Compositor compositor_;
+   Shell shell_;
+   Seat seat_;
+   Surface surface_;
+   ShellSurface shellSurface_;
+   SharedMemory shm_;
+   SharedMemoryBuffer buffer_;
+};
+
+void MultiResourceTest::setup()
+{
+   wl_surface_attach(surface_, buffer_, 0, 0);
+   wl_surface_damage(surface_, 0, 0, buffer_.width(), buffer_.height());
+   surface_.commit();
+
+   queueStep(boost::bind(MultiResourceTest::testPointer, 
boost::ref(*this)));
+   queueStep(boost::bind(MultiResourceTest::testKeyboard, 
boost::ref(*this)));
+}
+

[PATCH v3 1/2] config: verify that the config file is not null

2013-09-25 Thread Alex DAMIAN
From: Alexandru DAMIAN alexandru.dam...@intel.com

weston_config_parse may return NULL,
leading to an ungraceful exit via SIGSEGV if we
try to reference the structure.

Adding a check in weston_config_full_path so that
we return the empty file /dev/null as filename
if we started without a config file.

Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
---
 shared/config-parser.c | 2 +-
 src/compositor.c   | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/shared/config-parser.c b/shared/config-parser.c
index e1bf212..ef5c5b9 100644
--- a/shared/config-parser.c
+++ b/shared/config-parser.c
@@ -385,7 +385,7 @@ weston_config_parse(const char *name)
 const char *
 weston_config_get_full_path(struct weston_config *config)
 {
-   return config-path;
+   return config == NULL ? /dev/null : config-path;
 }
 
 int
1.8.1.2

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


Re: [xkbcommon] How to distinguish left-shift and right-shift

2013-09-25 Thread Ran Benita
On Tue, Sep 24, 2013 at 08:02:30PM -0300, Wander Lairson Costa wrote:
 Hi,
 

Hi,

 I am working for some time porting Blender to wayland [1] and I am now
 adding keyboard handing support. For that, I am following weston
 clients code as reference and using libxkbcommon.
 
 To make a long history short, I need to differentiate left-shift and
 right-shift modifiers, but I didn't figure out how to do that with
 libxkbcommon. I printed all modifiers names and there is just one
 Shift modifier, there aren't LShift and RShift like there are
 for control and alt key modifiers.

I'm afraid we have no way to do that. We currently only expose the 8 old
X11 modifiers, which only have Shift. There are also what's called
virtual modifiers, which I have some pending work to expose, but these
*too* don't have separate RShift and LShift modifiers (as opposed to
e.g. LControl and RControl) - that's in current xkeyboard-config, which
is the project where all the keyboard data is maintained. And there was
some sensible resistence to addings these, for example here:
https://bugs.freedesktop.org/show_bug.cgi?id=50935#c18

Of course, X11/XKB is in exactly the same boat here, so I from a quick
look I found that what the blender X11 backend does (as far as I can
see), is that it gets the key state with the XQueryKeymap(3) request,
and then sees if the left/right *keys* are down. That's not entirely
correct purely speaking, but I guess it works in 99.% percent of the
cases. So I can only suggest you do the same for now for Wayland.

But... xkbcommon doesn't (and prefereably won't) expose an equivalent of
XQueryKeymap itself, and the Wayland protocol only provides it on the
wl_keyboard enter event (with the 'keys' field). So unfortunatly what
you have do is get/reset this array on wl_keyboard enter event, and
keep it updated manually on the client side on key events. That should
work... Unless someone has a better idea!

Sorry!
Ran

 
 [1] https://github.com/walac/blender-wayland
 
 -- 
 Best Regards,
 Wander Lairson Costa
 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v3 1/2] config: verify that the config file is not null

2013-09-25 Thread Bill Spitzak
Do you think anything is really going to try to open and read the file, 
and then crash because it does not exist? If this is a concern then you 
also have to worry about the config file being deleted after Wayland 
starts up.


I think returning  or some other non-existent file (perhaps the 
default path+name for a config file) would work.


Alex DAMIAN wrote:

From: Alexandru DAMIAN alexandru.dam...@intel.com

weston_config_parse may return NULL,
leading to an ungraceful exit via SIGSEGV if we
try to reference the structure.

Adding a check in weston_config_full_path so that
we return the empty file /dev/null as filename
if we started without a config file.

Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
---
 shared/config-parser.c | 2 +-
 src/compositor.c   | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/shared/config-parser.c b/shared/config-parser.c
index e1bf212..ef5c5b9 100644
--- a/shared/config-parser.c
+++ b/shared/config-parser.c
@@ -385,7 +385,7 @@ weston_config_parse(const char *name)
 const char *
 weston_config_get_full_path(struct weston_config *config)
 {
-   return config-path;
+   return config == NULL ? /dev/null : config-path;
 }
 
 int

1.8.1.2

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

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


Re: [PATCH v3 1/2] config: verify that the config file is not null

2013-09-25 Thread Kristian Høgsberg
On Wed, Sep 25, 2013 at 11:07 AM, Alex DAMIAN
alexandru.dam...@intel.com wrote:
 From: Alexandru DAMIAN alexandru.dam...@intel.com

 weston_config_parse may return NULL,
 leading to an ungraceful exit via SIGSEGV if we
 try to reference the structure.

 Adding a check in weston_config_full_path so that
 we return the empty file /dev/null as filename
 if we started without a config file.

I think it's ok for weston_config_get_full_path() to crash if passed a
NULL pointer.  Returning /dev/null or anything else is a little odd.
Maybe returning NULL would be ok, but I think we can just have an if
statement and log No config file found if config is NULL, and log
the file name if we did get a config.

Kristian

 Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
 ---
  shared/config-parser.c | 2 +-
  src/compositor.c   | 1 +
  2 files changed, 2 insertions(+), 1 deletion(-)

 diff --git a/shared/config-parser.c b/shared/config-parser.c
 index e1bf212..ef5c5b9 100644
 --- a/shared/config-parser.c
 +++ b/shared/config-parser.c
 @@ -385,7 +385,7 @@ weston_config_parse(const char *name)
  const char *
  weston_config_get_full_path(struct weston_config *config)
  {
 -   return config-path;
 +   return config == NULL ? /dev/null : config-path;
  }

  int
 1.8.1.2

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


RE: [PATCH 2/3 wayland-fits] core: Add a wrapper for the keyboard

2013-09-25 Thread Eoff, Ullysses A
Comments inline.

 -Original Message-
 From: wayland-devel-bounces+ullysses.a.eoff=intel@lists.freedesktop.org 
 [mailto:wayland-devel-
 bounces+ullysses.a.eoff=intel@lists.freedesktop.org] On Behalf Of Neil 
 Roberts
 Sent: Wednesday, September 25, 2013 11:07 AM
 To: wayland-devel@lists.freedesktop.org
 Subject: [PATCH 2/3 wayland-fits] core: Add a wrapper for the keyboard
 
 This adds a wrapper for a wl_keyboard in a similar way to the pointer
 wrapper. It keeps track of the keys that are pressed so that they can
 be quickly verified. wayland-fits now depends on libxkbcommon so that
 the keyboard wrapper can pass the keymap to it and get the modifier
 indices.
 ---
  configure.ac   |   2 +
  src/test/Makefile.am   |   2 +
  src/test/core/Makefile.am  |   3 +
  src/test/core/display.cpp  |  13 +++
  src/test/core/display.h|   3 +
  src/test/core/keyboard.cpp | 213 
 +
  src/test/core/keyboard.h   | 107 +++
  src/test/efl/Makefile.am   |   2 +
  src/test/gtk+/Makefile.am  |   2 +
  9 files changed, 347 insertions(+)
  create mode 100644 src/test/core/keyboard.cpp
  create mode 100644 src/test/core/keyboard.h
 
 diff --git a/configure.ac b/configure.ac
 index de84adf..5ad59ca 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -61,6 +61,8 @@ PKG_CHECK_MODULES([WAYLAND],
  )
  PKG_CHECK_MODULES(WAYLAND_SERVER, [wayland-server = wayland_req_version])
 
 +PKG_CHECK_MODULES([XKBCOMMON], xkbcommon)
 +
  have_weston=no
  want_weston_extensions=auto
  AC_ARG_ENABLE([weston-extensions],
 diff --git a/src/test/Makefile.am b/src/test/Makefile.am
 index ba5e5ce..e50c0fc 100644
 --- a/src/test/Makefile.am
 +++ b/src/test/Makefile.am
 @@ -14,6 +14,7 @@ wfits_SOURCES = \
  wfits_LDADD =\
   core/libwfits-core.la   \
   $(CHECK_LIBS)   \
 + $(XKBCOMMON_LIBS)   \
   $(WAYLAND_LIBS) \
   $(BOOST_LIBS)
 
 @@ -23,6 +24,7 @@ wfits_LDFLAGS = \
  wfits_CPPFLAGS = \
   -I$(top_srcdir)/src \
   $(CHECK_CFLAGS) \
 + $(XKBCOMMON_CFLAGS) \
   $(WAYLAND_CFLAGS)   \
   $(BOOST_CPPFLAGS)
 
 diff --git a/src/test/core/Makefile.am b/src/test/core/Makefile.am
 index bff1c5a..3a93a67 100644
 --- a/src/test/core/Makefile.am
 +++ b/src/test/core/Makefile.am
 @@ -3,6 +3,7 @@ noinst_LTLIBRARIES = libwfits-core.la
  libwfits_core_la_SOURCES =   \
   display.cpp \
   compositor.cpp  \
 + keyboard.cpp\
   shell.cpp   \
   seat.cpp\
   pointer.cpp \
 @@ -23,11 +24,13 @@ libwfits_core_la_SOURCES =\
 
  libwfits_core_la_LIBADD =\
   $(WAYLAND_LIBS) \
 + $(XKBCOMMON_LIBS)   \
   $(CHECK_LIBS)
 
  libwfits_core_la_CPPFLAGS =  \
   -I$(top_srcdir)/src \
   $(WAYLAND_CFLAGS)   \
 + $(XKBCOMMON_CFLAGS) \
   $(CHECK_CFLAGS)
 
  AM_CXXFLAGS =\
 diff --git a/src/test/core/display.cpp b/src/test/core/display.cpp
 index 1c29dbf..5c9a459 100644
 --- a/src/test/core/display.cpp
 +++ b/src/test/core/display.cpp
 @@ -30,6 +30,7 @@ Display::Display()
   : wl_display_(wl_display_connect(0))
   , wl_registry_(NULL)
   , globals_()
 + , xkbContext_(NULL)
  {
   ASSERT(wl_display_ != NULL);
 
 @@ -49,6 +50,8 @@ Display::Display()
 
  /*virtual*/ Display::~Display()
  {
 + if (xkbContext_)
 + xkb_context_unref(xkbContext_);
   wl_registry_destroy(wl_registry_);
   wl_display_disconnect(*this);
  }
 @@ -69,6 +72,16 @@ void Display::yield(const unsigned time) const
   usleep(time);
  }
 
 +struct xkb_context *Display::xkbContext() const
 +{
 + if (xkbContext_ == NULL) {
 + xkbContext_ = xkb_context_new((enum xkb_context_flags) 0);
 + ASSERT(xkbContext_ != NULL);
 + }
 +
 + return xkbContext_;
 +}
 +
  /*static*/ void Display::global(
   void *data, struct wl_registry *wl_registry, uint32_t id,
   const char* interface, uint32_t version)
 diff --git a/src/test/core/display.h b/src/test/core/display.h
 index 0a4c10a..bf09057 100644
 --- a/src/test/core/display.h
 +++ b/src/test/core/display.h
 @@ -25,6 +25,7 @@
 
  #include map
  #include wayland-client.h
 +#include xkbcommon/xkbcommon.h
  #include test/tools.h
 
  namespace wfits {
 @@ -63,6 +64,7 @@ public:
   void roundtrip() const;
   void dispatch() const;
   void yield(const unsigned = 0.001 * 1e6) const;
 + struct xkb_context *xkbContext() const;
 
   operator wl_display*() const { return wl_display_; }
   operator wl_registry*() const { return 

Re: [PATCH v3 1/2] config: verify that the config file is not null

2013-09-25 Thread Damian, Alexandru
The nice thing was that even if some other code besides the _log tries to
read the file path, that code can expect to open the path and read from it
just fine, instead of receiving a NULL and maybe crashing.

I'm gonna check tomorrow if this happens somewhere else in the code, and if
not, modify _full_path to return NULL and change the log message.


On Wed, Sep 25, 2013 at 9:43 PM, Kristian Høgsberg k...@bitplanet.netwrote:

 On Wed, Sep 25, 2013 at 11:07 AM, Alex DAMIAN
 alexandru.dam...@intel.com wrote:
  From: Alexandru DAMIAN alexandru.dam...@intel.com
 
  weston_config_parse may return NULL,
  leading to an ungraceful exit via SIGSEGV if we
  try to reference the structure.
 
  Adding a check in weston_config_full_path so that
  we return the empty file /dev/null as filename
  if we started without a config file.

 I think it's ok for weston_config_get_full_path() to crash if passed a
 NULL pointer.  Returning /dev/null or anything else is a little odd.
 Maybe returning NULL would be ok, but I think we can just have an if
 statement and log No config file found if config is NULL, and log
 the file name if we did get a config.

 Kristian

  Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com
  ---
   shared/config-parser.c | 2 +-
   src/compositor.c   | 1 +
   2 files changed, 2 insertions(+), 1 deletion(-)
 
  diff --git a/shared/config-parser.c b/shared/config-parser.c
  index e1bf212..ef5c5b9 100644
  --- a/shared/config-parser.c
  +++ b/shared/config-parser.c
  @@ -385,7 +385,7 @@ weston_config_parse(const char *name)
   const char *
   weston_config_get_full_path(struct weston_config *config)
   {
  -   return config-path;
  +   return config == NULL ? /dev/null : config-path;
   }
 
   int
  1.8.1.2
 
  ___
  wayland-devel mailing list
  wayland-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/wayland-devel




-- 
Alex Damian
Yocto Project
SSG / OTC
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel