Busy loop in the wl_priv_signal_final_emit
Hi,
I'm moving my compositor to latest libweston 5.0 /wayland 1.16 library and
I noticed a regression when I terminate my compositor.
The compositor will never exit because is stuck in a busy loop.
The busy loop is in the "wl_priv_signal_final_emit" that was added in the
latest wayland.
And precisely is looping to notify that the same client is disconnected.
With this code (from here:
https://github.com/wayland-project/wayland/blame/bb1a8ca91e7d99f54b43ece01674ccbd720ec4bd/src/wayland-server.c#L2057
)
while (!wl_list_empty(>listener_list)) {
pos = signal->listener_list.next;
l = wl_container_of(pos, l, link);
wl_list_remove(pos);
wl_list_init(pos);
l->notify(l, data);
}
The listener_list is pointing to itself
$3 = {listener_list = {prev = 0x24f7eb0, next = 0x24f7eb0}, emit_list =
{prev = 0x298f888, next = 0x298f888}}
But to be empty next should point to _list that it is
p &$3->listener_list
$11 = (struct wl_list *) 0x298f878
A more expert eye on this codebase can confirm that there is no issue on
how the wl_list_remove is done in wl_priv_signal_final_emit?
Best
Matteo Valdina
--
“There are two ways of constructing a software design: One way is to make
it so simple that there are obviously no deficiencies, and the other way is
to make it so complicated that there are no obvious deficiencies. The first
method is far more difficult.”
- Tony Hoare
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[Patch v4][weston] gl-renderer.c: Use gr->egl_config to create pbuffer surface
The original implementation always chose first egl config for pbuffer
surface type, however the returned configs are implementation specific
and egl config may not always match between ctx and surface. Hence,
use gr->egl_config which already has the matching config but ensure that
windows and pbuffer bit are set for the surface type.
Signed-off-by: Madhurkiran Harikrishnan
---
libweston/gl-renderer.c | 31 +++
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/libweston/gl-renderer.c b/libweston/gl-renderer.c
index da29b07..13d7707 100644
--- a/libweston/gl-renderer.c
+++ b/libweston/gl-renderer.c
@@ -3033,7 +3033,7 @@ gl_renderer_setup_egl_extensions(struct weston_compositor
*ec)
}
static const EGLint gl_renderer_opaque_attribs[] = {
- EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
+ EGL_SURFACE_TYPE, EGL_WINDOW_BIT | EGL_PBUFFER_BIT,
EGL_RED_SIZE, 1,
EGL_GREEN_SIZE, 1,
EGL_BLUE_SIZE, 1,
@@ -3043,7 +3043,7 @@ static const EGLint gl_renderer_opaque_attribs[] = {
};
static const EGLint gl_renderer_alpha_attribs[] = {
- EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
+ EGL_SURFACE_TYPE, EGL_WINDOW_BIT | EGL_PBUFFER_BIT,
EGL_RED_SIZE, 1,
EGL_GREEN_SIZE, 1,
EGL_BLUE_SIZE, 1,
@@ -3146,17 +3146,7 @@ output_handle_destroy(struct wl_listener *listener, void
*data)
static int
gl_renderer_create_pbuffer_surface(struct gl_renderer *gr) {
- EGLConfig pbuffer_config;
-
- static const EGLint pbuffer_config_attribs[] = {
- EGL_SURFACE_TYPE, EGL_PBUFFER_BIT,
- EGL_RED_SIZE, 1,
- EGL_GREEN_SIZE, 1,
- EGL_BLUE_SIZE, 1,
- EGL_ALPHA_SIZE, 0,
- EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
- EGL_NONE
- };
+ EGLint surface_type;
static const EGLint pbuffer_attribs[] = {
EGL_WIDTH, 10,
@@ -3164,13 +3154,22 @@ gl_renderer_create_pbuffer_surface(struct gl_renderer
*gr) {
EGL_NONE
};
- if (egl_choose_config(gr, pbuffer_config_attribs, NULL, 0,
_config) < 0) {
- weston_log("failed to choose EGL config for PbufferSurface\n");
+
+ if(!eglGetConfigAttrib(gr->egl_display, gr->egl_config,
EGL_SURFACE_TYPE, _type)) {
+ weston_log("failed to get surface type for PbufferSurface\n");
+ return -1;
+ }
+
+ if (!((surface_type & EGL_WINDOW_BIT) && (surface_type &
EGL_PBUFFER_BIT)) &&
+ !gr->has_configless_context) {
+ weston_log("attempted to use a different EGL config for an "
+ "output but EGL_KHR_no_config_context or "
+ "EGL_MESA_configless_context is not supported\n");
return -1;
}
gr->dummy_surface = eglCreatePbufferSurface(gr->egl_display,
- pbuffer_config,
+ gr->egl_config,
pbuffer_attribs);
if (gr->dummy_surface == EGL_NO_SURFACE) {
--
2.7.4
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v4 1/5] build/doc: Ensure destination dir exists despite VPATH
Hi Dan, On 28 August 2018 at 23:19, Daniel Stone wrote: > Make considers a variable called VPATH when trying to satisfy > dependencies, e.g. for a target 'foo', it will consider the target > extant if VPATH is '../../bar' and '../../bar/foo' exists. > > Part of the doc build, the '$(alldirs)' target, exists to create the > target directories if they do not exist. For example, before generating > xml/wayland-architecture.png, it will ensure the 'xml' target is > considered up-to-date thanks to the target dependency. > > Creating $(srcdir)/doc/doxygen/xml thus means that the 'xml' dependency > will be satisfied, so we'll never create the output directory, and the > doc build will fail. > > Change the alldirs target list to be absolute paths, so VPATH will not > be consulted and defeat the entire point of what we're trying to do. > This fixes the Meson build, where we later create > doc/doxygen/xml/meson.build. > Have you tried something as trivial as the below sed? s/$(AM_V_GEN)/$(AM_V_GEN)$(MKDIR_P) $@/g It will allow you to remove the, dare I say it, bonkers "let's make a target that only creates a folder". Plus avoid all the complexity that you've proposing. -Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 6/6] tests: Remove memory leak checking infrastructure
On 29 August 2018 at 07:17, Daniel Stone wrote: > There are far better ways to detect memory leaks, such as either > valgrind or ASan. Having Meson makes it really easy to use these tools > in our tests, and we can do that in CI as well. > > Having these local wrappers actually completely broke ASan usage, so > remove them in favour of using the more powerful options. > Fwiw adding valgrind to any build-system is a 2-5 line task ;-) Sanitizers on the other hand (esp. on shared libraries) is a pain. -Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 5/6] tests: Overly elaborate compiler warning workaround
Hi Dan, On 29 August 2018 at 07:17, Daniel Stone wrote: > Clang will rightly point out that example_sockaddr_un in socket-test > will get discarded from the compilation unit as it is completely unused. > Put in a couple of lines which of no value other than stopping Clang > from complaining. > > Signed-off-by: Daniel Stone > --- > tests/socket-test.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/tests/socket-test.c b/tests/socket-test.c > index e9705628..8d39edce 100644 > --- a/tests/socket-test.c > +++ b/tests/socket-test.c > @@ -42,7 +42,7 @@ > * See `man 7 unix`. > */ > > -static const struct sockaddr_un example_sockaddr_un; > +static struct sockaddr_un example_sockaddr_un; > > #define TOO_LONG (1 + sizeof example_sockaddr_un.sun_path) > > @@ -69,6 +69,11 @@ TEST(socket_path_overflow_client_connect) > d = wl_display_connect(path); > assert(d == NULL); > assert(errno == ENAMETOOLONG); > + > + /* This is useless, but prevents a warning about example_sockaddr_un > +* being discarded from the compilation unit. */ > + strcpy(example_sockaddr_un.sun_path, "happy now clang?"); > + assert(example_sockaddr_un.sun_path[0] != '\0'); Why don't you add _attrubute__((used)) instead of doing the blame game? We already use it in the repo. Side note: the manpage says "sun_path[108]" and also points out that "some implementations have sun_path as short as 92 bytes" The check in wl_socket_init_for_display_name uses sizeof, yet prints a "exceeds 108 bytes" message. I guess the message should be fixed? HTH Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v4 3/5] Add Meson build to Wayland
On Tue, 28 Aug 2018 23:19:17 +0100
Daniel Stone wrote:
> From: Emmanuele Bassi
>
> Meson is a next generation build system, simpler than Autotools and,
> more importantly, faster and more portable. While the latter
> consideration is of lesser importance for Wayland, being easier to
> understand and faster are pretty much key reasons to switch.
>
> This is mostly a mechanical port of Wayland to the Meson build system.
>
> The goal is to maintain feature parity of the Meson build with the
> Autotools build, until such time when we can drop the latter.
>
> [daniels: Changed to bump version, use GitLab issues URL, remove header
> checks not used in any code, remove pre-pkg-config Expat
> support, added missing include paths to wayland-egl and
> cpp-compile-test, added GitLab CI.
> Bumped version, removed unnecessary pkg-config paths.]
>
> Reviewed-by: Daniel Stone
> Acked-by: Emre Ucan
> Acked-by: Peter Hutterer
> ---
> .gitlab-ci.yml | 24 +-
> cursor/meson.build | 29 +++
> doc/meson.build| 5 ++
> egl/meson.build| 43 ++
> meson.build| 94 +
> meson_options.txt | 20 +
> src/meson.build| 200 +
> tests/meson.build | 180
> 8 files changed, 593 insertions(+), 2 deletions(-)
> create mode 100644 cursor/meson.build
> create mode 100644 doc/meson.build
> create mode 100644 egl/meson.build
> create mode 100644 meson.build
> create mode 100644 meson_options.txt
> create mode 100644 src/meson.build
> create mode 100644 tests/meson.build
Hi Daniel,
I see that the doc stuff comes as follow-up. This all looks quite good,
but I had a bunch of comments below.
> diff --git a/egl/meson.build b/egl/meson.build
> new file mode 100644
> index ..5ae9805e
> --- /dev/null
> +++ b/egl/meson.build
> @@ -0,0 +1,43 @@
> +wayland_egl = library(
> + 'wayland-egl',
> + sources: [
> +'wayland-egl.c',
> +wayland_client_protocol_h,
> + ],
> + include_directories: src_inc,
> + version: '1.0.0',
> + install: true,
> +)
> +
> +executable('wayland-egl-abi-check', 'wayland-egl-abi-check.c')
> +
> +nm_path = find_program('nm').path()
> +
> +test(
> + 'wayland-egl symbols check',
> + find_program('wayland-egl-symbols-check'),
> + env: [
> +'WAYLAND_EGL_LIB=@0@'.format(wayland_egl.full_path()),
> +'NM=@0@'.format(nm_path),
> + ],
> +)
Oddly enough, autotools build does run wayland-egl-abi-check during
'make check'. It comes via the variable built_test_programs. It is
explicitly assigned to check_PROGRAMS which are not run by default, so
that probably confused.
The Meson build does not run it, but I think it should.
> +
> +install_headers([
> + 'wayland-egl.h',
> + 'wayland-egl-core.h',
> + 'wayland-egl-backend.h',
> +])
> +
> +pkgconfig.generate(
> + name: 'wayland-egl',
> + description: 'Frontend wayland-egl library',
> + version: '18.1.0',
> + requires: 'wayland-client',
> + libraries: wayland_egl,
> +)
> +
> +pkgconfig.generate(
> + name: 'wayland-egl-backend',
> + description: 'Backend wayland-egl library',
This should be "interface", not "library", to match autotools build.
> + version: '3',
> +)
> diff --git a/meson.build b/meson.build
> new file mode 100644
> index ..6d90625a
> --- /dev/null
> +++ b/meson.build
> +
> +if get_option('libraries')
> + ffi_dep = dependency('libffi')
> +
> + decls = [
> +[ 'sys/signalfd.h', 'SFD_CLOEXEC' ],
> +[ 'sys/timerfd.h', 'TFD_CLOEXEC' ],
> +[ 'time.h', 'CLOCK_MONOTONIC' ],
> + ]
> +
> + foreach d: decls
> +if not cc.has_header_symbol(d[0], d[1])
> + error('@0@ is needed to compile Wayland libraries'.format(d[1]))
> +endif
> + endforeach
> +endif
> +
> +expat_dep = dependency('expat', required: false)
If the scanner gets built, how will we get a nice dependency failure
for expat?
Or, if we do get that failure already, then...
> +
> +if get_option('dtd_validation')
> + libxml2_dep = dependency('libxml-2.0')
...there is no way to build without libxml too, if dtd_validation
is disabled, because src/meson.build has:
+dependencies: [ expat_dep, libxml2_dep, wayland_util_dep, ],
> + config_h.set('HAVE_LIBXML', 1)
> +endif
> +
> diff --git a/src/meson.build b/src/meson.build
> new file mode 100644
> index ..388ccebe
> --- /dev/null
> +++ b/src/meson.build
> +
> + pkgconfig.generate(
> +name: 'Wayland Scanner',
> +description: 'Wayland scanner',
> +version: meson.project_version(),
> +variables: [
> + 'bindir=${prefix}/@0@'.format(get_option('bindir')),
> + 'datarootdir=${prefix}/@0@'.format(get_option('datadir')),
> + 'pkgdatadir=${prefix}/@0@/@1@'.format(get_option('datadir'),
> meson.project_name()),
> + 'wayland_scanner=${bindir}/wayland-scanner',
> +],
> +filebase: 'wayland-scanner',
> + )
> +endif
> +
> +if get_option('libraries')
>
[PATCH wayland-protocols v4] unstable/drm-lease: DRM lease protocol support
Simple protocol extension to manage DRM lease. Based on the work by Keith Packard in [1], respectively [2]. [1] https://cgit.freedesktop.org/mesa/drm/commit/?id=c4171535389d72e9135c9615cecd07b346fd6d7e [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.15-rc9=62884cd386b876638720ef88374b31a84ca7ee5f Changes since v3: - instead of advertising all leasable connectors at once, advertise each connector for each leasable output. Use an object to represent a leasable connector and provide a request to add that leasable connector when creating the lease (Philipp Zabel) - add two events to handle add_connector request - add some comments in the manager interface to explain how the protocol can/should be used Changes since v2: - advertise connectors when creating a lease request object (Pekka Paalanen) - when revoking the lease use the lease object instead of lessee id (Pekka Paalanen) - various grammar/spelling fixes (Pekka Paalanen) Changes since v1: - added manager: advertise lease capability and manage the lease (Daniel Stone) - add request(s) for adding connector/crtc/plane to behave more like dmabuf (Daniel Stone) Signed-off-by: Marius Vlad --- A (rather incomplete) implementation for this version can be found at https://gitlab.freedesktop.org/marius.vlad0/weston/tree/drm-lease-advertise-each-connnector-object One interesting question is how to handle the situation when the client deliberately, or not, holds the lease indefinitely. This has multiple ramification like what happens when the client un-expectingly dies and doesn't revoking the lease, or when hot-plugging the connector and weston tries to get a hold of a connector previously leased? It could be there might be other situations where the compositor will need to revoke the lease. In the implementation we could deny the compositor to get a hold of the connector when hot-plugging that output, if that's desirable, and presumably establish a way to detect periodically if the lease is still in use. Alternatively, shouldn't we use something like a ping/pong approach in which the client (in rendering part), sends periodically an alive signal? Makefile.am | 1 + unstable/drm-lease/README| 4 + unstable/drm-lease/drm-lease-unstable-v1.xml | 244 +++ 3 files changed, 249 insertions(+) create mode 100644 unstable/drm-lease/README create mode 100644 unstable/drm-lease/drm-lease-unstable-v1.xml diff --git a/Makefile.am b/Makefile.am index 6394e26..5d13dca 100644 --- a/Makefile.am +++ b/Makefile.am @@ -4,6 +4,7 @@ unstable_protocols = \ unstable/pointer-gestures/pointer-gestures-unstable-v1.xml \ unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml \ unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml \ + unstable/drm-lease/drm-lease-unstable-v1.xml \ unstable/text-input/text-input-unstable-v1.xml \ unstable/text-input/text-input-unstable-v3.xml \ unstable/input-method/input-method-unstable-v1.xml \ diff --git a/unstable/drm-lease/README b/unstable/drm-lease/README new file mode 100644 index 000..a25600c --- /dev/null +++ b/unstable/drm-lease/README @@ -0,0 +1,4 @@ +Linux DRM lease + +Maintainers: +Marius Vlad diff --git a/unstable/drm-lease/drm-lease-unstable-v1.xml b/unstable/drm-lease/drm-lease-unstable-v1.xml new file mode 100644 index 000..40eedb4 --- /dev/null +++ b/unstable/drm-lease/drm-lease-unstable-v1.xml @@ -0,0 +1,244 @@ + + + + +Copyright 2018 NXP + +Permission is hereby granted, free of charge, to any person obtaining a +copy of this software and associated documentation files (the "Software"), +to deal in the Software without restriction, including without limitation +the rights to use, copy, modify, merge, publish, distribute, sublicense, +and/or sell copies of the Software, and to permit persons to whom the +Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice (including the next +paragraph) shall be included in all copies or substantial portions of the +Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL +THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +DEALINGS IN THE SOFTWARE. + + + + + This interface makes use of DRM lease written by
Re: [PATCH wayland v4 2/5] Support running tests from different build directories
On Tue, 28 Aug 2018 23:19:16 +0100
Daniel Stone wrote:
> From: Emmanuele Bassi
>
> The tests that run exec-fd-leak-checker expect the binary to be located
> in the current directory. This is not always the case; for instance, the
> binaries could be built under `tests`, but be invoked under the
> top-level build directory.
Hi,
saying Meson will need this would have been nice.
>
> We can use an environment variable to control what's the location of the
> test binaries, and fall back to the current directory if the variable is
> unset.
>
> Reviewed-by: Daniel Stone
> ---
> tests/test-helpers.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/tests/test-helpers.c b/tests/test-helpers.c
> index b2189d8e..20b66903 100644
> --- a/tests/test-helpers.c
> +++ b/tests/test-helpers.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -67,11 +68,19 @@ count_open_fds(void)
> void
> exec_fd_leak_check(int nr_expected_fds)
> {
> - const char *exe = "./exec-fd-leak-checker";
> + const char *exe = "exec-fd-leak-checker";
> char number[16] = { 0 };
> + const char *test_build_dir = getenv("TEST_BUILD_DIR");
> + char exe_path[256] = { 0 };
256 bytes is very short for various build systems that really like long
paths
like
/var/tmp/buildbuilder/packages/dev-lib/wayland/wayland-1.123/tmp/work/install-prefix/sysroot/tmp/build/...
Use PATH_MAX instead?
Otherwise looks good to me, so
Reviewed-by: Pekka Paalanen
Thanks,
pq
> +
> + if (test_build_dir == NULL || test_build_dir[0] == 0) {
> + test_build_dir = ".";
> + }
> +
> + snprintf(exe_path, sizeof exe_path - 1, "%s/%s", test_build_dir, exe);
>
> snprintf(number, sizeof number - 1, "%d", nr_expected_fds);
> - execl(exe, exe, number, (char *)NULL);
> + execl(exe_path, exe, number, (char *)NULL);
> assert(0 && "execing fd leak checker failed");
> }
>
pgpa_Eo7SYYPy.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 0/6] Minor test/scanner fixes
On Wed, 29 Aug 2018 at 09:51, Pekka Paalanen wrote: > On Wed, 29 Aug 2018 07:17:09 +0100 Daniel Stone wrote: > > These first 5 patches fix some issues I found by running the test suite > > under an expanded set of toolchains, including the ASan address > > sanitiser and Clang's static analyser. > > > > Patch 6 removes the leak checking from the test suite completely. > > Previously due to toolchain immaturity, it was really difficult to run > > the test suite with checks for memory leaks, so we rolled our own. On > > the other hand, with Meson we can now just pass '-Db_sanitize=address' > > to the configure process, or run 'meson test --wrapper=valgrind', to get > > more powerful and useful checkers. > > > > The existing leak checker we have breaks ASan completely, and I couldn't > > figure out how to fix it. Removing it altogether seemed like a better > > idea. > > > > I've implemented this for GitLab CI, and you can see example output > > here: https://gitlab.freedesktop.org/daniels/wayland/pipelines/3663 > > > > This issue tracks the work left on the CI pipeline to get everything > > upstream: > > https://gitlab.freedesktop.org/wayland/wayland/issues/54 > > > > If anyone wants to help out, please feel free to grab that branch and > > run with it; in the meantime, these seem like good fixes to have > > regardless. > > I'm thrilled where this is going. This series is: > Reviewed-by: Pekka Paalanen > > Patch 3 could maybe use a code comment for being so subtle. Great, I've added that now and pushed the series after a run through CI. Thanks both for the review! Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v4 1/5] build/doc: Ensure destination dir exists despite VPATH
On Tue, 28 Aug 2018 23:19:15 +0100 Daniel Stone wrote: > Make considers a variable called VPATH when trying to satisfy > dependencies, e.g. for a target 'foo', it will consider the target > extant if VPATH is '../../bar' and '../../bar/foo' exists. > > Part of the doc build, the '$(alldirs)' target, exists to create the > target directories if they do not exist. For example, before generating > xml/wayland-architecture.png, it will ensure the 'xml' target is > considered up-to-date thanks to the target dependency. > > Creating $(srcdir)/doc/doxygen/xml thus means that the 'xml' dependency > will be satisfied, so we'll never create the output directory, and the > doc build will fail. > > Change the alldirs target list to be absolute paths, so VPATH will not > be consulted and defeat the entire point of what we're trying to do. > This fixes the Meson build, where we later create > doc/doxygen/xml/meson.build. > > Signed-off-by: Daniel Stone > --- > doc/doxygen/Makefile.am | 13 +++-- > 1 file changed, 7 insertions(+), 6 deletions(-) Hi Daniel, this makes sense to me, so even though I didn't test: Reviewed-by: Pekka Paalanen Thanks, pq > > diff --git a/doc/doxygen/Makefile.am b/doc/doxygen/Makefile.am > index f8b0b3aa..31d953c0 100644 > --- a/doc/doxygen/Makefile.am > +++ b/doc/doxygen/Makefile.am > @@ -53,20 +53,21 @@ diagram_maps := $(patsubst > $(diagramsdir)/%,xml/%,$(diagramssrc:.gv=.map)) > dist_man3_MANS = $(shell test -d man && find man/man3 -name "wl_*.3" -printf > "man/man3/%P\n") > > # Listing various directories that might need to be created. > -alldirs := xml xml/Client xml/Server man/man3 html/Client html/Server > +alldirsrel := xml xml/Client xml/Server man/man3 html/Client html/Server > +alldirs := $(patsubst %,$(CURDIR)/%,$(alldirsrel)) > > $(diagrams): $(diagramssrc) > > $(diagram_maps): $(diagramssrc) > > -xml/%/index.xml: $(top_srcdir)/src/scanner.c $(scanned_src_files_%) > wayland.doxygen $(diagrams) $(diagram_maps) | xml/% > +xml/%/index.xml: $(top_srcdir)/src/scanner.c $(scanned_src_files_%) > wayland.doxygen $(diagrams) $(diagram_maps) | $(CURDIR)/xml/% > $(AM_V_GEN)(cat wayland.doxygen; \ >echo "GENERATE_XML=YES"; \ >echo "XML_OUTPUT=xml/$*"; \ >echo "INPUT= $(scanned_src_files_$*)"; \ >) | $(DOXYGEN) - > > -html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) > $(diagram_maps) | html/% > +html/%/index.html: $(scanned_src_files_%) wayland.doxygen $(diagrams) > $(diagram_maps) | $(CURDIR)/html/% > $(AM_V_GEN)(cat wayland.doxygen; \ >echo "PROJECT_NAME=\"Wayland $* API\""; \ >echo "GENERATE_HTML=YES"; \ > @@ -74,7 +75,7 @@ html/%/index.html: $(scanned_src_files_%) wayland.doxygen > $(diagrams) $(diagram_ >echo "INPUT= $(scanned_src_files_$*) $(extra_doxygen_$*)"; \ >) | $(DOXYGEN) - > > -man/man3/wl_display.3: $(top_srcdir)/src/scanner.c $(scanned_src_files_man) > wayland.doxygen | man/man3 > +man/man3/wl_display.3: $(top_srcdir)/src/scanner.c $(scanned_src_files_man) > wayland.doxygen | $(CURDIR)/man/man3 > $(AM_V_GEN)(cat wayland.doxygen; \ >echo "GENERATE_MAN=YES"; \ >echo "MAN_OUTPUT=man"; \ > @@ -82,10 +83,10 @@ man/man3/wl_display.3: $(top_srcdir)/src/scanner.c > $(scanned_src_files_man) wayl >echo "INPUT= $(scanned_src_files_man)"; \ >) | $(DOXYGEN) - > > -xml/%.png: $(diagramsdir)/%.gv | xml > +xml/%.png: $(diagramsdir)/%.gv | $(CURDIR)/xml > $(AM_V_GEN)$(DOT) -Tpng -o$@ $< > > -xml/%.map: $(diagramsdir)/%.gv | xml > +xml/%.map: $(diagramsdir)/%.gv | $(CURDIR)/xml > $(AM_V_GEN)$(DOT) -Tcmapx_np -o$@ $< > > # general rule to create one of the listed directories. pgpsmJsjHgXL2.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 0/6] Minor test/scanner fixes
On Wed, 29 Aug 2018 07:17:09 +0100 Daniel Stone wrote: > Hi, > These first 5 patches fix some issues I found by running the test suite > under an expanded set of toolchains, including the ASan address > sanitiser and Clang's static analyser. > > Patch 6 removes the leak checking from the test suite completely. > Previously due to toolchain immaturity, it was really difficult to run > the test suite with checks for memory leaks, so we rolled our own. On > the other hand, with Meson we can now just pass '-Db_sanitize=address' > to the configure process, or run 'meson test --wrapper=valgrind', to get > more powerful and useful checkers. > > The existing leak checker we have breaks ASan completely, and I couldn't > figure out how to fix it. Removing it altogether seemed like a better > idea. > > I've implemented this for GitLab CI, and you can see example output > here: https://gitlab.freedesktop.org/daniels/wayland/pipelines/3663 > > This issue tracks the work left on the CI pipeline to get everything > upstream: > https://gitlab.freedesktop.org/wayland/wayland/issues/54 > > If anyone wants to help out, please feel free to grab that branch and > run with it; in the meantime, these seem like good fixes to have > regardless. Hi, I'm thrilled where this is going. This series is: Reviewed-by: Pekka Paalanen Patch 3 could maybe use a code comment for being so subtle. Thanks, pq pgpA96Ily_EcG.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland 6/6] tests: Remove memory leak checking infrastructure
On Wed, Aug 29, 2018 at 07:17:15AM +0100, Daniel Stone wrote:
> There are far better ways to detect memory leaks, such as either
> valgrind or ASan. Having Meson makes it really easy to use these tools
> in our tests, and we can do that in CI as well.
>
> Having these local wrappers actually completely broke ASan usage, so
> remove them in favour of using the more powerful options.
>
> Signed-off-by: Daniel Stone
series Reviewed-by: Peter Hutterer
Cheers,
Peter
> ---
> tests/sanity-test.c | 66 ++
> tests/test-compositor.c | 5 ++-
> tests/test-runner.c | 79 ++---
> tests/test-runner.h | 13 +++
> 4 files changed, 20 insertions(+), 143 deletions(-)
>
> diff --git a/tests/sanity-test.c b/tests/sanity-test.c
> index 2495a115..98beca8d 100644
> --- a/tests/sanity-test.c
> +++ b/tests/sanity-test.c
> @@ -35,7 +35,7 @@
>
> #include "test-compositor.h"
>
> -extern int leak_check_enabled;
> +extern int fd_leak_check_enabled;
>
> TEST(empty)
> {
> @@ -83,71 +83,11 @@ FAIL_TEST(sanity_assert)
> assert(0);
> }
>
> -FAIL_TEST(sanity_malloc_direct)
> -{
> - void *p;
> -
> - assert(leak_check_enabled);
> -
> - p = malloc(10); /* memory leak */
> - assert(p); /* assert that we got memory, also prevents
> - * the malloc from getting optimized away. */
> - free(NULL); /* NULL must not be counted */
> - test_disable_coredumps();
> -}
> -
> -TEST(disable_leak_checks)
> -{
> - volatile void *mem;
> - assert(leak_check_enabled);
> - /* normally this should be on the beginning of the test.
> - * Here we need to be sure, that the leak checks are
> - * turned on */
> - DISABLE_LEAK_CHECKS;
> -
> - mem = malloc(16);
> - assert(mem);
> -}
> -
> -FAIL_TEST(sanity_malloc_indirect)
> -{
> - struct wl_array array;
> -
> - assert(leak_check_enabled);
> -
> - wl_array_init();
> -
> - /* call into library that calls malloc */
> - wl_array_add(, 14);
> -
> - /* not freeing array, must leak */
> -
> - test_disable_coredumps();
> -}
> -
> -FAIL_TEST(tc_client_memory_leaks)
> -{
> - struct display *d = display_create();
> - client_create_noarg(d, sanity_malloc_direct);
> - display_run(d);
> - test_disable_coredumps();
> - display_destroy(d);
> -}
> -
> -FAIL_TEST(tc_client_memory_leaks2)
> -{
> - struct display *d = display_create();
> - client_create_noarg(d, sanity_malloc_indirect);
> - display_run(d);
> - test_disable_coredumps();
> - display_destroy(d);
> -}
> -
> FAIL_TEST(sanity_fd_leak)
> {
> int fd[2];
>
> - assert(leak_check_enabled);
> + assert(fd_leak_check_enabled);
>
> /* leak 2 file descriptors */
> if (pipe(fd) < 0)
> @@ -185,7 +125,7 @@ sanity_fd_no_leak(void)
> {
> int fd[2];
>
> - assert(leak_check_enabled);
> + assert(fd_leak_check_enabled);
>
> /* leak 2 file descriptors */
> if (pipe(fd) < 0)
> diff --git a/tests/test-compositor.c b/tests/test-compositor.c
> index 0631f614..72f63515 100644
> --- a/tests/test-compositor.c
> +++ b/tests/test-compositor.c
> @@ -156,7 +156,7 @@ run_client(void (*client_main)(void *data), void *data,
> int wayland_sock, int client_pipe)
> {
> char s[8];
> - int cur_alloc, cur_fds;
> + int cur_fds;
> int can_continue = 0;
>
> /* Wait until display signals that client can continue */
> @@ -169,7 +169,6 @@ run_client(void (*client_main)(void *data), void *data,
> snprintf(s, sizeof s, "%d", wayland_sock);
> setenv("WAYLAND_SOCKET", s, 0);
>
> - cur_alloc = get_current_alloc_num();
> cur_fds = count_open_fds();
>
> client_main(data);
> @@ -182,7 +181,7 @@ run_client(void (*client_main)(void *data), void *data,
> if (!getenv("WAYLAND_SOCKET"))
> cur_fds--;
>
> - check_leaks(cur_alloc, cur_fds);
> + check_fd_leaks(cur_fds);
> }
>
> static struct client_info *
> diff --git a/tests/test-runner.c b/tests/test-runner.c
> index 82a0a7b8..1487dc48 100644
> --- a/tests/test-runner.c
> +++ b/tests/test-runner.c
> @@ -44,16 +44,10 @@
>
> #include "test-runner.h"
>
> -static int num_alloc;
> -static void* (*sys_malloc)(size_t);
> -static void (*sys_free)(void*);
> -static void* (*sys_realloc)(void*, size_t);
> -static void* (*sys_calloc)(size_t, size_t);
> -
> -/* when set to 1, check if tests are not leaking memory and opened files.
> +/* when set to 1, check if tests are not leaking opened files.
> * It is turned on by default. It can be turned off by
> * WAYLAND_TEST_NO_LEAK_CHECK environment variable. */
> -int leak_check_enabled;
> +int fd_leak_check_enabled;
>
> /* when this var is set to 0, every call to test_set_timeout() is
> * suppressed - handy when debugging the test. Can be set by
> @@ -65,40 +59,6 @@ static int is_atty = 0;
>
> extern const
Re: EXT: [PATCH wayland 6/6] tests: Remove memory leak checking infrastructure
Hi Ian, On Wed, 29 Aug 2018 at 07:45, Ray, Ian (GE Healthcare) wrote: > > On 29 Aug 2018, at 9.17, Daniel Stone wrote: > > @@ -200,7 +147,7 @@ run_test(const struct test *t) > > assert(sigaction(SIGALRM, , NULL) == 0); > > } > > > > - cur_alloc = get_current_alloc_num(); > > + //cur_alloc = get_current_alloc_num(); > > Nit: the above line could/should have been deleted. Oops, yes, thanks for catching that. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: EXT: [PATCH wayland 6/6] tests: Remove memory leak checking infrastructure
> On 29 Aug 2018, at 9.17, Daniel Stone wrote:
>
> There are far better ways to detect memory leaks, such as either
> valgrind or ASan. Having Meson makes it really easy to use these tools
> in our tests, and we can do that in CI as well.
>
> Having these local wrappers actually completely broke ASan usage, so
> remove them in favour of using the more powerful options.
>
> Signed-off-by: Daniel Stone
> ---
> tests/sanity-test.c | 66 ++
> tests/test-compositor.c | 5 ++-
> tests/test-runner.c | 79 ++---
> tests/test-runner.h | 13 +++
> 4 files changed, 20 insertions(+), 143 deletions(-)
>
> diff --git a/tests/sanity-test.c b/tests/sanity-test.c
> index 2495a115..98beca8d 100644
> --- a/tests/sanity-test.c
> +++ b/tests/sanity-test.c
> @@ -35,7 +35,7 @@
>
> #include "test-compositor.h"
>
> -extern int leak_check_enabled;
> +extern int fd_leak_check_enabled;
>
> TEST(empty)
> {
> @@ -83,71 +83,11 @@ FAIL_TEST(sanity_assert)
> assert(0);
> }
>
> -FAIL_TEST(sanity_malloc_direct)
> -{
> - void *p;
> -
> - assert(leak_check_enabled);
> -
> - p = malloc(10); /* memory leak */
> - assert(p); /* assert that we got memory, also prevents
> - * the malloc from getting optimized away. */
> - free(NULL); /* NULL must not be counted */
> - test_disable_coredumps();
> -}
> -
> -TEST(disable_leak_checks)
> -{
> - volatile void *mem;
> - assert(leak_check_enabled);
> - /* normally this should be on the beginning of the test.
> - * Here we need to be sure, that the leak checks are
> - * turned on */
> - DISABLE_LEAK_CHECKS;
> -
> - mem = malloc(16);
> - assert(mem);
> -}
> -
> -FAIL_TEST(sanity_malloc_indirect)
> -{
> - struct wl_array array;
> -
> - assert(leak_check_enabled);
> -
> - wl_array_init();
> -
> - /* call into library that calls malloc */
> - wl_array_add(, 14);
> -
> - /* not freeing array, must leak */
> -
> - test_disable_coredumps();
> -}
> -
> -FAIL_TEST(tc_client_memory_leaks)
> -{
> - struct display *d = display_create();
> - client_create_noarg(d, sanity_malloc_direct);
> - display_run(d);
> - test_disable_coredumps();
> - display_destroy(d);
> -}
> -
> -FAIL_TEST(tc_client_memory_leaks2)
> -{
> - struct display *d = display_create();
> - client_create_noarg(d, sanity_malloc_indirect);
> - display_run(d);
> - test_disable_coredumps();
> - display_destroy(d);
> -}
> -
> FAIL_TEST(sanity_fd_leak)
> {
> int fd[2];
>
> - assert(leak_check_enabled);
> + assert(fd_leak_check_enabled);
>
> /* leak 2 file descriptors */
> if (pipe(fd) < 0)
> @@ -185,7 +125,7 @@ sanity_fd_no_leak(void)
> {
> int fd[2];
>
> - assert(leak_check_enabled);
> + assert(fd_leak_check_enabled);
>
> /* leak 2 file descriptors */
> if (pipe(fd) < 0)
> diff --git a/tests/test-compositor.c b/tests/test-compositor.c
> index 0631f614..72f63515 100644
> --- a/tests/test-compositor.c
> +++ b/tests/test-compositor.c
> @@ -156,7 +156,7 @@ run_client(void (*client_main)(void *data), void *data,
> int wayland_sock, int client_pipe)
> {
> char s[8];
> - int cur_alloc, cur_fds;
> + int cur_fds;
> int can_continue = 0;
>
> /* Wait until display signals that client can continue */
> @@ -169,7 +169,6 @@ run_client(void (*client_main)(void *data), void *data,
> snprintf(s, sizeof s, "%d", wayland_sock);
> setenv("WAYLAND_SOCKET", s, 0);
>
> - cur_alloc = get_current_alloc_num();
> cur_fds = count_open_fds();
>
> client_main(data);
> @@ -182,7 +181,7 @@ run_client(void (*client_main)(void *data), void *data,
> if (!getenv("WAYLAND_SOCKET"))
> cur_fds--;
>
> - check_leaks(cur_alloc, cur_fds);
> + check_fd_leaks(cur_fds);
> }
>
> static struct client_info *
> diff --git a/tests/test-runner.c b/tests/test-runner.c
> index 82a0a7b8..1487dc48 100644
> --- a/tests/test-runner.c
> +++ b/tests/test-runner.c
> @@ -44,16 +44,10 @@
>
> #include "test-runner.h"
>
> -static int num_alloc;
> -static void* (*sys_malloc)(size_t);
> -static void (*sys_free)(void*);
> -static void* (*sys_realloc)(void*, size_t);
> -static void* (*sys_calloc)(size_t, size_t);
> -
> -/* when set to 1, check if tests are not leaking memory and opened files.
> +/* when set to 1, check if tests are not leaking opened files.
> * It is turned on by default. It can be turned off by
> * WAYLAND_TEST_NO_LEAK_CHECK environment variable. */
> -int leak_check_enabled;
> +int fd_leak_check_enabled;
>
> /* when this var is set to 0, every call to test_set_timeout() is
> * suppressed - handy when debugging the test. Can be set by
> @@ -65,40 +59,6 @@ static int is_atty = 0;
>
> extern const struct test __start_test_section, __stop_test_section;
>
> -__attribute__ ((visibility("default"))) void *
[PATCH wayland 1/6] scanner: Plug two memory leaks
Found with both ASan leak sanitizer and Valgrind. We were trivially leaking the enum name for every arg parsed by the scanner which had one. If libxml-based DTD validation was enabled, we would also leak the DTD itself, despite diligently freeing the document, context, etc. Signed-off-by: Daniel Stone --- src/scanner.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/scanner.c b/src/scanner.c index 205c28a9..3afc3d3d 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -133,6 +133,7 @@ is_dtd_valid(FILE *input, const char *filename) rc = xmlValidateDtd(dtdctx, doc, dtd); xmlFreeDoc(doc); xmlFreeParserCtxt(ctx); + xmlFreeDtd(dtd); xmlFreeValidCtxt(dtdctx); /* xmlIOParseDTD consumes buffer */ @@ -432,6 +433,7 @@ free_arg(struct arg *arg) free(arg->name); free(arg->interface_name); free(arg->summary); + free(arg->enumeration_name); free(arg); } -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 5/6] tests: Overly elaborate compiler warning workaround
Clang will rightly point out that example_sockaddr_un in socket-test will get discarded from the compilation unit as it is completely unused. Put in a couple of lines which of no value other than stopping Clang from complaining. Signed-off-by: Daniel Stone --- tests/socket-test.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/socket-test.c b/tests/socket-test.c index e9705628..8d39edce 100644 --- a/tests/socket-test.c +++ b/tests/socket-test.c @@ -42,7 +42,7 @@ * See `man 7 unix`. */ -static const struct sockaddr_un example_sockaddr_un; +static struct sockaddr_un example_sockaddr_un; #define TOO_LONG (1 + sizeof example_sockaddr_un.sun_path) @@ -69,6 +69,11 @@ TEST(socket_path_overflow_client_connect) d = wl_display_connect(path); assert(d == NULL); assert(errno == ENAMETOOLONG); + + /* This is useless, but prevents a warning about example_sockaddr_un +* being discarded from the compilation unit. */ + strcpy(example_sockaddr_un.sun_path, "happy now clang?"); + assert(example_sockaddr_un.sun_path[0] != '\0'); } TEST(socket_path_overflow_server_create) -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 4/6] tests: Use volatile pointer for NULL dereference
Clang warns that it can silently discard a non-volatile write to a NULL
pointer (perhaps it constitutes undefined behaviour?), and recommends
changing it to volatile.
This patch slavishly complies with the demand of the unfeeling machine.
Signed-off-by: Daniel Stone
---
tests/sanity-test.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tests/sanity-test.c b/tests/sanity-test.c
index 66ca16fb..2495a115 100644
--- a/tests/sanity-test.c
+++ b/tests/sanity-test.c
@@ -70,8 +70,10 @@ FAIL_TEST(fail_kill)
FAIL_TEST(fail_segv)
{
+ char * volatile *null = 0;
+
test_disable_coredumps();
- * (char **) 0 = "Goodbye, world";
+ *null = "Goodbye, world";
}
FAIL_TEST(sanity_assert)
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 2/6] scanner: Mark fail() as noreturn
Help static analysers by letting them know that once we fail(),
execution will terminally complete.
Signed-off-by: Daniel Stone
---
src/scanner.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/scanner.c b/src/scanner.c
index 3afc3d3d..084f196d 100644
--- a/src/scanner.c
+++ b/src/scanner.c
@@ -371,7 +371,7 @@ desc_dump(char *desc, const char *fmt, ...)
putchar('\n');
}
-static void
+static void __attribute__ ((noreturn))
fail(struct location *loc, const char *msg, ...)
{
va_list ap;
--
2.17.1
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 3/6] scanner: Reverse expat/libxml include order
libxml2 unconditonally defines XMLCALL to nothing. Expat does not redefine XMLCALL if it is already defined, but if it is not, and we are building with gcc on i386 (not x86-64), it will define it as 'cdecl'. Including Expat before libxml thus results in a warning about XMLCALL being redefined. Luckily we can get around this by just reversing the include order: cdecl is a no-op on Unix-like systems, so by having libxml first define XMLCALL to nothing and including Expat afterwards, we avoid the warning and lose nothing. Signed-off-by: Daniel Stone --- src/scanner.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/scanner.c b/src/scanner.c index 084f196d..47836f9a 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -35,7 +35,6 @@ #include #include #include -#include #include #include #include @@ -48,6 +47,8 @@ extern char DTD_DATA_begin; extern int DTD_DATA_len; #endif +#include + #include "wayland-util.h" #define PROGRAM_NAME "wayland-scanner" -- 2.17.1 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH wayland 6/6] tests: Remove memory leak checking infrastructure
There are far better ways to detect memory leaks, such as either
valgrind or ASan. Having Meson makes it really easy to use these tools
in our tests, and we can do that in CI as well.
Having these local wrappers actually completely broke ASan usage, so
remove them in favour of using the more powerful options.
Signed-off-by: Daniel Stone
---
tests/sanity-test.c | 66 ++
tests/test-compositor.c | 5 ++-
tests/test-runner.c | 79 ++---
tests/test-runner.h | 13 +++
4 files changed, 20 insertions(+), 143 deletions(-)
diff --git a/tests/sanity-test.c b/tests/sanity-test.c
index 2495a115..98beca8d 100644
--- a/tests/sanity-test.c
+++ b/tests/sanity-test.c
@@ -35,7 +35,7 @@
#include "test-compositor.h"
-extern int leak_check_enabled;
+extern int fd_leak_check_enabled;
TEST(empty)
{
@@ -83,71 +83,11 @@ FAIL_TEST(sanity_assert)
assert(0);
}
-FAIL_TEST(sanity_malloc_direct)
-{
- void *p;
-
- assert(leak_check_enabled);
-
- p = malloc(10); /* memory leak */
- assert(p); /* assert that we got memory, also prevents
-* the malloc from getting optimized away. */
- free(NULL); /* NULL must not be counted */
- test_disable_coredumps();
-}
-
-TEST(disable_leak_checks)
-{
- volatile void *mem;
- assert(leak_check_enabled);
- /* normally this should be on the beginning of the test.
-* Here we need to be sure, that the leak checks are
-* turned on */
- DISABLE_LEAK_CHECKS;
-
- mem = malloc(16);
- assert(mem);
-}
-
-FAIL_TEST(sanity_malloc_indirect)
-{
- struct wl_array array;
-
- assert(leak_check_enabled);
-
- wl_array_init();
-
- /* call into library that calls malloc */
- wl_array_add(, 14);
-
- /* not freeing array, must leak */
-
- test_disable_coredumps();
-}
-
-FAIL_TEST(tc_client_memory_leaks)
-{
- struct display *d = display_create();
- client_create_noarg(d, sanity_malloc_direct);
- display_run(d);
- test_disable_coredumps();
- display_destroy(d);
-}
-
-FAIL_TEST(tc_client_memory_leaks2)
-{
- struct display *d = display_create();
- client_create_noarg(d, sanity_malloc_indirect);
- display_run(d);
- test_disable_coredumps();
- display_destroy(d);
-}
-
FAIL_TEST(sanity_fd_leak)
{
int fd[2];
- assert(leak_check_enabled);
+ assert(fd_leak_check_enabled);
/* leak 2 file descriptors */
if (pipe(fd) < 0)
@@ -185,7 +125,7 @@ sanity_fd_no_leak(void)
{
int fd[2];
- assert(leak_check_enabled);
+ assert(fd_leak_check_enabled);
/* leak 2 file descriptors */
if (pipe(fd) < 0)
diff --git a/tests/test-compositor.c b/tests/test-compositor.c
index 0631f614..72f63515 100644
--- a/tests/test-compositor.c
+++ b/tests/test-compositor.c
@@ -156,7 +156,7 @@ run_client(void (*client_main)(void *data), void *data,
int wayland_sock, int client_pipe)
{
char s[8];
- int cur_alloc, cur_fds;
+ int cur_fds;
int can_continue = 0;
/* Wait until display signals that client can continue */
@@ -169,7 +169,6 @@ run_client(void (*client_main)(void *data), void *data,
snprintf(s, sizeof s, "%d", wayland_sock);
setenv("WAYLAND_SOCKET", s, 0);
- cur_alloc = get_current_alloc_num();
cur_fds = count_open_fds();
client_main(data);
@@ -182,7 +181,7 @@ run_client(void (*client_main)(void *data), void *data,
if (!getenv("WAYLAND_SOCKET"))
cur_fds--;
- check_leaks(cur_alloc, cur_fds);
+ check_fd_leaks(cur_fds);
}
static struct client_info *
diff --git a/tests/test-runner.c b/tests/test-runner.c
index 82a0a7b8..1487dc48 100644
--- a/tests/test-runner.c
+++ b/tests/test-runner.c
@@ -44,16 +44,10 @@
#include "test-runner.h"
-static int num_alloc;
-static void* (*sys_malloc)(size_t);
-static void (*sys_free)(void*);
-static void* (*sys_realloc)(void*, size_t);
-static void* (*sys_calloc)(size_t, size_t);
-
-/* when set to 1, check if tests are not leaking memory and opened files.
+/* when set to 1, check if tests are not leaking opened files.
* It is turned on by default. It can be turned off by
* WAYLAND_TEST_NO_LEAK_CHECK environment variable. */
-int leak_check_enabled;
+int fd_leak_check_enabled;
/* when this var is set to 0, every call to test_set_timeout() is
* suppressed - handy when debugging the test. Can be set by
@@ -65,40 +59,6 @@ static int is_atty = 0;
extern const struct test __start_test_section, __stop_test_section;
-__attribute__ ((visibility("default"))) void *
-malloc(size_t size)
-{
- num_alloc++;
- return sys_malloc(size);
-}
-
-__attribute__ ((visibility("default"))) void
-free(void* mem)
-{
- if (mem != NULL)
- num_alloc--;
- sys_free(mem);
-}
-
[PATCH wayland 0/6] Minor test/scanner fixes
Hi, These first 5 patches fix some issues I found by running the test suite under an expanded set of toolchains, including the ASan address sanitiser and Clang's static analyser. Patch 6 removes the leak checking from the test suite completely. Previously due to toolchain immaturity, it was really difficult to run the test suite with checks for memory leaks, so we rolled our own. On the other hand, with Meson we can now just pass '-Db_sanitize=address' to the configure process, or run 'meson test --wrapper=valgrind', to get more powerful and useful checkers. The existing leak checker we have breaks ASan completely, and I couldn't figure out how to fix it. Removing it altogether seemed like a better idea. I've implemented this for GitLab CI, and you can see example output here: https://gitlab.freedesktop.org/daniels/wayland/pipelines/3663 This issue tracks the work left on the CI pipeline to get everything upstream: https://gitlab.freedesktop.org/wayland/wayland/issues/54 If anyone wants to help out, please feel free to grab that branch and run with it; in the meantime, these seem like good fixes to have regardless. Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
