[PATCH v2] client: Fix handling display-reader_count if poll fails
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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