From d7f149c1aae0e277fabfe3bb6441e333f867444f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 12 May 2017 15:20:54 -0400 Subject: [PATCH 1/5] tests: make sure that our headers are valid c++ This adds a meson test because it fits nicely into the existing framework. It should be enough to run this test once in a while, so I don't think it's crucial to also have it under autotools. --- src/systemd/meson.build | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/systemd/meson.build b/src/systemd/meson.build index 43fd0130b9..75435acb14 100644 --- a/src/systemd/meson.build +++ b/src/systemd/meson.build @@ -38,16 +38,17 @@ install_headers( ############################################################ -opts = [[], - ['-ansi'], - ['-std=iso9899:1990']] +opts = [['c'], + ['c', '-ansi'], + ['c', '-std=iso9899:1990'], + ['c++']] foreach header : _systemd_headers foreach opt : opts - name = ''.join([header] + opt) + name = ''.join([header, ':'] + opt) test('cc-' + name, check_compilation_sh, - args : cc.cmd_array() + ['-x', 'c', '-c'] + opt + + args : cc.cmd_array() + ['-c', '-x'] + opt + ['-Werror', '-include', join_paths(meson.current_source_dir(), header)]) endforeach From afc5fc1ffa51f9d24e65b953899b15e76c4dad98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 13 May 2017 11:44:51 -0400 Subject: [PATCH 2/5] tree-wide: drop assert.h includes We provide an independent reimplementation in macro.h, and that's the one we want to use. Including the system header is unnecessary and confusing. --- src/basic/macro.h | 1 - src/boot/bootctl.c | 1 - src/journal/journal-qrcode.c | 2 +- src/libsystemd-network/test-ipv4ll.c | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/basic/macro.h b/src/basic/macro.h index 6b2aeb933f..a51562db35 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -19,7 +19,6 @@ along with systemd; If not, see . ***/ -#include #include #include #include diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index 3358dc32a5..8efa605089 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -18,7 +18,6 @@ along with systemd; If not, see . ***/ -#include #include #include #include diff --git a/src/journal/journal-qrcode.c b/src/journal/journal-qrcode.c index e38730d65c..5ee10498d1 100644 --- a/src/journal/journal-qrcode.c +++ b/src/journal/journal-qrcode.c @@ -17,7 +17,6 @@ along with systemd; If not, see . ***/ -#include #include #include #include @@ -25,6 +24,7 @@ #include #include "journal-qrcode.h" +#include "macro.h" #define WHITE_ON_BLACK "\033[40;37;1m" #define NORMAL "\033[0m" diff --git a/src/libsystemd-network/test-ipv4ll.c b/src/libsystemd-network/test-ipv4ll.c index fe70697075..9b3fde1b26 100644 --- a/src/libsystemd-network/test-ipv4ll.c +++ b/src/libsystemd-network/test-ipv4ll.c @@ -17,7 +17,6 @@ along with systemd; If not, see . ***/ -#include #include #include #include From 3bf47e73929dbc9361574af1df84e437901a5538 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 13 May 2017 11:47:36 -0400 Subject: [PATCH 3/5] test-ipv4ll: use assert_se consistently We use assert_se in tests so that the asserts get evaluated even if compiled with NDEBUG. --- src/libsystemd-network/test-ipv4ll.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libsystemd-network/test-ipv4ll.c b/src/libsystemd-network/test-ipv4ll.c index 9b3fde1b26..89864fd39c 100644 --- a/src/libsystemd-network/test-ipv4ll.c +++ b/src/libsystemd-network/test-ipv4ll.c @@ -71,10 +71,10 @@ int arp_send_probe(int fd, int ifindex, be32_t pa, const struct ether_addr *ha) { struct ether_arp ea = {}; - assert(fd >= 0); - assert(ifindex > 0); - assert(pa != 0); - assert(ha); + assert_se(fd >= 0); + assert_se(ifindex > 0); + assert_se(pa != 0); + assert_se(ha); return arp_network_send_raw_socket(fd, ifindex, &ea); } @@ -83,10 +83,10 @@ int arp_send_announcement(int fd, int ifindex, be32_t pa, const struct ether_addr *ha) { struct ether_arp ea = {}; - assert(fd >= 0); - assert(ifindex > 0); - assert(pa != 0); - assert(ha); + assert_se(fd >= 0); + assert_se(ifindex > 0); + assert_se(pa != 0); + assert_se(ha); return arp_network_send_raw_socket(fd, ifindex, &ea); } From 94e2523b702c4490950017171902ea6bc8f776cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 13 May 2017 13:23:28 -0400 Subject: [PATCH 4/5] tests,meson: add test-bus-vtable, compiled as C and C++ This test is mostly a compilation test that checks that various defines in sd-bus-vtable.h are valid C++. The code is executed, but the results are not checked (apart from sd-bus functions not returning an error). test-bus-objects contains pretty extensive tests for this functionality. The C++ version is only added to meson, since it's simpler there. Because of the .cc extension, meson will compile the executable with c++. This test is necessary to properly check the macros in sd-bus-vtable.h. Just running the headers through g++ is not enough, because the macros are not exercised. Follow-up for #5941. --- .gitignore | 2 + Makefile.am | 7 +++ meson.build | 6 ++ src/libsystemd/sd-bus/test-bus-vtable-cc.cc | 1 + src/libsystemd/sd-bus/test-bus-vtable.c | 67 +++++++++++++++++++++ src/test/meson.build | 12 ++++ 6 files changed, 95 insertions(+) create mode 120000 src/libsystemd/sd-bus/test-bus-vtable-cc.cc create mode 100644 src/libsystemd/sd-bus/test-bus-vtable.c diff --git a/.gitignore b/.gitignore index 7cbf0a9e0c..3864249848 100644 --- a/.gitignore +++ b/.gitignore @@ -160,6 +160,8 @@ /test-bus-server /test-bus-signature /test-bus-track +/test-bus-vtable +/test-bus-vtable-cc /test-bus-zero-copy /test-calendarspec /test-cap-list diff --git a/Makefile.am b/Makefile.am index 771efa4f8e..3c042600b7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -3461,6 +3461,7 @@ tests += \ test-bus-zero-copy \ test-bus-introspect \ test-bus-objects \ + test-bus-vtable \ test-bus-error \ test-bus-creds \ test-bus-gvariant \ @@ -3530,6 +3531,12 @@ test_bus_objects_SOURCES = \ test_bus_objects_LDADD = \ libsystemd-shared.la +test_bus_vtable_SOURCES = \ + src/libsystemd/sd-bus/test-bus-vtable.c + +test_bus_vtable_LDADD = \ + libsystemd-shared.la + test_bus_error_SOURCES = \ src/libsystemd/sd-bus/test-bus-error.c diff --git a/meson.build b/meson.build index bd05757b87..0f90da1ee9 100644 --- a/meson.build +++ b/meson.build @@ -224,6 +224,12 @@ cc = meson.get_compiler('c') pkgconfig = import('pkgconfig') check_compilation_sh = find_program('tools/meson-check-compilation.sh') +cxx = find_program('c++', required : false) +if cxx.found() + # Used only for tests + add_languages('cpp') +endif + foreach arg : ['-Wundef', '-Wlogical-op', '-Wmissing-include-dirs', diff --git a/src/libsystemd/sd-bus/test-bus-vtable-cc.cc b/src/libsystemd/sd-bus/test-bus-vtable-cc.cc new file mode 120000 index 0000000000..abee398630 --- /dev/null +++ b/src/libsystemd/sd-bus/test-bus-vtable-cc.cc @@ -0,0 +1 @@ +test-bus-vtable.c \ No newline at end of file diff --git a/src/libsystemd/sd-bus/test-bus-vtable.c b/src/libsystemd/sd-bus/test-bus-vtable.c new file mode 100644 index 0000000000..2d894ccac9 --- /dev/null +++ b/src/libsystemd/sd-bus/test-bus-vtable.c @@ -0,0 +1,67 @@ +#include +#include + +/* We use system assert.h here, because we don't want to keep macro.h and log.h C++ compatible */ +#undef NDEBUG +#include + +#include "sd-bus-vtable.h" + +#define DEFAULT_BUS_PATH "unix:path=/run/dbus/system_bus_socket" + +struct context { + bool quit; + char *something; + char *automatic_string_property; + uint32_t automatic_integer_property; +}; + +static int handler(sd_bus_message *m, void *userdata, sd_bus_error *error) { + return 1; +} + +static int get_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { + return 1; +} + +static int set_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *value, void *userdata, sd_bus_error *error) { + return 1; +} + +static const sd_bus_vtable vtable[] = { + SD_BUS_VTABLE_START(0), + SD_BUS_METHOD("AlterSomething", "s", "s", handler, 0), + SD_BUS_METHOD("Exit", "", "", handler, 0), + SD_BUS_METHOD_WITH_OFFSET("AlterSomething2", "s", "s", handler, 200, 0), + SD_BUS_METHOD_WITH_OFFSET("Exit2", "", "", handler, 200, 0), + SD_BUS_WRITABLE_PROPERTY("Something", "s", get_handler, set_handler, 0, 0), + SD_BUS_WRITABLE_PROPERTY("AutomaticStringProperty", "s", NULL, NULL, + offsetof(struct context, automatic_string_property), 0), + SD_BUS_WRITABLE_PROPERTY("AutomaticIntegerProperty", "u", NULL, NULL, + offsetof(struct context, automatic_integer_property), 0), + SD_BUS_METHOD("NoOperation", NULL, NULL, NULL, 0), + SD_BUS_SIGNAL("DummySignal", "b", 0), + SD_BUS_SIGNAL("DummySignal2", "so", 0), + SD_BUS_VTABLE_END +}; + +static void test_vtable(void) { + sd_bus *bus = NULL; + struct context c = {}; + + assert(sd_bus_new(&bus) >= 0); + + assert(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtable", vtable, &c) >= 0); + assert(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.testVtable2", vtable, &c) >= 0); + + assert(sd_bus_set_address(bus, DEFAULT_BUS_PATH) >= 0); + assert(sd_bus_start(bus) >= 0); + + sd_bus_unref(bus); +} + +int main(int argc, char **argv) { + test_vtable(); + + return 0; +} diff --git a/src/test/meson.build b/src/test/meson.build index 4ae1210fe1..05d0d1db3d 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -734,6 +734,10 @@ tests += [ [], [threads]], + [['src/libsystemd/sd-bus/test-bus-vtable.c'], + [], + []], + [['src/libsystemd/sd-bus/test-bus-gvariant.c'], [], [libglib, @@ -790,6 +794,14 @@ tests += [ '', 'manual'], ] +if cxx.found() + tests += [ + [['src/libsystemd/sd-bus/test-bus-vtable-cc.cc'], + [], + []] + ] +endif + ############################################################ tests += [ From 0f771be9caddd7b212e0df5f6186dd687f1b8da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 14 May 2017 15:09:29 -0400 Subject: [PATCH 5/5] test-bus-vtable: add SD_BUS_PROPERTY Without cc9daff228, this results in: src/libsystemd/sd-bus/test-bus-vtable-cc.cc:56:1: sorry, unimplemented: non-trivial designated initializers not supported }; ^ --- src/libsystemd/sd-bus/test-bus-vtable.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libsystemd/sd-bus/test-bus-vtable.c b/src/libsystemd/sd-bus/test-bus-vtable.c index 2d894ccac9..31c35e8540 100644 --- a/src/libsystemd/sd-bus/test-bus-vtable.c +++ b/src/libsystemd/sd-bus/test-bus-vtable.c @@ -20,6 +20,10 @@ static int handler(sd_bus_message *m, void *userdata, sd_bus_error *error) { return 1; } +static int value_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { + return 1; +} + static int get_handler(sd_bus *bus, const char *path, const char *interface, const char *property, sd_bus_message *reply, void *userdata, sd_bus_error *error) { return 1; } @@ -34,6 +38,12 @@ static const sd_bus_vtable vtable[] = { SD_BUS_METHOD("Exit", "", "", handler, 0), SD_BUS_METHOD_WITH_OFFSET("AlterSomething2", "s", "s", handler, 200, 0), SD_BUS_METHOD_WITH_OFFSET("Exit2", "", "", handler, 200, 0), + SD_BUS_PROPERTY("Value", "s", value_handler, 10, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("Value2", "s", value_handler, 10, SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), + SD_BUS_PROPERTY("Value3", "s", value_handler, 10, SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("Value4", "s", value_handler, 10, 0), + SD_BUS_PROPERTY("AnExplicitProperty", "s", NULL, offsetof(struct context, something), + SD_BUS_VTABLE_PROPERTY_EXPLICIT|SD_BUS_VTABLE_PROPERTY_EMITS_INVALIDATION), SD_BUS_WRITABLE_PROPERTY("Something", "s", get_handler, set_handler, 0, 0), SD_BUS_WRITABLE_PROPERTY("AutomaticStringProperty", "s", NULL, NULL, offsetof(struct context, automatic_string_property), 0),