Summary: | test suite fails on test-dbus with --enable-tests (but no assertions) | ||
---|---|---|---|
Product: | dbus | Reporter: | Ralf Habacker <ralf.habacker> |
Component: | core | Assignee: | Havoc Pennington <hp> |
Status: | RESOLVED FIXED | QA Contact: | |
Severity: | normal | ||
Priority: | medium | CC: | msniko14 |
Version: | 1.5 | Keywords: | love |
Hardware: | All | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
Fix test failure caused by incomplete dbus_assert implementation when running tests configured with --enable-tests --enable-embedded-tests
Fix test suite fails on test-dbus with enabled-tests |
Description
Ralf Habacker
2014-09-17 08:04:17 UTC
(In reply to comment #0) > ../../dbus/test-driver: Zeile 95: 359 Speicherzugriffsfehler "$@" > > $log_file 2>&1 My technical German is not very good; please run this in English, or say what a Speicherzugriffsfehler is. I assume, from the fact that the log cuts off with no obvious error messages, that it is a segmentation fault or something. If so, backtrace please. Works for me (Debian unstable, x86-64), albeit possibly not with exactly the same configure options. Configuring in the same way you did produces this message:
> NOTE: building with embedded tests but without assertions means
> tests may not properly report failures (this configuration is
> only useful when doing something like profiling the tests)
I wouldn't be surprised if there's a bug in this configuration. Add --enable-asserts to get something more useful.
Yes, the bug is that dbus/dbus-message-util.c has side-effects in assertions, e.g.: _dbus_assert (dbus_message_iter_open_container (&array_iter, DBUS_TYPE_STRUCT, NULL, &struct_iter)); Patches welcome. Created attachment 106499 [details] [review] Fix test failure caused by incomplete dbus_assert implementation when running tests configured with --enable-tests --enable-embedded-tests (In reply to comment #4) > Yes, the bug is that dbus/dbus-message-util.c has side-effects in > assertions, e.g.: > > _dbus_assert (dbus_message_iter_open_container (&array_iter, > DBUS_TYPE_STRUCT, > NULL, &struct_iter)); > The problem here is that with disabled asserts dbus_assert(...) compiles to a noop, which skips the contained functions call. > Patches welcome. see attachment Comment on attachment 106499 [details] [review] Fix test failure caused by incomplete dbus_assert implementation when running tests configured with --enable-tests --enable-embedded-tests Review of attachment 106499 [details] [review]: ----------------------------------------------------------------- ::: dbus/dbus-internals.h @@ +125,4 @@ > const char* _dbus_strerror (int error_number); > > #ifdef DBUS_DISABLE_ASSERT > +#define _dbus_assert(condition) do { dbus_bool_t r = (condition) != 0 ? 0 : 1; } while (0) No, the intention is that _dbus_assert(condition) does nothing in production builds of D-Bus, even if @condition is expensive to evaluate - just like standard C assert(). Possible solutions are - don't configure dbus like this, it makes very little sense to enable tests but not enable the assertions that check for failures - add a _dbus_assert_se() or something which *does* guarantee to evaluate its argument for its side-effects even if assertions are disabled - use this idiom instead of _dbus_assert (dbus_do_some_thing (...)): dbus_bool_t ok; ... ok = dbus_do_some_thing (...); _dbus_assert (ok); > dbus_bool_t r = (condition) != 0 ? 0 : 1;
This is going to cause compiler warnings in recent gcc: "value assigned to r is never used" or similar.
If you're using code similar to this to implement _dbus_assert_se(), either use _DBUS_UNUSED, or something like
do { if ((condition)) { /* do nothing */ } } while (0)
or have a look at what other projects do for their equivalent macros (I think systemd has an assert_se() and GLib has g_assert_true()).
Created attachment 106814 [details] [review] Fix test suite fails on test-dbus with enabled-tests Choosed the _dbus_bool_t ok approach. Comment on attachment 106814 [details] [review] Fix test suite fails on test-dbus with enabled-tests Review of attachment 106814 [details] [review]: ----------------------------------------------------------------- Looks good to me! Comment on attachment 106814 [details] [review] Fix test suite fails on test-dbus with enabled-tests committed to master |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.