[systemd-commits] 3 commits - src/libsystemd src/shared
David Herrmann
dvdhrm at kemper.freedesktop.org
Sat Apr 11 04:33:59 PDT 2015
src/libsystemd/sd-bus/bus-util.c | 128 +++++++++++++++++++++++++++++++
src/libsystemd/sd-bus/bus-util.h | 3
src/libsystemd/sd-bus/test-bus-marshal.c | 11 ++
src/shared/bus-label.c | 21 ++---
src/shared/bus-label.h | 9 +-
src/shared/hashmap.c | 33 ++++---
src/shared/hashmap.h | 26 +++---
7 files changed, 192 insertions(+), 39 deletions(-)
New commits:
commit 98a4c30ba117bddad3cd0934042721621a328e9a
Author: David Herrmann <dh.herrmann at gmail.com>
Date: Fri Apr 10 17:44:30 2015 +0200
bus: implement bus_path_{en,de}code_unique()
Whenever we provide a bus API that allows clients to create and manage
server-side objects, we need to provide a unique name for these objects.
There are two ways to provide them:
1) Let the server choose a name and return it as method reply.
2) Let the client pass its name of choice in the method arguments.
The first method is the easiest one to implement. However, it suffers from
a race condition: If a client creates an object asynchronously, it cannot
destroy that object until it received the method reply. It cannot know the
name of the new object, thus, it cannot destroy it. Furthermore, this
method enforces a round-trip. If the client _depends_ on the method call
to succeed (eg., it would close() the connection if it failed), the client
usually has no reason to wait for the method reply. Instead, the client
can immediately schedule further method calls on the newly created object
(in case the API guarantees in-order method-call handling).
The second method fixes both problems: The client passes an object name
with the method-call. The server uses it to create the object. Therefore,
the client can schedule object destruction even if the object-creation
hasn't finished, yet (again, requiring in-order method-call handling).
Furthermore, the client can schedule further method calls on the newly
created object, before the constructor returned.
There're two problems to solve, though:
1) Object names are usually defined via dbus object paths, which are
usually globally namespaced. Therefore, multiple clients must be able
to choose unique object names without interference.
2) If multiple libraries share the same bus connection, they must be
able to choose unique object names without interference.
The first problem is solved easily by prefixing a name with the
unique-bus-name of a connection. The server side must enforce this and
reject any other name.
The second problem is solved by providing unique suffixes from within
sd-bus. As long as sd-bus always returns a fresh new ID, if requested,
multiple libraries will never interfere. This implementation re-uses
bus->cookie as ID generator, which already provides unique IDs for each
bus connection.
This patch introduces two new helpers:
bus_path_encode_unique(sd_bus *bus,
const char *prefix,
const char *sender_id,
const char *external_id,
char **ret_path);
This creates a new object-path via the template
'/prefix/sender_id/external_id'. That is, it appends two new labels to
the given prefix. If 'sender_id' is NULL, it will use
bus->unique_name, if 'external_id' is NULL, it will allocate a fresh,
unique cookie from bus->cookie.
bus_path_decode_unique(const char *path,
const char *prefix,
char **ret_sender,
char **ret_external);
This reverses what bus_path_encode_unique() did. It parses 'path' from
the template '/prefix/sender/external' and returns both suffix-labels
in 'ret_sender' and 'ret_external'. In case the template does not
match, 0 is returned and both output arguments are set to NULL.
Otherwise, 1 is returned and the output arguments contain the decoded
labels.
Note: Client-side allocated IDs are inspired by the Wayland protocol
(which itself was inspired by X11). Wayland uses those IDs heavily
to avoid round-trips. Clients can create server-side objects and
send method calls without any round-trip and waiting for any object
IDs to be returned. But unlike Wayland, DBus uses gobally namespaced
object names. Therefore, we have to add the extra step by adding the
unique-name of the bus connection.
diff --git a/src/libsystemd/sd-bus/bus-util.c b/src/libsystemd/sd-bus/bus-util.c
index fcb14a4..abb4829 100644
--- a/src/libsystemd/sd-bus/bus-util.c
+++ b/src/libsystemd/sd-bus/bus-util.c
@@ -34,6 +34,7 @@
#include "sd-bus.h"
#include "bus-error.h"
+#include "bus-label.h"
#include "bus-message.h"
#include "bus-util.h"
#include "bus-internal.h"
@@ -1891,3 +1892,130 @@ int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet) {
return 0;
}
+
+/**
+ * bus_path_encode_unique() - encode unique object path
+ * @b: bus connection or NULL
+ * @prefix: object path prefix
+ * @sender_id: unique-name of client, or NULL
+ * @external_id: external ID to be chosen by client, or NULL
+ * @ret_path: storage for encoded object path pointer
+ *
+ * Whenever we provide a bus API that allows clients to create and manage
+ * server-side objects, we need to provide a unique name for these objects. If
+ * we let the server choose the name, we suffer from a race condition: If a
+ * client creates an object asynchronously, it cannot destroy that object until
+ * it received the method reply. It cannot know the name of the new object,
+ * thus, it cannot destroy it. Furthermore, it enforces a round-trip.
+ *
+ * Therefore, many APIs allow the client to choose the unique name for newly
+ * created objects. There're two problems to solve, though:
+ * 1) Object names are usually defined via dbus object paths, which are
+ * usually globally namespaced. Therefore, multiple clients must be able
+ * to choose unique object names without interference.
+ * 2) If multiple libraries share the same bus connection, they must be
+ * able to choose unique object names without interference.
+ * The first problem is solved easily by prefixing a name with the
+ * unique-bus-name of a connection. The server side must enforce this and
+ * reject any other name. The second problem is solved by providing unique
+ * suffixes from within sd-bus.
+ *
+ * This helper allows clients to create unique object-paths. It uses the
+ * template '/prefix/sender_id/external_id' and returns the new path in
+ * @ret_path (must be freed by the caller).
+ * If @sender_id is NULL, the unique-name of @b is used. If @external_id is
+ * NULL, this function allocates a unique suffix via @b (by requesting a new
+ * cookie). If both @sender_id and @external_id are given, @b can be passed as
+ * NULL.
+ *
+ * Returns: 0 on success, negative error code on failure.
+ */
+int bus_path_encode_unique(sd_bus *b, const char *prefix, const char *sender_id, const char *external_id, char **ret_path) {
+ _cleanup_free_ char *sender_label = NULL, *external_label = NULL;
+ char external_buf[DECIMAL_STR_MAX(uint64_t)], *p;
+ int r;
+
+ assert_return(b || (sender_id && external_id), -EINVAL);
+ assert_return(object_path_is_valid(prefix), -EINVAL);
+ assert_return(ret_path, -EINVAL);
+
+ if (!sender_id) {
+ r = sd_bus_get_unique_name(b, &sender_id);
+ if (r < 0)
+ return r;
+ }
+
+ if (!external_id) {
+ xsprintf(external_buf, "%"PRIu64, ++b->cookie);
+ external_id = external_buf;
+ }
+
+ sender_label = bus_label_escape(sender_id);
+ if (!sender_label)
+ return -ENOMEM;
+
+ external_label = bus_label_escape(external_id);
+ if (!external_label)
+ return -ENOMEM;
+
+ p = strjoin(prefix, "/", sender_label, "/", external_label, NULL);
+ if (!p)
+ return -ENOMEM;
+
+ *ret_path = p;
+ return 0;
+}
+
+/**
+ * bus_path_decode_unique() - decode unique object path
+ * @path: object path to decode
+ * @prefix: object path prefix
+ * @ret_sender: output parameter for sender-id label
+ * @ret_external: output parameter for external-id label
+ *
+ * This does the reverse of bus_path_encode_unique() (see its description for
+ * details). Both trailing labels, sender-id and external-id, are unescaped and
+ * returned in the given output parameters (the caller must free them).
+ *
+ * Note that this function returns 0 if the path does not match the template
+ * (see bus_path_encode_unique()), 1 if it matched.
+ *
+ * Returns: Negative error code on failure, 0 if the given object path does not
+ * match the template (return parameters are set to NULL), 1 if it was
+ * parsed successfully (return parameters contain allocated labels).
+ */
+int bus_path_decode_unique(const char *path, const char *prefix, char **ret_sender, char **ret_external) {
+ const char *p, *q;
+ char *sender, *external;
+
+ assert(object_path_is_valid(path));
+ assert(object_path_is_valid(prefix));
+ assert(ret_sender);
+ assert(ret_external);
+
+ p = object_path_startswith(path, prefix);
+ if (!p) {
+ *ret_sender = NULL;
+ *ret_external = NULL;
+ return 0;
+ }
+
+ q = strchr(p, '/');
+ if (!q) {
+ *ret_sender = NULL;
+ *ret_external = NULL;
+ return 0;
+ }
+
+ sender = bus_label_unescape_n(p, q - p);
+ external = bus_label_unescape(q + 1);
+ if (!sender || !external) {
+ free(sender);
+ free(external);
+ return -ENOMEM;
+ }
+
+ *ret_sender = sender;
+ *ret_external = external;
+ return 1;
+}
diff --git a/src/libsystemd/sd-bus/bus-util.h b/src/libsystemd/sd-bus/bus-util.h
index cc16a9d..b97729e 100644
--- a/src/libsystemd/sd-bus/bus-util.h
+++ b/src/libsystemd/sd-bus/bus-util.h
@@ -211,3 +211,6 @@ int bus_wait_for_jobs(BusWaitForJobs *d, bool quiet);
DEFINE_TRIVIAL_CLEANUP_FUNC(BusWaitForJobs*, bus_wait_for_jobs_free);
int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet);
+
+int bus_path_encode_unique(sd_bus *b, const char *prefix, const char *sender_id, const char *external_id, char **ret_path);
+int bus_path_decode_unique(const char *path, const char *prefix, char **ret_sender, char **ret_external);
diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c
index 7569ff9..f8ecadf 100644
--- a/src/libsystemd/sd-bus/test-bus-marshal.c
+++ b/src/libsystemd/sd-bus/test-bus-marshal.c
@@ -39,6 +39,16 @@
#include "bus-dump.h"
#include "bus-label.h"
+static void test_bus_path_encode_unique(void) {
+ _cleanup_free_ char *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL;
+
+ assert_se(bus_path_encode_unique(NULL, "/foo/bar", "some.sender", "a.suffix", &a) >= 0 && streq_ptr(a, "/foo/bar/some_2esender/a_2esuffix"));
+ assert_se(bus_path_decode_unique(a, "/foo/bar", &b, &c) > 0 && streq_ptr(b, "some.sender") && streq_ptr(c, "a.suffix"));
+ assert_se(bus_path_decode_unique(a, "/bar/foo", &d, &d) == 0 && !d);
+ assert_se(bus_path_decode_unique("/foo/bar/onlyOneSuffix", "/foo/bar", &d, &d) == 0 && !d);
+ assert_se(bus_path_decode_unique("/foo/bar/_/_", "/foo/bar", &d, &e) > 0 && streq_ptr(d, "") && streq_ptr(e, ""));
+}
+
static void test_bus_path_encode(void) {
_cleanup_free_ char *a = NULL, *b = NULL, *c = NULL, *d = NULL, *e = NULL, *f = NULL;
@@ -357,6 +367,7 @@ int main(int argc, char *argv[]) {
test_bus_label_escape();
test_bus_path_encode();
+ test_bus_path_encode_unique();
return 0;
}
commit ad922737f743a82ce95c5554c36e38016678e68e
Author: David Herrmann <dh.herrmann at gmail.com>
Date: Fri Apr 10 17:43:04 2015 +0200
bus: implement bus_label_unescape_n()
This is like bus_label_unescape() but takes a maximum length instead of
relying on NULL-terminated strings. This is highly useful to unescape
labels that are not at the end of a path.
diff --git a/src/shared/bus-label.c b/src/shared/bus-label.c
index 9e9eaf4..ccc9f2b 100644
--- a/src/shared/bus-label.c
+++ b/src/shared/bus-label.c
@@ -63,34 +63,35 @@ char *bus_label_escape(const char *s) {
return r;
}
-char *bus_label_unescape(const char *f) {
+char *bus_label_unescape_n(const char *f, size_t l) {
char *r, *t;
+ size_t i;
assert_return(f, NULL);
/* Special case for the empty string */
- if (streq(f, "_"))
+ if (l == 1 && *f == '_')
return strdup("");
- r = new(char, strlen(f) + 1);
+ r = new(char, l + 1);
if (!r)
return NULL;
- for (t = r; *f; f++) {
-
- if (*f == '_') {
+ for (i = 0, t = r; i < l; ++i) {
+ if (f[i] == '_') {
int a, b;
- if ((a = unhexchar(f[1])) < 0 ||
- (b = unhexchar(f[2])) < 0) {
+ if (l - i < 3 ||
+ (a = unhexchar(f[i + 1])) < 0 ||
+ (b = unhexchar(f[i + 2])) < 0) {
/* Invalid escape code, let's take it literal then */
*(t++) = '_';
} else {
*(t++) = (char) ((a << 4) | b);
- f += 2;
+ i += 2;
}
} else
- *(t++) = *f;
+ *(t++) = f[i];
}
*t = 0;
diff --git a/src/shared/bus-label.h b/src/shared/bus-label.h
index c27c851..ed1dc4e 100644
--- a/src/shared/bus-label.h
+++ b/src/shared/bus-label.h
@@ -21,5 +21,12 @@
along with systemd; If not, see <http://www.gnu.org/licenses/>.
***/
+#include <stdlib.h>
+#include <string.h>
+
char *bus_label_escape(const char *s);
-char *bus_label_unescape(const char *f);
+char *bus_label_unescape_n(const char *f, size_t l);
+
+static inline char *bus_label_unescape(const char *f) {
+ return bus_label_unescape_n(f, f ? strlen(f) : 0);
+}
commit cfe561a456c9ce61579c8e1207f9a13faf050e8a
Author: David Herrmann <dh.herrmann at gmail.com>
Date: Fri Apr 10 11:15:12 2015 +0200
hashmap: return NULL from destructor
We _always_ return NULL from destructors to allow direct assignments to
the variable holding the object. Especially on hashmaps, which treat NULL
as empty hashmap, this is pretty neat.
diff --git a/src/shared/hashmap.c b/src/shared/hashmap.c
index d8ea9d5..c7512b2 100644
--- a/src/shared/hashmap.c
+++ b/src/shared/hashmap.c
@@ -866,38 +866,41 @@ static void hashmap_free_no_clear(HashmapBase *h) {
free(h);
}
-void internal_hashmap_free(HashmapBase *h) {
+HashmapBase *internal_hashmap_free(HashmapBase *h) {
/* Free the hashmap, but nothing in it */
- if (!h)
- return;
+ if (h) {
+ internal_hashmap_clear(h);
+ hashmap_free_no_clear(h);
+ }
- internal_hashmap_clear(h);
- hashmap_free_no_clear(h);
+ return NULL;
}
-void internal_hashmap_free_free(HashmapBase *h) {
+HashmapBase *internal_hashmap_free_free(HashmapBase *h) {
/* Free the hashmap and all data objects in it, but not the
* keys */
- if (!h)
- return;
+ if (h) {
+ internal_hashmap_clear_free(h);
+ hashmap_free_no_clear(h);
+ }
- internal_hashmap_clear_free(h);
- hashmap_free_no_clear(h);
+ return NULL;
}
-void hashmap_free_free_free(Hashmap *h) {
+Hashmap *hashmap_free_free_free(Hashmap *h) {
/* Free the hashmap and all data and key objects in it */
- if (!h)
- return;
+ if (h) {
+ hashmap_clear_free_free(h);
+ hashmap_free_no_clear(HASHMAP_BASE(h));
+ }
- hashmap_clear_free_free(h);
- hashmap_free_no_clear(HASHMAP_BASE(h));
+ return NULL;
}
void internal_hashmap_clear(HashmapBase *h) {
diff --git a/src/shared/hashmap.h b/src/shared/hashmap.h
index 894f679..a03ee58 100644
--- a/src/shared/hashmap.h
+++ b/src/shared/hashmap.h
@@ -144,25 +144,25 @@ OrderedHashmap *internal_ordered_hashmap_new(const struct hash_ops *hash_ops HA
#define hashmap_new(ops) internal_hashmap_new(ops HASHMAP_DEBUG_SRC_ARGS)
#define ordered_hashmap_new(ops) internal_ordered_hashmap_new(ops HASHMAP_DEBUG_SRC_ARGS)
-void internal_hashmap_free(HashmapBase *h);
-static inline void hashmap_free(Hashmap *h) {
- internal_hashmap_free(HASHMAP_BASE(h));
+HashmapBase *internal_hashmap_free(HashmapBase *h);
+static inline Hashmap *hashmap_free(Hashmap *h) {
+ return (void*)internal_hashmap_free(HASHMAP_BASE(h));
}
-static inline void ordered_hashmap_free(OrderedHashmap *h) {
- internal_hashmap_free(HASHMAP_BASE(h));
+static inline OrderedHashmap *ordered_hashmap_free(OrderedHashmap *h) {
+ return (void*)internal_hashmap_free(HASHMAP_BASE(h));
}
-void internal_hashmap_free_free(HashmapBase *h);
-static inline void hashmap_free_free(Hashmap *h) {
- internal_hashmap_free_free(HASHMAP_BASE(h));
+HashmapBase *internal_hashmap_free_free(HashmapBase *h);
+static inline Hashmap *hashmap_free_free(Hashmap *h) {
+ return (void*)internal_hashmap_free_free(HASHMAP_BASE(h));
}
-static inline void ordered_hashmap_free_free(OrderedHashmap *h) {
- internal_hashmap_free_free(HASHMAP_BASE(h));
+static inline OrderedHashmap *ordered_hashmap_free_free(OrderedHashmap *h) {
+ return (void*)internal_hashmap_free_free(HASHMAP_BASE(h));
}
-void hashmap_free_free_free(Hashmap *h);
-static inline void ordered_hashmap_free_free_free(OrderedHashmap *h) {
- hashmap_free_free_free(PLAIN_HASHMAP(h));
+Hashmap *hashmap_free_free_free(Hashmap *h);
+static inline OrderedHashmap *ordered_hashmap_free_free_free(OrderedHashmap *h) {
+ return (void*)hashmap_free_free_free(PLAIN_HASHMAP(h));
}
HashmapBase *internal_hashmap_copy(HashmapBase *h);
More information about the systemd-commits
mailing list