Simon McVittie [Tue, 20 Nov 2018 17:32:02 +0000 (17:32 +0000)]
dbus-spawn-unix: If a fd is not close-on-exec, look it up in /proc/self/fd
In operating systems where /proc/self/fd works like it does on Linux
(Linux itself, and FreeBSD with Linux /proc emulation) this will give
us a clue about the fd that was leaked or opened incorrectly.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Ralf Habacker [Tue, 20 Nov 2018 13:06:17 +0000 (14:06 +0100)]
Fix broken meinproc4 based docbook generating by already used xsltproc
Using xsltproc helps to reduce manual editing of xml doc and avoids
cyclic dependency (kdelibs depends on dbus and dbus depends on kdelibs).
It is available on all platforms (in the opposite to xmlto) and supports
freedesktop CI out of the box.
This commit adds docbook-xml and docbook-xsl as new dependency for cmake
and removes obsolate xmlto support, which depends on xsltproc.
Ralf Habacker [Tue, 20 Nov 2018 12:56:39 +0000 (13:56 +0100)]
Cleanup of cmake doc-related targets
There is now a top-level target "doc" that is always built.
Depending on the detected generators it depends on optional
targets like apidoc' and 'devhelp2'.
Simon McVittie [Fri, 16 Nov 2018 17:30:47 +0000 (17:30 +0000)]
BusDesktopFile: Refactor logic to free the parser contents
Now that we have _DBUS_STRING_INIT_INVALID, we can initialize
parser.data to a value that is safe for _dbus_string_free(), which
means we can put all the cleanup through a single code path that
definitely frees everything.
(This is just refactoring, not a correctness fix.)
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 19 Nov 2018 13:53:52 +0000 (13:53 +0000)]
containers test: Wait a few more seconds for the socket to be deleted
Previously, we were waiting a few seconds for the dbus-daemon to stop
listening, then trying to connect again and asserting that it failed,
then immediately asserting that the socket had actually been deleted.
However, there is a race here: the dbus-daemon stops listening on the
socket, and then deletes it. If the test client wins the race by
probing to see whether the socket is present after the dbus-daemon
has stopped listening but before the dbus-daemon has deleted it, then
the test will fail.
This intermittently happens on Gitlab-CI, most recently in
<https://gitlab.freedesktop.org/smcv/dbus/-/jobs/45694>.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 31 Aug 2018 18:01:03 +0000 (19:01 +0100)]
tests: Generate random values in unsigned integer space
When we're doing bitwise operations, addition with wraparound and
large left-shifts, it seems safer to use unsigned integers, where
the effect of overflow is well-defined (it wraps around). Signed
integer overflow is undefined behaviour, so compilers are free to
optimize by assuming that it cannot happen.
Detected by the undefined behaviour sanitizer (UBSan).
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 17 Aug 2018 18:38:07 +0000 (19:38 +0100)]
keyring: Avoid undefined out-of-range shift
Detected with UndefinedBehaviourSanitizer, which will warn on
about 50% of calls to this function, when s[3] is 128 or more,
because id is signed, so 128 << 24 is undefined signed overflow.
All we want here is a random non-negative signed int (in the range 0
to 2**31-1, with 31 bits varying). The intention seemed to be to
generate a random unsigned int, cast it to signed, and then negate it
if negative, but it seems simpler and more obviously correct to just
make sure the most significant byte fits in the non-negative range.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 16 Nov 2018 17:25:18 +0000 (17:25 +0000)]
BusDesktopFile: Don't leak content if key=value appears before [Section]
BusDesktopFile has a strange convention in which the various parser
helper functions (parse_section_start(), etc.) free the parser on error.
However, this particular error case happens outside the helper functions
and so will leak.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Fri, 16 Nov 2018 16:21:48 +0000 (16:21 +0000)]
build: Require va_copy() or __va_copy() on non-MSVC compilers
va_copy() is a C99 feature, and should be widely supported by now.
gcc in strict C89 mode implements an equivalent __va_copy() instead.
MSVC 2013 implements va_copy(), but at the moment we still aim to support
MSVC 2010 and 2012, which don't have it. However, we know that in
Windows ABIs, va_list is a pointer, so we can use
_DBUS_VA_COPY_ASSIGN. We do not support MSVC for Autotools builds, only
CMake, due to its non-Unixish command-line interface.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 15 Nov 2018 17:40:33 +0000 (17:40 +0000)]
Don't check how to copy a va_list if we already have va_copy()
If we already have ISO C va_copy() or its non-standard counterpart
__va_copy(), then there's no need to do an AC_RUN_IFELSE or its
CMake equivalent to detect whether "args2 = args1" or "*args2 = *args1"
works. AC_RUN_IFELSE is problematic during cross-compilation, where the
program cannot be run (you have to know in advance that the test program
will be run and what its result will be), so we want to avoid it whenever
possible.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 12 Jul 2018 16:38:04 +0000 (17:38 +0100)]
embedded tests: Make it easier to run a single test-case
When running tests under "make check" or similar to take advantage
of facilities like AM_TESTS_ENVIRONMENT and AX_VALGRIND_CHECK, it's
more straightforward to set an environment variable than to pass a
command-line option.
Signed-off-by: Simon McVittie <smcv@collabora.com> Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://gitlab.freedesktop.org/dbus/dbus/issues/218
Simon McVittie [Thu, 15 Nov 2018 18:55:57 +0000 (18:55 +0000)]
internals: Assume compiler supports a subset of ISO varargs syntax
We have considerable anecdotal evidence that every relevant compiler
supports at least the small part of ISO varargs syntax that we need
here, because tools/tool-common.h has contained
#define VERBOSE(...) do {} while (0)
since dbus 1.9.2 (2014) and nobody has complained yet. With that in
mind, let's simplify.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Ralf Habacker [Thu, 25 Oct 2018 18:14:24 +0000 (20:14 +0200)]
Refactor cmake checks for DBUS_VA_COPY and DBUS_VA_COPY_ARRAY
For test case execution, CheckCSourceCompiles is now used instead
of try_compile and the determination of DBUS_VA_AS_ARRAY is
performed with a separate test instead of evaluating the result
of HAVE_VA_COPY and HAVE___VA_COPY.
The tests are performed for all supported compilers. Since older
MSVC compilers (< 2013) do not support va_copy(), the macro
_DBUS_VA_ASSIGN(a1,a2) with the implementation { a1 = a2; } is used
as a fallback.
Use SELINUX_CB_POLICYLOAD instead of AVC_CALLBACK_RESET callback
Use SELINUX_CB_POLICYLOAD instead of AVC_CALLBACK_RESET callback as this
only seems necessary on policy reload and not if the enforcing mode is
changing.
See discussion at https://marc.info/?l=selinux&m=152173501930182&w=2
Stop using avc_init() and use avc_open() instead. With this commit
dbus-daemon will stop using a thread to monitor the avc netlink and will
poll it instead.
Ralf Habacker [Fri, 26 Oct 2018 13:12:29 +0000 (15:12 +0200)]
Avoid double slashes in paths created by pkg-config
If in a .pc variable a path is created from another
variable, such as exec_prefix=${prefix}/lib, prefix
must not contain a trailing slash to avoid double
slashes in the generated path.
Simon McVittie [Wed, 31 Oct 2018 15:09:24 +0000 (15:09 +0000)]
tests: Build installable helpers whenever any tests are enabled
We might need these to run tests at build-time, so we should build them
whenever either modular or embedded tests are enabled, even if
installed-tests aren't.
We haven't noticed this bug until now because $(installable_helpers)
only contained test-apparmor-activation, which isn't normally needed at
build-time because the AppArmor test can only work when run as root.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 22 Oct 2018 09:20:37 +0000 (10:20 +0100)]
cmake: Install d-d-launch-helper to ${CMAKE_INSTALL_LIBEXECDIR}
Instead of hard-coding the lib subdirectory of the installation prefix,
use the ${CMAKE_INSTALL_LIBEXECDIR} provided by the GNUInstallDirs
module, which defaults to the libexec subdirectory of the installation
prefix. This is consistent with the Autotools build system, which
installs to ${exec_prefix}/libexec unless modified with --libexecdir.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Ralf Habacker [Tue, 23 Oct 2018 19:56:25 +0000 (21:56 +0200)]
Cleanup of the cmake definitions of source directories
Since the cmake build system is located below the cmake/
subdirectory, references to the source files used such as
../../bus or ${CMAKE_SOURCE_DIR}/../dbus etc. are required.
To standardize and simplify this, a cmake variable is now
defined in each of the listed directories, which contains
the corresponding path.
Ralf Habacker [Sat, 20 Oct 2018 20:34:33 +0000 (22:34 +0200)]
Refactor cmake checks for DBUS_VA_COPY and DBUS_VA_COPY_ARRAY
For test case execution, CheckCSourceCompiles is now used instead
of try_compile and the determination of DBUS_VA_AS_ARRAY is
performed with a separate test instead of evaluating the result
of HAVE_VA_COPY and HAVE___VA_COPY.
The tests are performed for all supported compilers. Since older
MSVC compilers (< 2013) do not support va_copy(), the macro
_DBUS_VA_ASSIGN(a1,a2) with the implementation { a1 = a2; } is used
as a fallback.
Reviewed-by: Simon McVittie <smcv@collabora.com>
Bug: https://gitlab.freedesktop.org/dbus/dbus/merge_requests/18
Simon McVittie [Tue, 23 Oct 2018 10:43:23 +0000 (11:43 +0100)]
dbus-daemon test: Allow much longer for pending fd timeout
The timeout we're using here is 0.5s (500ms), but the actual time taken
is unbounded, because the OS scheduler might not schedule our process
for an arbitrary length of time after we become runnable.
We previously allowed up to 1 second, but in the CI jobs for dbus!9
and dbus!18 we've seen this take up to 3.4 seconds (presumably
because other tests, or other jobs running on the same shared
infrastructure, starved this process). Allow up to 10 seconds to guard
against spurious failures.
The timeout used in the production system.conf is 150 seconds (2½
minutes), and we're only using the shorter 500ms timeout here to make
the test complete more quickly, so ±10 seconds is relatively
insignificant: the main thing is that it's finite.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Mon, 22 Oct 2018 10:45:45 +0000 (11:45 +0100)]
build: Never use poll() on Darwin family (macOS, etc.) or Interix
Doing a runtime check in configure.ac (AC_RUN_IFELSE) has several
disadvantages:
* It doesn't work when cross-compiling. For example, if we build macOS
binaries on a Linux system, we'd assume that poll() works, but in
fact it won't.
* It checks the build system capabilities, but that is not necessarily
appropriate if (for example) a macOS 10.10 user builds binaries that
could be used by macOS 10.12 or macOS 10.9 users.
* It checks for one specific failure mode, but macOS seems to have a
history of various implementation issues in poll().
* If we want it to work in CMake, we have to duplicate it in the CMake
build system.
None of these is a showstopper on its own, but the combination of all
of them makes the current approach to avoiding the broken poll() on
macOS look unreliable. libcurl, a widely-portable library making
extensive use of sockets, specifically doesn't use poll() on Darwin
(macOS, iOS, etc.) or on Interix; let's follow their example here.
See also https://bugzilla.gnome.org/show_bug.cgi?id=302672 and
https://daniel.haxx.se/blog/2016/10/11/poll-on-mac-10-12-is-broken/
for some relevant history.
Signed-off-by: Simon McVittie <smcv@collabora.com>
Resolves: https://gitlab.freedesktop.org/dbus/dbus/issues/232
Ralf Habacker [Mon, 22 Oct 2018 07:27:30 +0000 (09:27 +0200)]
cmake: Fix incorrect header list separator in configure checks
CMake expects a semicolon-separated list of headers, not a
space-separated list. In particular, this meant we failed to detect
getpwnam_r() on Linux, and fell back to getpwnam().
Ralf Habacker [Tue, 16 Oct 2018 10:21:55 +0000 (12:21 +0200)]
Keep macro values in config.h.cmake synchronized with Autotools
Autotools sets the value of HAVE_xxx macros in config.h to 1 if
the corresponding value exists. This has not been used consistently
in config.h.cmake so far.
Bug: https://gitlab.freedesktop.org/dbus/dbus/issues/117 Reviewed-by: Simon McVittie <smcv@collabora.com>
Simon McVittie [Thu, 18 Oct 2018 16:36:37 +0000 (17:36 +0100)]
desktop-file: Treat backslashes in section names as literal
The Desktop Entry Specification doesn't give any special meaning to
backslashes in section names: a line "[\n]" starts a section whose
name is the two characters (backslash, n), not a section whose name
is a newline. GKeyFile in GLib matches this interpretation.
In practice, the only section used by dbus-daemon is "D-BUS Service",
only way this could make a difference is if someone had written it
as "D-BUS\sService". According to
https://codesearch.debian.net/search?q=%5C%5BD-BUS%5C%5CsService%5C%5D
there is no instance of that pattern in Debian.
Signed-off-by: Simon McVittie <smcv@collabora.com>