bus: synthesize timeout message errors instead of returning error codes

10 files changed, 177 insertions, 42 deletions

diff --git a/TODO b/TODO
index d862dfb504..14ed4b43ca 100644
--- a/TODO
+++ b/TODO
@@ -29,6 +29,22 @@ Fedora 19:
 
 Features:
 
+* libsystemd-bus:
+  - default policy (allow uid == 0 and our own uid)
+  - enforce alignment of pointers passed in
+  - negotiation for attach attributes
+  - verify that the PID doesn't change for existing busses
+  - when kdbus doesn't take our message without memfds, try again with memfds
+  - kdbus: generate correct bloom filter for matches
+  - implement translator service
+  - port systemd to new library
+  - implement busname unit type in systemd
+  - move to gvariant
+  - minimal locking around the memfd cache
+  - keep the connection fds around as long as the bus is open
+  - make ref counting atomic
+  - merge busctl into systemctl or so?
+
 * in the final killing spree, detect processes from the root directory, and
   complain loudly if they have argv[0][0] == '@' set.
   https://bugzilla.redhat.com/show_bug.cgi?id=961044
diff --git a/src/libsystemd-bus/bus-error.c b/src/libsystemd-bus/bus-error.c
index 5faa17384e..4696a88f76 100644
--- a/src/libsystemd-bus/bus-error.c
+++ b/src/libsystemd-bus/bus-error.c
@@ -142,6 +142,9 @@ int bus_error_to_errno(const sd_bus_error* e) {
 
         /* Better replce this with a gperf table */
 
+        if (!e)
+                return -EIO;
+
         if (!e->name)
                 return -EIO;
 
@@ -152,6 +155,30 @@ int bus_error_to_errno(const sd_bus_error* e) {
             streq(e->name, "org.freedesktop.DBus.Error.AccessDenied"))
                 return -EPERM;
 
+        if (streq(e->name, "org.freedesktop.DBus.Error.InvalidArgs"))
+                return -EINVAL;
+
+        if (streq(e->name, "org.freedesktop.DBus.Error.UnixProcessIdUnknown"))
+                return -ESRCH;
+
+        if (streq(e->name, "org.freedesktop.DBus.Error.FileNotFound"))
+                return -ENOENT;
+
+        if (streq(e->name, "org.freedesktop.DBus.Error.FileExists"))
+                return -EEXIST;
+
+        if (streq(e->name, "org.freedesktop.DBus.Error.Timeout"))
+                return -ETIMEDOUT;
+
+        if (streq(e->name, "org.freedesktop.DBus.Error.IOError"))
+                return -EIO;
+
+        if (streq(e->name, "org.freedesktop.DBus.Error.Disconnected"))
+                return -ECONNRESET;
+
+        if (streq(e->name, "org.freedesktop.DBus.Error.NotSupported"))
+                return -ENOTSUP;
+
         return -EIO;
 }
 
@@ -159,13 +186,54 @@ int bus_error_from_errno(sd_bus_error *e, int error) {
         if (!e)
                 return error;
 
-        if (error == -ENOMEM)
-                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.NoMemory", strerror(-error));
-        else if (error == -EPERM || error == -EACCES)
-                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.AccessDenied", strerror(-error));
-        else
-                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.Failed", "Operation failed");
+        switch (error) {
+
+        case -ENOMEM:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.NoMemory", "Out of memory");
+                break;
+
+        case -EPERM:
+        case -EACCES:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.AccessDenied", "Access denied");
+                break;
+
+        case -EINVAL:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.InvalidArgs", "Invalid argument");
+                break;
+
+        case -ESRCH:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.UnixProcessIdUnknown", "No such process");
+                break;
+
+        case -ENOENT:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.FileNotFound", "File not found");
+                break;
+
+        case -EEXIST:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.FileExists", "File exists");
+                break;
+
+        case -ETIMEDOUT:
+        case -ETIME:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.Timeout", "Timed out");
+                break;
+
+        case -EIO:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.IOError", "Input/output error");
+                break;
+
+        case -ENETRESET:
+        case -ECONNABORTED:
+        case -ECONNRESET:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.Disconnected", "Disconnected");
+                break;
+
+        case -ENOTSUP:
+                sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.NotSupported", "Not supported");
+                break;
+        }
 
+        sd_bus_error_set_const(e, "org.freedesktop.DBus.Error.Failed", "Operation failed");
         return error;
 }
 
diff --git a/src/libsystemd-bus/bus-match.c b/src/libsystemd-bus/bus-match.c
index 501a38df70..ed871d9d77 100644
--- a/src/libsystemd-bus/bus-match.c
+++ b/src/libsystemd-bus/bus-match.c
@@ -199,7 +199,6 @@ static bool value_node_same(
 int bus_match_run(
                 sd_bus *bus,
                 struct bus_match_node *node,
-                int ret,
                 sd_bus_message *m) {
 
 
@@ -230,7 +229,7 @@ int bus_match_run(
                  * we won't call any. The children of the root node
                  * are compares or leaves, they will automatically
                  * call their siblings. */
-                return bus_match_run(bus, node->child, ret, m);
+                return bus_match_run(bus, node->child, m);
 
         case BUS_MATCH_VALUE:
 
@@ -240,7 +239,7 @@ int bus_match_run(
                  * automatically call their siblings */
 
                 assert(node->child);
-                return bus_match_run(bus, node->child, ret, m);
+                return bus_match_run(bus, node->child, m);
 
         case BUS_MATCH_LEAF:
 
@@ -257,11 +256,11 @@ int bus_match_run(
 
                 /* Run the callback. And then invoke siblings. */
                 assert(node->leaf.callback);
-                r = node->leaf.callback(bus, ret, m, node->leaf.userdata);
+                r = node->leaf.callback(bus, m, node->leaf.userdata);
                 if (r != 0)
                         return r;
 
-                return bus_match_run(bus, node->next, ret, m);
+                return bus_match_run(bus, node->next, m);
 
         case BUS_MATCH_MESSAGE_TYPE:
                 test_u8 = m->header->type;
@@ -318,7 +317,7 @@ int bus_match_run(
                         found = NULL;
 
                 if (found) {
-                        r = bus_match_run(bus, found, ret, m);
+                        r = bus_match_run(bus, found, m);
                         if (r != 0)
                                 return r;
                 }
@@ -331,7 +330,7 @@ int bus_match_run(
                         if (!value_node_test(c, node->type, test_u8, test_str))
                                 continue;
 
-                        r = bus_match_run(bus, c, ret, m);
+                        r = bus_match_run(bus, c, m);
                         if (r != 0)
                                 return r;
                 }
@@ -341,7 +340,7 @@ int bus_match_run(
                 return 0;
 
         /* And now, let's invoke our siblings */
-        return bus_match_run(bus, node->next, ret, m);
+        return bus_match_run(bus, node->next, m);
 }
 
 static int bus_match_add_compare_value(
diff --git a/src/libsystemd-bus/bus-match.h b/src/libsystemd-bus/bus-match.h
index 075f1a9e3a..4d46cf9aff 100644
--- a/src/libsystemd-bus/bus-match.h
+++ b/src/libsystemd-bus/bus-match.h
@@ -69,7 +69,7 @@ struct bus_match_node {
         };
 };
 
-int bus_match_run(sd_bus *bus, struct bus_match_node *root, int ret, sd_bus_message *m);
+int bus_match_run(sd_bus *bus, struct bus_match_node *root, sd_bus_message *m);
 
 int bus_match_add(struct bus_match_node *root, const char *match, sd_bus_message_handler_t callback, void *userdata, struct bus_match_node **ret);
 int bus_match_remove(struct bus_match_node *root, const char *match, sd_bus_message_handler_t callback, void *userdata);
diff --git a/src/libsystemd-bus/bus-message.c b/src/libsystemd-bus/bus-message.c
index dbb3337619..c72b52d546 100644
--- a/src/libsystemd-bus/bus-message.c
+++ b/src/libsystemd-bus/bus-message.c
@@ -623,6 +623,43 @@ fail:
         return r;
 }
 
+int bus_message_new_synthetic_error(
+                sd_bus *bus,
+                uint64_t serial,
+                const sd_bus_error *e,
+                sd_bus_message **m) {
+
+        sd_bus_message *t;
+        int r;
+
+        assert(sd_bus_error_is_set(e));
+        assert(m);
+
+        t = message_new(bus, SD_BUS_MESSAGE_TYPE_METHOD_ERROR);
+        if (!t)
+                return -ENOMEM;
+
+        t->header->flags |= SD_BUS_MESSAGE_NO_REPLY_EXPECTED;
+        t->reply_serial = serial;
+
+        r = message_append_field_uint32(t, SD_BUS_MESSAGE_HEADER_REPLY_SERIAL, t->reply_serial);
+        if (r < 0)
+                goto fail;
+
+        if (bus && bus->unique_name) {
+                r = message_append_field_string(t, SD_BUS_MESSAGE_HEADER_DESTINATION, SD_BUS_TYPE_STRING, bus->unique_name, &t->destination);
+                if (r < 0)
+                        goto fail;
+        }
+
+        *m = t;
+        return 0;
+
+fail:
+        message_free(t);
+        return r;
+}
+
 sd_bus_message* sd_bus_message_ref(sd_bus_message *m) {
         if (!m)
                 return NULL;
@@ -4240,3 +4277,12 @@ int bus_header_message_size(struct bus_header *h, size_t *sum) {
         *sum = sizeof(struct bus_header) + ALIGN8(fs) + bs;
         return 0;
 }
+
+int bus_message_to_errno(sd_bus_message *m) {
+        assert(m);
+
+        if (m->header->type != SD_BUS_MESSAGE_TYPE_METHOD_ERROR)
+                return 0;
+
+        return bus_error_to_errno(&m->error);
+}
diff --git a/src/libsystemd-bus/bus-message.h b/src/libsystemd-bus/bus-message.h
index 44390c6b50..2fb11ea3b1 100644
--- a/src/libsystemd-bus/bus-message.h
+++ b/src/libsystemd-bus/bus-message.h
@@ -235,3 +235,7 @@ struct bus_body_part *message_append_part(sd_bus_message *m);
 
 int bus_body_part_map(struct bus_body_part *part);
 void bus_body_part_unmap(struct bus_body_part *part);
+
+int bus_message_to_errno(sd_bus_message *m);
+
+int bus_message_new_synthetic_error(sd_bus *bus, uint64_t serial, const sd_bus_error *e, sd_bus_message **m);
diff --git a/src/libsystemd-bus/sd-bus.c b/src/libsystemd-bus/sd-bus.c
index 2537ba52d5..7b937d9999 100644
--- a/src/libsystemd-bus/sd-bus.c
+++ b/src/libsystemd-bus/sd-bus.c
@@ -229,18 +229,18 @@ int sd_bus_set_anonymous(sd_bus *bus, int b) {
         return 0;
 }
 
-static int hello_callback(sd_bus *bus, int error, sd_bus_message *reply, void *userdata) {
+static int hello_callback(sd_bus *bus, sd_bus_message *reply, void *userdata) {
         const char *s;
         int r;
 
         assert(bus);
         assert(bus->state == BUS_HELLO);
-
-        if (error != 0)
-                return -error;
-
         assert(reply);
 
+        r = bus_message_to_errno(reply);
+        if (r < 0)
+                return r;
+
         r = sd_bus_message_read(reply, "s", &s);
         if (r < 0)
                 return r;
@@ -1523,6 +1523,7 @@ int sd_bus_get_timeout(sd_bus *bus, uint64_t *timeout_usec) {
 }
 
 static int process_timeout(sd_bus *bus) {
+        _cleanup_bus_message_unref_ sd_bus_message* m = NULL;
         struct reply_callback *c;
         usec_t n;
         int r;
@@ -1537,10 +1538,18 @@ static int process_timeout(sd_bus *bus) {
         if (c->timeout > n)
                 return 0;
 
+        r = bus_message_new_synthetic_error(
+                        bus,
+                        c->serial,
+                        &SD_BUS_ERROR_MAKE("org.freedesktop.DBus.Error.Timeout", "Timed out"),
+                        &m);
+        if (r < 0)
+                return r;
+
         assert_se(prioq_pop(bus->reply_callbacks_prioq) == c);
         hashmap_remove(bus->reply_callbacks, &c->serial);
 
-        r = c->callback(bus, ETIMEDOUT, NULL, c->userdata);
+        r = c->callback(bus, m, c->userdata);
         free(c);
 
         return r < 0 ? r : 1;
@@ -1590,7 +1599,7 @@ static int process_reply(sd_bus *bus, sd_bus_message *m) {
         if (r < 0)
                 return r;
 
-        r = c->callback(bus, 0, m, c->userdata);
+        r = c->callback(bus, m, c->userdata);
         free(c);
 
         return r;
@@ -1621,7 +1630,7 @@ static int process_filter(sd_bus *bus, sd_bus_message *m) {
                         if (r < 0)
                                 return r;
 
-                        r = l->callback(bus, 0, m, l->userdata);
+                        r = l->callback(bus, m, l->userdata);
                         if (r != 0)
                                 return r;
 
@@ -1641,7 +1650,7 @@ static int process_match(sd_bus *bus, sd_bus_message *m) {
         do {
                 bus->match_callbacks_modified = false;
 
-                r = bus_match_run(bus, &bus->match_callbacks, 0, m);
+                r = bus_match_run(bus, &bus->match_callbacks, m);
                 if (r != 0)
                         return r;
 
@@ -1734,7 +1743,7 @@ static int process_object(sd_bus *bus, sd_bus_message *m) {
                         if (r < 0)
                                 return r;
 
-                        r = c->callback(bus, 0, m, c->userdata);
+                        r = c->callback(bus, m, c->userdata);
                         if (r != 0)
                                 return r;
 
@@ -1764,7 +1773,7 @@ static int process_object(sd_bus *bus, sd_bus_message *m) {
                                 if (r < 0)
                                         return r;
 
-                                r = c->callback(bus, 0, m, c->userdata);
+                                r = c->callback(bus, m, c->userdata);
                                 if (r != 0)
                                         return r;
 
diff --git a/src/libsystemd-bus/test-bus-chat.c b/src/libsystemd-bus/test-bus-chat.c
index f457c8f88a..f308eddbb0 100644
--- a/src/libsystemd-bus/test-bus-chat.c
+++ b/src/libsystemd-bus/test-bus-chat.c
@@ -35,17 +35,17 @@
 #include "bus-match.h"
 #include "bus-internal.h"
 
-static int match_callback(sd_bus *bus, int error, sd_bus_message *m, void *userdata) {
+static int match_callback(sd_bus *bus, sd_bus_message *m, void *userdata) {
         log_info("Match triggered! interface=%s member=%s", strna(sd_bus_message_get_interface(m)), strna(sd_bus_message_get_member(m)));
         return 0;
 }
 
-static int object_callback(sd_bus *bus, int error, sd_bus_message *m, void *userdata) {
+static int object_callback(sd_bus *bus, sd_bus_message *m, void *userdata) {
         int r;
 
         assert(bus);
 
-        if (error != 0)
+        if (sd_bus_message_is_method_error(m, NULL))
                 return 0;
 
         if (sd_bus_message_is_method_call(m, "org.object.test", "Foobar")) {
@@ -356,10 +356,10 @@ finish:
         return INT_TO_PTR(r);
 }
 
-static int quit_callback(sd_bus *b, int ret, sd_bus_message *m, void *userdata) {
+static int quit_callback(sd_bus *b, sd_bus_message *m, void *userdata) {
         bool *x = userdata;
 
-        log_error("Quit callback: %s", strerror(ret));
+        log_error("Quit callback: %s", strerror(bus_message_to_errno(m)));
 
         *x = 1;
         return 1;
diff --git a/src/libsystemd-bus/test-bus-match.c b/src/libsystemd-bus/test-bus-match.c
index 9cf994009d..605d86a85c 100644
--- a/src/libsystemd-bus/test-bus-match.c
+++ b/src/libsystemd-bus/test-bus-match.c
@@ -30,7 +30,7 @@
 
 static bool mask[32];
 
-static int filter(sd_bus *b, int ret, sd_bus_message *m, void *userdata) {
+static int filter(sd_bus *b, sd_bus_message *m, void *userdata) {
         log_info("Ran %i", PTR_TO_INT(userdata));
         mask[PTR_TO_INT(userdata)] = true;
         return 0;
@@ -85,7 +85,7 @@ int main(int argc, char *argv[]) {
         assert_se(bus_message_seal(m, 1) >= 0);
 
         zero(mask);
-        assert_se(bus_match_run(NULL, &root, 0, m) == 0);
+        assert_se(bus_match_run(NULL, &root, m) == 0);
         assert_se(mask_contains((unsigned[]) { 9, 8, 7, 5, 10, 12, 13, 14 }, 8));
 
         assert_se(bus_match_remove(&root, "member='waldo',path='/foo/bar'", filter, INT_TO_PTR(8)) > 0);
@@ -95,7 +95,7 @@ int main(int argc, char *argv[]) {
         bus_match_dump(&root, 0);
 
         zero(mask);
-        assert_se(bus_match_run(NULL, &root, 0, m) == 0);
+        assert_se(bus_match_run(NULL, &root, m) == 0);
         assert_se(mask_contains((unsigned[]) { 9, 5, 10, 12, 14, 7 }, 6));
 
         for (i = 0; i < _BUS_MATCH_NODE_TYPE_MAX; i++) {
diff --git a/src/systemd/sd-bus.h b/src/systemd/sd-bus.h
index 4c3c26d611..85deacf63f 100644
--- a/src/systemd/sd-bus.h
+++ b/src/systemd/sd-bus.h
@@ -41,13 +41,6 @@ extern "C" {
 #  endif
 #endif
 
-/* TODO:
- * - merge busctl into systemctl or so?
- * - default policy (allow uid == 0 and our own uid)
- * - enforce alignment of pointers passed in
- * - negotiation for attach attributes
- */
-
 typedef struct sd_bus sd_bus;
 typedef struct sd_bus_message sd_bus_message;
 
@@ -57,7 +50,7 @@ typedef struct {
         int need_free;
 } sd_bus_error;
 
-typedef int (*sd_bus_message_handler_t)(sd_bus *bus, int ret, sd_bus_message *m, void *userdata);
+typedef int (*sd_bus_message_handler_t)(sd_bus *bus, sd_bus_message *m, void *userdata);
 
 /* Connections */