Upstream: https://gitlab.freedesktop.org/bolt/bolt/merge_requests/140 From 7c0218e69062409f936bea943b0fdfe2e90eb85f Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 17:18:20 +0100 Subject: [PATCH 01/14] test: bolt_closedir error check only on glibc The error check for bolt_closedir is based on the fact that glibc will return EINVAL when passing NULL. On "musl" there is no such check and instead we crash. Let's not crash, and instead omit the test case on !glibc. --- tests/test-common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test-common.c b/tests/test-common.c index e25dd6a..49b7d4d 100644 --- a/tests/test-common.c +++ b/tests/test-common.c @@ -694,10 +694,12 @@ test_io_errors (TestIO *tt, gconstpointer user_data) g_clear_pointer (&err, g_error_free); /* closedir */ +#ifdef __GLIBC__ ok = bolt_closedir (NULL, &err); g_assert_error (err, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); g_assert_false (ok); g_clear_pointer (&err, g_error_free); +#endif /* rmdir */ ok = bolt_rmdir (noexist, &err); -- 2.18.1 From b3b93465923c29df7ca0ce009a31d882d9a1922e Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 17:21:42 +0100 Subject: [PATCH 02/14] test: just assert G_IO_ERROR for bolt_copy_bytes When passing an invalid file descriptor to bolt_copy_bytes, to check its error handling is correct, just check we get an G_IO_ERROR back, but don't expect a specific error code, because that depends on the copy primitive used in bolt_copy_bytes; currently this is only the copy_file_range function used, but in the future sendfile should be added and that will return a different error code (EBADF). --- tests/test-common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test-common.c b/tests/test-common.c index 49b7d4d..2c261df 100644 --- a/tests/test-common.c +++ b/tests/test-common.c @@ -806,7 +806,8 @@ test_io_errors (TestIO *tt, gconstpointer user_data) /* copy_bytes */ ok = bolt_copy_bytes (to, from, 1, &err); - g_assert_error (err, G_IO_ERROR, G_IO_ERROR_INVALID_ARGUMENT); + g_assert_nonnull (err); + g_assert_cmpint (err->domain, ==, G_IO_ERROR); g_assert_false (ok); g_clear_pointer (&err, g_error_free); } -- 2.18.1 From 1d4f037e3f607732444069ea6e096c9fb389cf38 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 17:25:53 +0100 Subject: [PATCH 03/14] test: power: explicitly include stdlib.h The EXIT_FAILURE/EXIT_SUCCESS "constants" are defined in the "stdlib.h" header, so explicitly include that file to make sure they are properly defined. --- tests/test-power.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test-power.c b/tests/test-power.c index a97c19c..d806723 100644 --- a/tests/test-power.c +++ b/tests/test-power.c @@ -38,6 +38,7 @@ #include #include #include +#include typedef struct udev_device udev_device; G_DEFINE_AUTOPTR_CLEANUP_FUNC (udev_device, udev_device_unref); -- 2.18.1 From cea5cd2cd3337fc3e611c23c54befb851684147a Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 17:28:48 +0100 Subject: [PATCH 04/14] cli: boltctl: explicitly include stdlib.h The EXIT_FAILURE/EXIT_SUCCESS "constants" are defined in the "stdlib.h" header, so explicitly include that file to make sure they are properly defined. --- cli/boltctl-authorize.c | 2 ++ cli/boltctl-domains.c | 2 ++ cli/boltctl-enroll.c | 2 ++ cli/boltctl-forget.c | 2 ++ cli/boltctl-info.c | 2 ++ cli/boltctl-list.c | 2 ++ cli/boltctl-monitor.c | 2 ++ cli/boltctl-power.c | 2 ++ 8 files changed, 16 insertions(+) diff --git a/cli/boltctl-authorize.c b/cli/boltctl-authorize.c index b774889..a2f47b3 100644 --- a/cli/boltctl-authorize.c +++ b/cli/boltctl-authorize.c @@ -25,6 +25,8 @@ #include "bolt-error.h" #include "bolt-str.h" +#include + int authorize (BoltClient *client, int argc, char **argv) { diff --git a/cli/boltctl-domains.c b/cli/boltctl-domains.c index 7ac2467..40d7643 100644 --- a/cli/boltctl-domains.c +++ b/cli/boltctl-domains.c @@ -25,6 +25,8 @@ #include "bolt-str.h" +#include + /* domain related commands */ static void print_domain (BoltDomain *domain, gboolean verbose) diff --git a/cli/boltctl-enroll.c b/cli/boltctl-enroll.c index 498c57e..59d8aac 100644 --- a/cli/boltctl-enroll.c +++ b/cli/boltctl-enroll.c @@ -24,6 +24,8 @@ #include "bolt-str.h" +#include + int enroll (BoltClient *client, int argc, char **argv) { diff --git a/cli/boltctl-forget.c b/cli/boltctl-forget.c index 8bf34fc..def60bb 100644 --- a/cli/boltctl-forget.c +++ b/cli/boltctl-forget.c @@ -24,6 +24,8 @@ #include "bolt-str.h" +#include + int forget (BoltClient *client, int argc, char **argv) { diff --git a/cli/boltctl-info.c b/cli/boltctl-info.c index 1df612a..02454d9 100644 --- a/cli/boltctl-info.c +++ b/cli/boltctl-info.c @@ -24,6 +24,8 @@ #include "bolt-str.h" +#include + int info (BoltClient *client, int argc, char **argv) { diff --git a/cli/boltctl-list.c b/cli/boltctl-list.c index 0ef1bd0..ae7ee95 100644 --- a/cli/boltctl-list.c +++ b/cli/boltctl-list.c @@ -24,6 +24,8 @@ #include "bolt-str.h" +#include + int list_devices (BoltClient *client, int argc, char **argv) { diff --git a/cli/boltctl-monitor.c b/cli/boltctl-monitor.c index 77d143a..93dd8a6 100644 --- a/cli/boltctl-monitor.c +++ b/cli/boltctl-monitor.c @@ -24,6 +24,8 @@ #include "bolt-str.h" +#include + static void handle_domain_added (BoltClient *cli, const char *opath, diff --git a/cli/boltctl-power.c b/cli/boltctl-power.c index 1cdeee7..1c93251 100644 --- a/cli/boltctl-power.c +++ b/cli/boltctl-power.c @@ -24,6 +24,8 @@ #include "bolt-str.h" +#include + static gboolean quit_main_loop (gpointer user_data) { -- 2.18.1 From 4f33281c98c92c1592d5810cb1aabca9b9d97ce9 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 17:31:38 +0100 Subject: [PATCH 05/14] meson: auto-detect system unit installation This deprecates the "systemd" meson option. Instead of requiring systemd (for the unit file installation) depending on the meson option, make "systemd" an optional dependency and if it is found we install the unit file. --- meson.build | 4 ++-- meson_options.txt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/meson.build b/meson.build index 585c0c0..7a0e020 100644 --- a/meson.build +++ b/meson.build @@ -102,8 +102,8 @@ dbdir = join_paths(statedir, 'lib', dbname) udevdir = udev.get_pkgconfig_variable('udevdir') unitdir = '' -if get_option('systemd') - systemd = dependency('systemd') +systemd = dependency('systemd', required: false) +if systemd.found() unitdir = systemd.get_pkgconfig_variable('systemdsystemunitdir') endif diff --git a/meson_options.txt b/meson_options.txt index 9f835c9..ab9288a 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -2,5 +2,5 @@ option('db-path', type: 'string', description: 'DEPRECATED') option('db-name', type: 'string', value: 'boltd', description: 'Name for the device database') option('man', type: 'combo', choices: ['auto', 'true', 'false'], value: 'auto', description: 'Build man pages') option('privileged-group', type: 'string', value: 'wheel', description: 'Name of privileged group') -option('systemd', type: 'boolean', value: 'true', description: 'Whether or not to install the systemd unit') +option('systemd', type: 'boolean', value: 'true', description: 'DEPRECATED') option('coverity', type: 'boolean', value: 'false', description: 'Whether or not to do a coverity build') -- 2.18.1 From 3aa83dc313650248038f97fa7532b42890a3e951 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 17:42:12 +0100 Subject: [PATCH 06/14] meson: check if copy_file_range(2) is available Test if the copy_file_range(2) function is available and define HAVE_FN_COPY_FILE_RANGE as 1 if so, as 0 otherwise. _GNU_SOURCE must be defined in for glibc to expose it, so add that for all function tests. --- config.h.in | 1 + meson.build | 7 ++++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/config.h.in b/config.h.in index 16ea8e9..11bf48e 100644 --- a/config.h.in +++ b/config.h.in @@ -34,6 +34,7 @@ /* availability of features */ #mesondefine HAVE_FN_EXPLICIT_BZERO #mesondefine HAVE_FN_GETRANDOM +#mesondefine HAVE_FN_COPY_FILE_RANGE #mesondefine HAVE_POLKIT_AUTOPTR /* constants */ diff --git a/meson.build b/meson.build index 7a0e020..11164e4 100644 --- a/meson.build +++ b/meson.build @@ -125,11 +125,12 @@ conf.set('VERSION_MINOR', version_minor) conf.set('_GNU_SOURCE', true) foreach fn : [ - ['explicit_bzero', '''#include '''], - ['getrandom', '''#include '''] + ['copy_file_range', '''#include '''], + ['explicit_bzero', '''#include '''], + ['getrandom', '''#include '''], ] - have = compiler.has_function(fn[0], prefix : fn[1]) + have = compiler.has_function(fn[0], prefix: fn[1], args: '-D_GNU_SOURCE') conf.set10('HAVE_FN_' + fn[0].to_upper(), have) endforeach -- 2.18.1 From 452bb4934e8cce48c2f198cdba170945a09993f5 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 17:44:46 +0100 Subject: [PATCH 07/14] common: io: fallback stub for copy_file_range If the copy_file_range function is not provided by the system libc, provide a fallback implementation that uses syscall(2). This wont work on older kernels (> 4.5) but on these systems bold won't work anyway. --- common/bolt-io.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/common/bolt-io.c b/common/bolt-io.c index 41f3730..33d17b0 100644 --- a/common/bolt-io.c +++ b/common/bolt-io.c @@ -30,6 +30,11 @@ #include #include +#if !HAVE_FN_COPY_FILE_RANGE +#include +#include +#endif + #include "bolt-error.h" #include "bolt-str.h" @@ -727,6 +732,23 @@ bolt_rename (const char *from, return FALSE; } +#if !HAVE_FN_COPY_FILE_RANGE +static loff_t +copy_file_range (int fd_in, + loff_t *off_in, + int fd_out, + loff_t *off_out, + size_t len, + unsigned int flags) +{ + return syscall (__NR_copy_file_range, + fd_in, off_in, + fd_out, off_out, + len, + flags); +} +#endif + gboolean bolt_copy_bytes (int fd_from, int fd_to, -- 2.18.1 From 679f09716bfe8e9255b960b67f90a4e68be2d7b3 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Wed, 12 Dec 2018 15:36:49 +0100 Subject: [PATCH 08/14] common: io: define __NR_copy_file_range if missing Define __NR_copy_file_range on x86 & x86_64 if missing; this should only be needed (and working!) on old distributions with a new kernel. --- common/bolt-io.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/common/bolt-io.c b/common/bolt-io.c index 33d17b0..2d3367e 100644 --- a/common/bolt-io.c +++ b/common/bolt-io.c @@ -33,6 +33,15 @@ #if !HAVE_FN_COPY_FILE_RANGE #include #include +# ifndef __NR_copy_file_range +# if defined(__x86_64__) +# define __NR_copy_file_range 326 +# elif defined(__i386__) +# define __NR_copy_file_range 377 +# else +# error "__NR_copy_file_range on this architecture" +# endif +# endif #endif #include "bolt-error.h" -- 2.18.1 From a395cb9a7ab12bb520ddb0ce2c8059ed673a2441 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 17:58:10 +0100 Subject: [PATCH 09/14] contrib: add an Alpine based container Alpine is uses "musl" as C library and does not use systemd, but instead uses eudev as "udev" implementation. --- contrib/Dockerfile-alpine | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 contrib/Dockerfile-alpine diff --git a/contrib/Dockerfile-alpine b/contrib/Dockerfile-alpine new file mode 100644 index 0000000..72ab6a6 --- /dev/null +++ b/contrib/Dockerfile-alpine @@ -0,0 +1,8 @@ +## -*- mode: dockerfile -*- +FROM alpine:latest +ENV LANG en_US.UTF-8 +ENV LANGUAGE en_US:en +ENV LC_ALL en_US.UTF-8 +RUN apk add --no-cache bash dbus dbus-dev eudev-dev gcc glib-dev libc-dev meson ninja polkit-dev udev +RUN mkdir /src +WORKDIR /src -- 2.18.1 From 2f30028c0dd688c041d044633551ac01cbd776cd Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Mon, 10 Dec 2018 18:01:21 +0100 Subject: [PATCH 10/14] ci: gitlab: test on alpine Alpine uses "musl" as C standard library, "eudev" for udev and does not use systemd. This makes Alpine a good candidate for more "esoteric" systems that bolt still wants to support. --- .gitlab-ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e864094..8846ac6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -25,3 +25,9 @@ debian: variables: OS: debian <<: *build + +alpine: + stage: build + variables: + OS: alpine + <<: *build -- 2.18.1 From 1d92269c8338523b7dc917b6dd3f53695df96bab Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Tue, 11 Dec 2018 17:09:48 +0100 Subject: [PATCH 11/14] test: common: bolt_copy_bytes check Verify we can successfully copy a bunch of random data from one file to another, but generating a random file, bolt_copy_bytes to a new one and then reading it back and verifying the checksums match. --- tests/test-common.c | 86 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/tests/test-common.c b/tests/test-common.c index 2c261df..415117e 100644 --- a/tests/test-common.c +++ b/tests/test-common.c @@ -901,6 +901,85 @@ test_io_file_write_all (TestIO *tt, gconstpointer user_data) g_assert_true (strncmp (data, ref, 5) == 0); } +static void +test_io_copy_bytes (TestIO *tt, gconstpointer user_data) +{ + g_autoptr(GError) error = NULL; + g_autoptr(GChecksum) chk = NULL; + g_autofree char *source = NULL; + g_autofree char *target = NULL; + g_autofree char *chksum = NULL; + const gsize N = 1024; + char buf[4096]; + gboolean ok; + int from = -1; + int to = -1; + + chk = g_checksum_new (G_CHECKSUM_SHA256); + + source = g_build_filename (tt->path, "copy_bytes_source", NULL); + from = bolt_open (source, O_RDWR | O_CREAT, 0666, &error); + g_assert_no_error (error); + g_assert_cmpint (from, >, -1); + + for (gsize i = 0; i < N; i++) + { + bolt_random_prng (buf, sizeof (buf)); + bolt_write_all (from, buf, sizeof (buf), &error); + g_assert_no_error (error); + g_checksum_update (chk, (const guchar *) buf, sizeof (buf)); + } + + chksum = g_strdup (g_checksum_get_string (chk)); + + /* close, reopen readonly */ + ok = bolt_close (from, &error); + g_assert_no_error (error); + g_assert_true (ok); + + from = bolt_open (source, O_CLOEXEC | O_RDONLY, 0, &error); + g_assert_no_error (error); + g_assert_cmpint (from, >, -1); + + + /* copy the data with bolt_copy_bytes */ + target = g_build_filename (tt->path, "copy_bytes_target", NULL); + to = bolt_open (target, O_RDWR | O_CREAT, 0666, &error); + g_assert_no_error (error); + g_assert_cmpint (to, >, -1); + + ok = bolt_copy_bytes (from, to, N * sizeof (buf), &error); + g_assert_no_error (error); + g_assert_true (ok); + + /* close, reopen, check checksum */ + ok = bolt_close (to, &error); + g_assert_no_error (error); + g_assert_true (ok); + + to = bolt_open (target, O_CLOEXEC | O_RDONLY, 0, &error); + g_assert_no_error (error); + g_assert_cmpint (to, >, -1); + + g_checksum_reset (chk); + for (gsize i = 0; i < N; i++) + { + gsize nread = 0; + + bolt_read_all (to, buf, sizeof (buf), &nread, &error); + g_assert_no_error (error); + g_checksum_update (chk, (const guchar *) buf, nread); + } + + ok = bolt_close (to, &error); + g_assert_no_error (error); + g_assert_true (ok); + + g_assert_cmpstr (g_checksum_get_string (chk), + ==, + chksum); +} + static void test_autoclose (TestIO *tt, gconstpointer user_data) { @@ -1677,6 +1756,13 @@ main (int argc, char **argv) test_io_file_write_all, test_io_tear_down); + g_test_add ("/common/io/copy_bytes", + TestIO, + NULL, + test_io_setup, + test_io_copy_bytes, + test_io_tear_down); + g_test_add ("/common/io/autoclose", TestIO, NULL, -- 2.18.1 From 65cf2c5552949dae0280afced908b15834d8b9bc Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Tue, 11 Dec 2018 18:36:47 +0100 Subject: [PATCH 12/14] udev: explicitly include string.h for strstr The strstr(3) function is used in boltd/bolt-udev.c but the string.h header not explicitly included. --- boltd/bolt-udev.c | 1 + 1 file changed, 1 insertion(+) diff --git a/boltd/bolt-udev.c b/boltd/bolt-udev.c index 1888f98..178404a 100644 --- a/boltd/bolt-udev.c +++ b/boltd/bolt-udev.c @@ -26,6 +26,7 @@ #include "bolt-sysfs.h" #include +#include #include -- 2.18.1 From b77c83acbef2f876ad18e35fb974e1a349a71bf7 Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Tue, 11 Dec 2018 19:14:32 +0100 Subject: [PATCH 13/14] contrib: ubuntu 18.05 based container image added export OS=ub1804 sudo docker build -t bolt-$OS -f ./contrib/Dockerfile-$OS . sudo docker run -e -t -v `pwd`:/src bolt-$OS ./contrib/docker-build.sh --- contrib/Dockerfile-ub1804 | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 contrib/Dockerfile-ub1804 diff --git a/contrib/Dockerfile-ub1804 b/contrib/Dockerfile-ub1804 new file mode 100644 index 0000000..3fc28a5 --- /dev/null +++ b/contrib/Dockerfile-ub1804 @@ -0,0 +1,25 @@ +## -*- mode: dockerfile -*- +FROM ubuntu:18.04 +ARG DEBIAN_FRONTEND=noninteractive +RUN apt-get update -q && apt-get install -yy \ + asciidoc \ + git \ + gobject-introspection \ + libgirepository1.0-dev \ + libpolkit-gobject-1-dev \ + locales \ + libudev-dev \ + libumockdev-dev \ + libsystemd-dev \ + meson \ + pkg-config \ + policykit-1 \ + python3-dbus \ + python3-dbusmock \ + udev \ + umockdev \ + systemd \ + && rm -rf /var/lib/apt/lists/* + +RUN mkdir /src +WORKDIR /src -- 2.18.1 From 65b3b3b3438a6923a3cc2f8a77cbee05a52ae10c Mon Sep 17 00:00:00 2001 From: Christian Kellner Date: Wed, 12 Dec 2018 14:54:03 +0100 Subject: [PATCH 14/14] test: explicitly include stdlib.h for exit(3) The exit(3) function is declared in so include that explicitly wherever needed. --- tests/test-common.c | 1 + tests/test-journal.c | 1 + tests/test-logging.c | 1 + 3 files changed, 3 insertions(+) diff --git a/tests/test-common.c b/tests/test-common.c index 415117e..2cb174d 100644 --- a/tests/test-common.c +++ b/tests/test-common.c @@ -41,6 +41,7 @@ #include #include #include +#include #include #include #include diff --git a/tests/test-journal.c b/tests/test-journal.c index 2398f15..752f6f8 100644 --- a/tests/test-journal.c +++ b/tests/test-journal.c @@ -31,6 +31,7 @@ #include #include +#include #include #include diff --git a/tests/test-logging.c b/tests/test-logging.c index 722f6e9..1e54cf6 100644 --- a/tests/test-logging.c +++ b/tests/test-logging.c @@ -36,6 +36,7 @@ #include #include +#include #include typedef struct _LogData -- 2.18.1