[systemd-devel] [PATCH v2] build: fix problems with undefined references in tests

Lennart Poettering lennart at poettering.net
Mon Nov 25 13:53:55 PST 2013


On Mon, 25.11.13 18:26, Lukasz Skalski (l.skalski at partner.samsung.com) wrote:

> Now code form src/shared don't use code of shared libs directly.
> It solves cyclic dependencies in the same way as sd_utf_is_valid()
> from src/libsystemd-bus/sd-utf8.c and src/shared/utf8.c.

Hmm, I'd prefer just moving the code in unit-names.[ch] that needs this
into some new convenience library that links against
libsystemd-bus. Using these calls without linking against libsystem-bus
anyway makes little sense, hence that sounds like the better approach to
me...

> ---
>  src/libsystemd-bus/sd-bus.c           |   64 +----------------------------
>  src/libsystemd-bus/test-bus-marshal.c |   25 ------------
>  src/shared/unit-name.c                |    5 +--
>  src/shared/util.c                     |   72 +++++++++++++++++++++++++++++++++
>  src/shared/util.h                     |    3 ++
>  src/test/test-util.c                  |   24 +++++++++++
>  6 files changed, 103 insertions(+), 90 deletions(-)
> 
> diff --git a/src/libsystemd-bus/sd-bus.c b/src/libsystemd-bus/sd-bus.c
> index 1207d5a..3042802 100644
> --- a/src/libsystemd-bus/sd-bus.c
> +++ b/src/libsystemd-bus/sd-bus.c
> @@ -2741,73 +2741,13 @@ _public_ int sd_bus_get_tid(sd_bus *b, pid_t *tid) {
>  }
>  
>  _public_ char *sd_bus_label_escape(const char *s) {
> -        char *r, *t;
> -        const char *f;
> -
>          assert_return(s, NULL);
>  
> -        /* Escapes all chars that D-Bus' object path cannot deal
> -         * with. Can be reversed with bus_path_unescape(). We special
> -         * case the empty string. */
> -
> -        if (*s == 0)
> -                return strdup("_");
> -
> -        r = new(char, strlen(s)*3 + 1);
> -        if (!r)
> -                return NULL;
> -
> -        for (f = s, t = r; *f; f++) {
> -
> -                /* Escape everything that is not a-zA-Z0-9. We also
> -                 * escape 0-9 if it's the first character */
> -
> -                if (!(*f >= 'A' && *f <= 'Z') &&
> -                    !(*f >= 'a' && *f <= 'z') &&
> -                    !(f > s && *f >= '0' && *f <= '9')) {
> -                        *(t++) = '_';
> -                        *(t++) = hexchar(*f >> 4);
> -                        *(t++) = hexchar(*f);
> -                } else
> -                        *(t++) = *f;
> -        }
> -
> -        *t = 0;
> -
> -        return r;
> +        return bus_path_escape(s);
>  }
>  
>  _public_ char *sd_bus_label_unescape(const char *f) {
> -        char *r, *t;
> -
>          assert_return(f, NULL);
>  
> -        /* Special case for the empty string */
> -        if (streq(f, "_"))
> -                return strdup("");
> -
> -        r = new(char, strlen(f) + 1);
> -        if (!r)
> -                return NULL;
> -
> -        for (t = r; *f; f++) {
> -
> -                if (*f == '_') {
> -                        int a, b;
> -
> -                        if ((a = unhexchar(f[1])) < 0 ||
> -                            (b = unhexchar(f[2])) < 0) {
> -                                /* Invalid escape code, let's take it literal then */
> -                                *(t++) = '_';
> -                        } else {
> -                                *(t++) = (char) ((a << 4) | b);
> -                                f += 2;
> -                        }
> -                } else
> -                        *(t++) = *f;
> -        }
> -
> -        *t = 0;
> -
> -        return r;
> +        return bus_path_unescape(f);
>  }
> diff --git a/src/libsystemd-bus/test-bus-marshal.c b/src/libsystemd-bus/test-bus-marshal.c
> index da072c7..b6e2c7b 100644
> --- a/src/libsystemd-bus/test-bus-marshal.c
> +++ b/src/libsystemd-bus/test-bus-marshal.c
> @@ -39,29 +39,6 @@
>  #include "bus-util.h"
>  #include "bus-dump.h"
>  
> -static void test_bus_label_escape_one(const char *a, const char *b) {
> -        _cleanup_free_ char *t = NULL, *x = NULL, *y = NULL;
> -
> -        assert_se(t = sd_bus_label_escape(a));
> -        assert_se(streq(t, b));
> -
> -        assert_se(x = sd_bus_label_unescape(t));
> -        assert_se(streq(a, x));
> -
> -        assert_se(y = sd_bus_label_unescape(b));
> -        assert_se(streq(a, y));
> -}
> -
> -static void test_bus_label_escape(void) {
> -        test_bus_label_escape_one("foo123bar", "foo123bar");
> -        test_bus_label_escape_one("foo.bar", "foo_2ebar");
> -        test_bus_label_escape_one("foo_2ebar", "foo_5f2ebar");
> -        test_bus_label_escape_one("", "_");
> -        test_bus_label_escape_one("_", "_5f");
> -        test_bus_label_escape_one("1", "_31");
> -        test_bus_label_escape_one(":1", "_3a1");
> -}
> -
>  int main(int argc, char *argv[]) {
>          _cleanup_bus_message_unref_ sd_bus_message *m = NULL, *copy = NULL;
>          int r, boolean;
> @@ -317,7 +294,5 @@ int main(int argc, char *argv[]) {
>          assert_se(streq(c, "ccc"));
>          assert_se(streq(d, "3"));
>  
> -        test_bus_label_escape();
> -
>          return 0;
>  }
> diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
> index 2335463..bc8094d 100644
> --- a/src/shared/unit-name.c
> +++ b/src/shared/unit-name.c
> @@ -23,7 +23,6 @@
>  #include <string.h>
>  #include <assert.h>
>  
> -#include "sd-bus.h"
>  #include "path-util.h"
>  #include "util.h"
>  #include "unit-name.h"
> @@ -460,7 +459,7 @@ char *unit_dbus_path_from_name(const char *name) {
>  
>          assert(name);
>  
> -        e = sd_bus_label_escape(name);
> +        e = bus_path_escape(name);
>          if (!e)
>                  return NULL;
>  
> @@ -475,7 +474,7 @@ int unit_name_from_dbus_path(const char *path, char **name) {
>          if (!e)
>                  return -EINVAL;
>  
> -        n = sd_bus_label_unescape(e);
> +        n = bus_path_unescape(e);
>          if (!n)
>                  return -ENOMEM;
>  
> diff --git a/src/shared/util.c b/src/shared/util.c
> index 97c9497..deb74c4 100644
> --- a/src/shared/util.c
> +++ b/src/shared/util.c
> @@ -1356,6 +1356,78 @@ char *xescape(const char *s, const char *bad) {
>          return r;
>  }
>  
> +char *bus_path_escape(const char *s) {
> +        char *r, *t;
> +        const char *f;
> +
> +        assert(s);
> +
> +        /* Escapes all chars that D-Bus' object path cannot deal
> +         * with. Can be reversed with bus_path_unescape(). We special
> +         * case the empty string. */
> +
> +        if (*s == 0)
> +                return strdup("_");
> +
> +        r = new(char, strlen(s)*3 + 1);
> +        if (!r)
> +                return NULL;
> +
> +        for (f = s, t = r; *f; f++) {
> +
> +                /* Escape everything that is not a-zA-Z0-9. We also
> +                 * escape 0-9 if it's the first character */
> +
> +                if (!(*f >= 'A' && *f <= 'Z') &&
> +                    !(*f >= 'a' && *f <= 'z') &&
> +                    !(f > s && *f >= '0' && *f <= '9')) {
> +                        *(t++) = '_';
> +                        *(t++) = hexchar(*f >> 4);
> +                        *(t++) = hexchar(*f);
> +                } else
> +                        *(t++) = *f;
> +        }
> +
> +        *t = 0;
> +
> +        return r;
> +}
> +
> +char *bus_path_unescape(const char *f) {
> +        char *r, *t;
> +
> +        assert(f);
> +
> +        /* Special case for the empty string */
> +        if (streq(f, "_"))
> +                return strdup("");
> +
> +        r = new(char, strlen(f) + 1);
> +        if (!r)
> +                return NULL;
> +
> +        for (t = r; *f; f++) {
> +
> +                if (*f == '_') {
> +                        int a, b;
> +
> +                        if ((a = unhexchar(f[1])) < 0 ||
> +                            (b = unhexchar(f[2])) < 0) {
> +                                /* Invalid escape code, let's take it literal then */
> +                                *(t++) = '_';
> +                        } else {
> +                                *(t++) = (char) ((a << 4) | b);
> +                                f += 2;
> +                        }
> +                } else
> +                        *(t++) = *f;
> +        }
> +
> +        *t = 0;
> +
> +        return r;
> +}
> +
>  char *ascii_strlower(char *t) {
>          char *p;
>  
> diff --git a/src/shared/util.h b/src/shared/util.h
> index e46438c..f46a721 100644
> --- a/src/shared/util.h
> +++ b/src/shared/util.h
> @@ -237,6 +237,9 @@ char *cunescape_length_with_prefix(const char *s, size_t length, const char *pre
>  
>  char *xescape(const char *s, const char *bad);
>  
> +char *bus_path_escape(const char *s);
> +char *bus_path_unescape(const char *s);
> +
>  char *ascii_strlower(char *path);
>  
>  bool dirent_is_file(const struct dirent *de) _pure_;
> diff --git a/src/test/test-util.c b/src/test/test-util.c
> index afc3247..7fd0572 100644
> --- a/src/test/test-util.c
> +++ b/src/test/test-util.c
> @@ -351,6 +351,29 @@ static void test_memdup_multiply(void) {
>          free(dup);
>  }
>  
> +static void test_bus_path_escape_one(const char *a, const char *b) {
> +        _cleanup_free_ char *t = NULL, *x = NULL, *y = NULL;
> +
> +        assert_se(t = bus_path_escape(a));
> +        assert_se(streq(t, b));
> +
> +        assert_se(x = bus_path_unescape(t));
> +        assert_se(streq(a, x));
> +
> +        assert_se(y = bus_path_unescape(b));
> +        assert_se(streq(a, y));
> +}
> +
> +static void test_bus_path_escape(void) {
> +        test_bus_path_escape_one("foo123bar", "foo123bar");
> +        test_bus_path_escape_one("foo.bar", "foo_2ebar");
> +        test_bus_path_escape_one("foo_2ebar", "foo_5f2ebar");
> +        test_bus_path_escape_one("", "_");
> +        test_bus_path_escape_one("_", "_5f");
> +        test_bus_path_escape_one("1", "_31");
> +        test_bus_path_escape_one(":1", "_3a1");
> +}
> +
>  static void test_hostname_is_valid(void) {
>          assert(hostname_is_valid("foobar"));
>          assert(hostname_is_valid("foobar.com"));
> @@ -593,6 +616,7 @@ int main(int argc, char *argv[]) {
>          test_foreach_word_quoted();
>          test_default_term_for_tty();
>          test_memdup_multiply();
> +        test_bus_path_escape();
>          test_hostname_is_valid();
>          test_u64log2();
>          test_get_process_comm();


Lennart

-- 
Lennart Poettering, Red Hat


More information about the systemd-devel mailing list