bus: rework sd_bus_error APIs

All calls that set a sd_bus_error structure will now return the same
error converted to a negative errno. This may be used as syntactic sugar
to return from a function and setting a bus_error structure in one go.

Also, translate all Linux Exyz (EIO, EINVAL, EUCLEAN, EPIPE, ...)
automatically into counterparts in the (new) "Posix.Error." namespace.

If we fail to allocate memory for the components of a sd_bus_error
automatically reset it to an OOM error which we always can write.

4 files changed, 417 insertions, 247 deletions

diff --git a/src/libsystemd-bus/bus-convenience.c b/src/libsystemd-bus/bus-convenience.c
index 6446874713..e765ddb21d 100644
--- a/src/libsystemd-bus/bus-convenience.c
+++ b/src/libsystemd-bus/bus-convenience.c
@@ -155,7 +155,6 @@ _public_ int sd_bus_reply_method_errorf(
 
         _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
         va_list ap;
-        int r;
 
         assert_return(call, -EINVAL);
         assert_return(call->sealed, -EPERM);
@@ -167,12 +166,9 @@ _public_ int sd_bus_reply_method_errorf(
                 return 0;
 
         va_start(ap, format);
-        r = bus_error_setfv(&error, name, format, ap);
+        bus_error_setfv(&error, name, format, ap);
         va_end(ap);
 
-        if (r < 0)
-                return r;
-
         return sd_bus_reply_method_error(call, &error);
 }
 
diff --git a/src/libsystemd-bus/bus-error.c b/src/libsystemd-bus/bus-error.c
index 7fef681413..968e80df7d 100644
--- a/src/libsystemd-bus/bus-error.c
+++ b/src/libsystemd-bus/bus-error.c
@@ -27,13 +27,188 @@
 #include <stdio.h>
 
 #include "util.h"
+#include "errno-list.h"
 
 #include "sd-bus.h"
 #include "bus-error.h"
 
+#define BUS_ERROR_OOM SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_NO_MEMORY, "Out of memory")
+#define BUS_ERROR_FAILED SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FAILED, "Operation failed")
+
+static int bus_error_name_to_errno(const char *name) {
+        const char *p;
+        int r;
+
+        if (!name)
+                return EINVAL;
+
+        p = startswith(name, "Posix.Error.");
+        if (p) {
+                r = errno_from_name(p);
+                if (r <= 0)
+                        return EIO;
+
+                return r;
+        }
+
+        /* Better replace this with a gperf table */
+
+        if (streq(name, SD_BUS_ERROR_NO_MEMORY))
+                return ENOMEM;
+
+        if (streq(name, SD_BUS_ERROR_SERVICE_UNKNOWN))
+                return EHOSTUNREACH;
+
+        if (streq(name, SD_BUS_ERROR_NAME_HAS_NO_OWNER))
+                return ENXIO;
+
+        if (streq(name, SD_BUS_ERROR_NO_REPLY) ||
+            streq(name, SD_BUS_ERROR_TIMEOUT) ||
+            streq(name, "org.freedesktop.DBus.Error.TimedOut"))
+                return ETIMEDOUT;
+
+        if (streq(name, SD_BUS_ERROR_IO_ERROR))
+                return EIO;
+
+        if (streq(name, SD_BUS_ERROR_BAD_ADDRESS))
+                return EADDRNOTAVAIL;
+
+        if (streq(name, SD_BUS_ERROR_NOT_SUPPORTED))
+                return ENOTSUP;
+
+        if (streq(name, SD_BUS_ERROR_LIMITS_EXCEEDED))
+                return ENOBUFS;
+
+        if (streq(name, SD_BUS_ERROR_ACCESS_DENIED) ||
+            streq(name, SD_BUS_ERROR_AUTH_FAILED))
+                return EACCES;
+
+        if (streq(name, SD_BUS_ERROR_NO_SERVER))
+                return EHOSTDOWN;
+
+        if (streq(name, SD_BUS_ERROR_NO_NETWORK))
+                return ENONET;
+
+        if (streq(name, SD_BUS_ERROR_ADDRESS_IN_USE))
+                return EADDRINUSE;
+
+        if (streq(name, SD_BUS_ERROR_DISCONNECTED))
+                return ECONNRESET;
+
+        if (streq(name, SD_BUS_ERROR_INVALID_ARGS) ||
+            streq(name, SD_BUS_ERROR_INVALID_SIGNATURE) ||
+            streq(name, "org.freedesktop.DBus.Error.MatchRuleInvalid") ||
+            streq(name, "org.freedesktop.DBus.Error.InvalidFileContent"))
+                return EINVAL;
+
+        if (streq(name, SD_BUS_ERROR_FILE_NOT_FOUND) ||
+            streq(name, "org.freedesktop.DBus.Error.MatchRuleNotFound"))
+                return ENOENT;
+
+        if (streq(name, SD_BUS_ERROR_FILE_EXISTS))
+                return EEXIST;
+
+        if (streq(name, SD_BUS_ERROR_UNKNOWN_METHOD) ||
+            streq(name, SD_BUS_ERROR_UNKNOWN_OBJECT) ||
+            streq(name, SD_BUS_ERROR_UNKNOWN_INTERFACE) ||
+            streq(name, SD_BUS_ERROR_UNKNOWN_PROPERTY))
+                return EBADR;
+
+        if (streq(name, SD_BUS_ERROR_PROPERTY_READ_ONLY))
+                return EROFS;
+
+        if (streq(name, SD_BUS_ERROR_UNIX_PROCESS_ID_UNKNOWN) ||
+            streq(name, "org.freedesktop.DBus.Error.SELinuxSecurityContextUnknown"))
+                return ESRCH;
+
+        if (streq(name, SD_BUS_ERROR_INCONSISTENT_MESSAGE))
+                return EBADMSG;
+
+        if (streq(name, "org.freedesktop.DBus.Error.ObjectPathInUse"))
+                return EBUSY;
+
+        return EIO;
+}
+
+static sd_bus_error errno_to_bus_error_const(int error) {
+
+        if (error < 0)
+                error = -error;
+
+        switch (error) {
+
+        case ENOMEM:
+                return BUS_ERROR_OOM;
+
+        case EPERM:
+        case EACCES:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_ACCESS_DENIED, "Access denied");
+
+        case EINVAL:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_INVALID_ARGS, "Invalid argument");
+
+        case ESRCH:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_UNIX_PROCESS_ID_UNKNOWN, "No such process");
+
+        case ENOENT:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FILE_NOT_FOUND, "File not found");
+
+        case EEXIST:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FILE_EXISTS, "File exists");
+
+        case ETIMEDOUT:
+        case ETIME:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_TIMEOUT, "Timed out");
+
+        case EIO:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_IO_ERROR, "Input/output error");
+
+        case ENETRESET:
+        case ECONNABORTED:
+        case ECONNRESET:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_DISCONNECTED, "Disconnected");
+
+        case ENOTSUP:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_NOT_SUPPORTED, "Not supported");
+
+        case EADDRNOTAVAIL:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_BAD_ADDRESS, "Address not available");
+
+        case ENOBUFS:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_LIMITS_EXCEEDED, "Limits exceeded");
+
+        case EADDRINUSE:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_ADDRESS_IN_USE, "Address in use");
+
+        case EBADMSG:
+                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_INCONSISTENT_MESSAGE, "Inconsistent message");
+        }
+
+        return SD_BUS_ERROR_NULL;
+}
+
+static int errno_to_bus_error_name_new(int error, char **ret) {
+        const char *name;
+        char *n;
+
+        if (error < 0)
+                error = -error;
+
+        name = errno_to_name(error);
+        if (!name)
+                return 0;
+
+        n = strappend("Posix.Error.", name);
+        if (!n)
+                return -ENOMEM;
+
+        *ret = n;
+        return 1;
+}
+
 bool bus_error_is_dirty(sd_bus_error *e) {
         if (!e)
-                return 0;
+                return false;
 
         return e->name || e->message || e->need_free;
 }
@@ -52,58 +227,51 @@ _public_ void sd_bus_error_free(sd_bus_error *e) {
 }
 
 _public_ int sd_bus_error_set(sd_bus_error *e, const char *name, const char *message) {
-        char *n, *m = NULL;
 
-        if (!e)
+        if (!name)
                 return 0;
+        if (!e)
+                goto finish;
 
         assert_return(!bus_error_is_dirty(e), -EINVAL);
-        assert_return(name, -EINVAL);
 
-        n = strdup(name);
-        if (!n)
+        e->name = strdup(name);
+        if (!e->name) {
+                *e = BUS_ERROR_OOM;
                 return -ENOMEM;
-
-        if (message) {
-                m = strdup(message);
-                if (!m)
-                        return -ENOMEM;
         }
 
-        e->name = n;
-        e->message = m;
+        if (message)
+                e->message = strdup(message);
+
         e->need_free = true;
 
-        return sd_bus_error_get_errno(e);
+finish:
+        return -bus_error_name_to_errno(name);
 }
 
 int bus_error_setfv(sd_bus_error *e, const char *name, const char *format, va_list ap) {
-        char *n, *m = NULL;
-        int r;
 
-        if (!e)
+        if (!name)
                 return 0;
+        if (!e)
+                goto finish;
 
         assert_return(!bus_error_is_dirty(e), -EINVAL);
-        assert_return(name, -EINVAL);
 
-        n = strdup(name);
-        if (!n)
+        e->name = strdup(name);
+        if (!e->name) {
+                *e = BUS_ERROR_OOM;
                 return -ENOMEM;
-
-        if (format) {
-                r = vasprintf(&m, format, ap);
-                if (r < 0) {
-                        free(n);
-                        return -ENOMEM;
-                }
         }
 
-        e->name = n;
-        e->message = m;
+        if (format)
+                vasprintf((char**) &e->message, format, ap);
+
         e->need_free = true;
 
-        return sd_bus_error_get_errno(e);
+finish:
+        return -bus_error_name_to_errno(name);
 }
 
 _public_ int sd_bus_error_setf(sd_bus_error *e, const char *name, const char *format, ...) {
@@ -123,42 +291,45 @@ _public_ int sd_bus_error_setf(sd_bus_error *e, const char *name, const char *fo
 }
 
 _public_ int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e) {
-        char *x, *y = NULL;
 
-        if (!dest)
-                return 0;
         if (!sd_bus_error_is_set(e))
                 return 0;
+        if (!dest)
+                goto finish;
 
         assert_return(!bus_error_is_dirty(dest), -EINVAL);
 
-        x = strdup(e->name);
-        if (!x)
-                return -ENOMEM;
-
-        if (e->message) {
-                y = strdup(e->message);
-                if (!y) {
-                        free(x);
+        if (!e->need_free)
+                *dest = *e;
+        else {
+                dest->name = strdup(e->name);
+                if (!dest->name) {
+                        *dest = BUS_ERROR_OOM;
                         return -ENOMEM;
                 }
+
+                if (e->message)
+                        dest->message = strdup(e->message);
+
+                dest->need_free = true;
         }
 
-        dest->name = x;
-        dest->message = y;
-        dest->need_free = true;
-        return sd_bus_error_get_errno(e);
+finish:
+        return -bus_error_name_to_errno(e->name);
 }
 
 _public_ int sd_bus_error_set_const(sd_bus_error *e, const char *name, const char *message) {
-        if (!e)
+        if (!name)
                 return 0;
+        if (!e)
+                goto finish;
 
         assert_return(!bus_error_is_dirty(e), -EINVAL);
-        assert_return(name, -EINVAL);
 
         *e = SD_BUS_ERROR_MAKE_CONST(name, message);
-        return sd_bus_error_get_errno(e);
+
+finish:
+        return -bus_error_name_to_errno(name);
 }
 
 _public_ int sd_bus_error_is_set(const sd_bus_error *e) {
@@ -176,111 +347,24 @@ _public_ int sd_bus_error_has_name(const sd_bus_error *e, const char *name) {
 }
 
 _public_ int sd_bus_error_get_errno(const sd_bus_error* e) {
-
-        /* Better replce this with a gperf table */
-
         if (!e)
-                return EIO;
-
-        if (!e->name)
-                return EIO;
-
-        if (streq(e->name, SD_BUS_ERROR_NO_MEMORY))
-                return ENOMEM;
-
-        if (streq(e->name, SD_BUS_ERROR_SERVICE_UNKNOWN))
-                return EHOSTUNREACH;
-
-        if (streq(e->name, SD_BUS_ERROR_NAME_HAS_NO_OWNER))
-                return ENXIO;
-
-        if (streq(e->name, SD_BUS_ERROR_NO_REPLY) ||
-            streq(e->name, SD_BUS_ERROR_TIMEOUT) ||
-            streq(e->name, "org.freedesktop.DBus.Error.TimedOut"))
-                return ETIMEDOUT;
-
-        if (streq(e->name, SD_BUS_ERROR_IO_ERROR))
-                return EIO;
-
-        if (streq(e->name, SD_BUS_ERROR_BAD_ADDRESS))
-                return EADDRNOTAVAIL;
-
-        if (streq(e->name, SD_BUS_ERROR_NOT_SUPPORTED))
-                return ENOTSUP;
-
-        if (streq(e->name, SD_BUS_ERROR_LIMITS_EXCEEDED))
-                return ENOBUFS;
-
-        if (streq(e->name, SD_BUS_ERROR_ACCESS_DENIED) ||
-            streq(e->name, SD_BUS_ERROR_AUTH_FAILED))
-                return EACCES;
-
-        if (streq(e->name, SD_BUS_ERROR_NO_SERVER))
-                return EHOSTDOWN;
-
-        if (streq(e->name, SD_BUS_ERROR_NO_NETWORK))
-                return ENONET;
-
-        if (streq(e->name, SD_BUS_ERROR_ADDRESS_IN_USE))
-                return EADDRINUSE;
-
-        if (streq(e->name, SD_BUS_ERROR_DISCONNECTED))
-                return ECONNRESET;
-
-        if (streq(e->name, SD_BUS_ERROR_INVALID_ARGS) ||
-            streq(e->name, SD_BUS_ERROR_INVALID_SIGNATURE) ||
-            streq(e->name, "org.freedesktop.DBus.Error.MatchRuleInvalid") ||
-            streq(e->name, "org.freedesktop.DBus.Error.InvalidFileContent"))
-                return EINVAL;
-
-        if (streq(e->name, SD_BUS_ERROR_FILE_NOT_FOUND) ||
-            streq(e->name, "org.freedesktop.DBus.Error.MatchRuleNotFound"))
-                return ENOENT;
-
-        if (streq(e->name, SD_BUS_ERROR_FILE_EXISTS))
-                return EEXIST;
-
-        if (streq(e->name, SD_BUS_ERROR_UNKNOWN_METHOD) ||
-            streq(e->name, SD_BUS_ERROR_UNKNOWN_OBJECT) ||
-            streq(e->name, SD_BUS_ERROR_UNKNOWN_INTERFACE) ||
-            streq(e->name, SD_BUS_ERROR_UNKNOWN_PROPERTY))
-                return EBADR;
-
-        if (streq(e->name, SD_BUS_ERROR_PROPERTY_READ_ONLY))
-                return EROFS;
-
-        if (streq(e->name, SD_BUS_ERROR_UNIX_PROCESS_ID_UNKNOWN) ||
-            streq(e->name, "org.freedesktop.DBus.Error.SELinuxSecurityContextUnknown"))
-                return ESRCH;
-
-        if (streq(e->name, SD_BUS_ERROR_INCONSISTENT_MESSAGE))
-                return EBADMSG;
-
-        if (streq(e->name, "org.freedesktop.DBus.Error.ObjectPathInUse"))
-                return EBUSY;
+                return 0;
 
-        return EIO;
+        return bus_error_name_to_errno(e->name);
 }
 
-static int bus_error_set_strerror_or_const(sd_bus_error *e, const char *name, int error, const char *fallback) {
+static void bus_error_strerror(sd_bus_error *e, int error) {
         size_t k = 64;
-        char *n = NULL, *m = NULL;
-
-        if (error < 0)
-                error = -error;
+        char *m;
 
-        if (!e)
-                return -error;
-
-        assert_return(!bus_error_is_dirty(e), -EINVAL);
-        assert_return(name, -EINVAL);
+        assert(e);
 
         for (;;) {
                 char *x;
 
                 m = new(char, k);
                 if (!m)
-                        goto use_fallback;
+                        return;
 
                 errno = 0;
                 x = strerror_r(error, m, k);
@@ -292,104 +376,88 @@ static int bus_error_set_strerror_or_const(sd_bus_error *e, const char *name, in
 
                 if (!x || errno) {
                         free(m);
-                        goto use_fallback;
+                        return;
                 }
 
-
-                if (x != m) {
+                if (x == m) {
+                        if (e->need_free) {
+                                /* Error is already dynamic, let's just update the message */
+                                free((char*) e->message);
+                                e->message = x;
+
+                        } else {
+                                char *t;
+                                /* Error was const so far, let's make it dynamic, if we can */
+
+                                t = strdup(e->name);
+                                if (!t) {
+                                        free(m);
+                                        return;
+                                }
+
+                                e->need_free = true;
+                                e->name = t;
+                                e->message = x;
+                        }
+                } else {
                         free(m);
-                        sd_bus_error_set_const(e, name, x);
-                        return -error;
-                }
-
-                break;
-        }
-
 
-        n = strdup(name);
-        if (!n) {
-                free(m);
-                goto use_fallback;
-        }
+                        if (e->need_free) {
+                                char *t;
 
-        e->name = n;
-        e->message = m;
-        e->need_free = true;
+                                /* Error is dynamic, let's hence make the message also dynamic */
+                                t = strdup(x);
+                                if (!t)
+                                        return;
 
-        return -error;
+                                free((char*) e->message);
+                                e->message = t;
+                        } else {
+                                /* Error is const, hence we can just override */
+                                e->message = x;
+                        }
+                }
 
-use_fallback:
-        sd_bus_error_set_const(e, name, fallback);
-        return -error;
+                return;
+        }
 }
 
-static sd_bus_error map_from_errno(int error) {
+_public_ int sd_bus_error_set_errno(sd_bus_error *e, int error) {
 
         if (error < 0)
                 error = -error;
 
-        switch (error) {
-
-        case ENOMEM:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_NO_NETWORK, "Out of memory");
-
-        case EPERM:
-        case EACCES:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_ACCESS_DENIED, "Access denied");
-
-        case EINVAL:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_INVALID_ARGS, "Invalid argument");
-
-        case ESRCH:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_UNIX_PROCESS_ID_UNKNOWN, "No such process");
-
-        case ENOENT:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FILE_NOT_FOUND, "File not found");
-
-        case EEXIST:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FILE_EXISTS, "File exists");
-
-        case ETIMEDOUT:
-        case ETIME:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_TIMEOUT, "Timed out");
-
-        case EIO:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_IO_ERROR, "Input/output error");
-
-        case ENETRESET:
-        case ECONNABORTED:
-        case ECONNRESET:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_DISCONNECTED, "Disconnected");
+        if (!e)
+                return -error;
+        if (error == 0)
+                return -error;
 
-        case ENOTSUP:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_NOT_SUPPORTED, "Not supported");
+        assert_return(!bus_error_is_dirty(e), -EINVAL);
 
-        case EADDRNOTAVAIL:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_BAD_ADDRESS, "Address not available");
+        /* First, try a const translation */
+        *e = errno_to_bus_error_const(error);
 
-        case ENOBUFS:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_LIMITS_EXCEEDED, "Limits exceeded");
+        if (!sd_bus_error_is_set(e)) {
+                int k;
 
-        case EADDRINUSE:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_ADDRESS_IN_USE, "Address in use");
+                /* If that didn't work, try a dynamic one. */
 
-        case EBADMSG:
-                return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_INCONSISTENT_MESSAGE, "Inconsistent message");
+                k = errno_to_bus_error_name_new(error, (char**) &e->name);
+                if (k > 0)
+                        e->need_free = true;
+                else if (k < 0) {
+                        *e = BUS_ERROR_OOM;
+                        return -error;
+                } else
+                        *e = BUS_ERROR_FAILED;
         }
 
-        return SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FAILED, "Operation failed");
-}
-
-_public_ int sd_bus_error_set_errno(sd_bus_error *e, int error) {
-        sd_bus_error x;
-
-        x = map_from_errno(error);
-
-        return bus_error_set_strerror_or_const(e, x.name, error, x.message);
+        /* Now, fill in the message from strerror() if we can */
+        bus_error_strerror(e, error);
+        return -error;
 }
 
 int bus_error_set_errnofv(sd_bus_error *e, int error, const char *format, va_list ap) {
-        sd_bus_error x;
         int r;
 
         if (error < 0)
@@ -397,32 +465,60 @@ int bus_error_set_errnofv(sd_bus_error *e, int error, const char *format, va_lis
 
         if (!e)
                 return -error;
+        if (error == 0)
+                return 0;
 
         assert_return(!bus_error_is_dirty(e), -EINVAL);
 
-        x = map_from_errno(error);
+        /* First, try a const translation */
+        *e = errno_to_bus_error_const(error);
+
+        if (!sd_bus_error_is_set(e)) {
+                int k;
+
+                /* If that didn't work, try a dynamic one */
+
+                k = errno_to_bus_error_name_new(error, (char**) &e->name);
+                if (k > 0)
+                        e->need_free = true;
+                else if (k < 0) {
+                        *e = BUS_ERROR_OOM;
+                        return -ENOMEM;
+                } else
+                        *e = BUS_ERROR_FAILED;
+        }
 
         if (format) {
-                char *n, *m;
+                char *m;
 
-                r = vasprintf(&m, format, ap);
-                if (r < 0)
-                        goto fallback;
+                /* First, let's try to fill in the supplied message */
 
-                n = strdup(x.name);
-                if (!n) {
-                        free(m);
-                        goto fallback;
+                r = vasprintf(&m, format, ap);
+                if (r >= 0) {
+
+                        if (!e->need_free) {
+                                char *t;
+
+                                t = strdup(e->name);
+                                if (t) {
+                                        e->need_free = true;
+                                        e->name = t;
+                                        e->message = m;
+                                        return -error;
+                                }
+
+                                free(m);
+                        } else {
+                                free((char*) e->message);
+                                e->message = m;
+                                return -error;
+                        }
                 }
-
-                e->name = n;
-                e->message = m;
-                e->need_free = true;
-                return -error;
         }
 
-fallback:
-        return bus_error_set_strerror_or_const(e, x.name, error, x.message);
+        /* If that didn't work, use strerror() for the message */
+        bus_error_strerror(e, error);
+        return -error;
 }
 
 _public_ int sd_bus_error_set_errnof(sd_bus_error *e, int error, const char *format, ...) {
@@ -433,6 +529,8 @@ _public_ int sd_bus_error_set_errnof(sd_bus_error *e, int error, const char *for
 
         if (!e)
                 return -error;
+        if (error == 0)
+                return 0;
 
         assert_return(!bus_error_is_dirty(e), -EINVAL);
 
diff --git a/src/libsystemd-bus/bus-message.c b/src/libsystemd-bus/bus-message.c
index 538a28af9e..132426f6fd 100644
--- a/src/libsystemd-bus/bus-message.c
+++ b/src/libsystemd-bus/bus-message.c
@@ -624,18 +624,14 @@ _public_ int sd_bus_message_new_method_errorf(
 
         _cleanup_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
         va_list ap;
-        int r;
 
         assert_return(name, -EINVAL);
         assert_return(m, -EINVAL);
 
         va_start(ap, format);
-        r = bus_error_setfv(&error, name, format, ap);
+        bus_error_setfv(&error, name, format, ap);
         va_end(ap);
 
-        if (r < 0)
-                return r;
-
         return sd_bus_message_new_method_error(call, &error, m);
 }
 
@@ -664,15 +660,11 @@ _public_ int sd_bus_message_new_method_errnof(
 
         _cleanup_free_ sd_bus_error berror = SD_BUS_ERROR_NULL;
         va_list ap;
-        int r;
 
         va_start(ap, format);
-        r = bus_error_set_errnofv(&berror, error, format, ap);
+        bus_error_set_errnofv(&berror, error, format, ap);
         va_end(ap);
 
-        if (r < 0)
-                return r;
-
         return sd_bus_message_new_method_error(call, &berror, m);
 }
 
diff --git a/src/libsystemd-bus/test-bus-error.c b/src/libsystemd-bus/test-bus-error.c
new file mode 100644
index 0000000000..9c0f4e0158
--- /dev/null
+++ b/src/libsystemd-bus/test-bus-error.c
@@ -0,0 +1,84 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2013 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "sd-bus.h"
+#include "bus-error.h"
+#include "bus-util.h"
+
+int main(int argc, char *argv[]) {
+
+        _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL, second = SD_BUS_ERROR_NULL;
+
+        assert_se(!sd_bus_error_is_set(&error));
+        assert_se(sd_bus_error_set(&error, SD_BUS_ERROR_NOT_SUPPORTED, "xxx") == -ENOTSUP);
+        assert_se(streq(error.name, SD_BUS_ERROR_NOT_SUPPORTED));
+        assert_se(streq(error.message, "xxx"));
+        assert_se(sd_bus_error_has_name(&error, SD_BUS_ERROR_NOT_SUPPORTED));
+        assert_se(sd_bus_error_get_errno(&error) == ENOTSUP);
+        assert_se(sd_bus_error_is_set(&error));
+        sd_bus_error_free(&error);
+
+        assert_se(!sd_bus_error_is_set(&error));
+        assert_se(sd_bus_error_setf(&error, SD_BUS_ERROR_FILE_NOT_FOUND, "yyy %i", -1) == -ENOENT);
+        assert_se(streq(error.name, SD_BUS_ERROR_FILE_NOT_FOUND));
+        assert_se(streq(error.message, "yyy -1"));
+        assert_se(sd_bus_error_has_name(&error, SD_BUS_ERROR_FILE_NOT_FOUND));
+        assert_se(sd_bus_error_get_errno(&error) == ENOENT);
+        assert_se(sd_bus_error_is_set(&error));
+
+        assert_se(!sd_bus_error_is_set(&second));
+        assert_se(sd_bus_error_copy(&second, &error) == -ENOENT);
+        assert_se(streq(error.name, second.name));
+        assert_se(streq(error.message, second.message));
+        assert_se(sd_bus_error_get_errno(&second) == ENOENT);
+        assert_se(sd_bus_error_has_name(&second, SD_BUS_ERROR_FILE_NOT_FOUND));
+        assert_se(sd_bus_error_is_set(&second));
+
+        sd_bus_error_free(&error);
+
+        assert_se(!sd_bus_error_is_set(&error));
+        assert_se(sd_bus_error_set_const(&error, "Posix.Error.EUCLEAN", "Hallo") == -EUCLEAN);
+        assert_se(streq(error.name, "Posix.Error.EUCLEAN"));
+        assert_se(streq(error.message, "Hallo"));
+        assert_se(sd_bus_error_has_name(&error, "Posix.Error.EUCLEAN"));
+        assert_se(sd_bus_error_get_errno(&error) == EUCLEAN);
+        assert_se(sd_bus_error_is_set(&error));
+        sd_bus_error_free(&error);
+
+        assert_se(!sd_bus_error_is_set(&error));
+        assert_se(sd_bus_error_set_errno(&error, EBUSY) == -EBUSY);
+        assert_se(streq(error.name, "Posix.Error.EBUSY"));
+        assert_se(streq(error.message, strerror(EBUSY)));
+        assert_se(sd_bus_error_has_name(&error, "Posix.Error.EBUSY"));
+        assert_se(sd_bus_error_get_errno(&error) == EBUSY);
+        assert_se(sd_bus_error_is_set(&error));
+        sd_bus_error_free(&error);
+
+        assert_se(!sd_bus_error_is_set(&error));
+        assert_se(sd_bus_error_set_errnof(&error, EIO, "Waldi %c", 'X') == -EIO);
+        assert_se(streq(error.name, SD_BUS_ERROR_IO_ERROR));
+        assert_se(streq(error.message, "Waldi X"));
+        assert_se(sd_bus_error_has_name(&error, SD_BUS_ERROR_IO_ERROR));
+        assert_se(sd_bus_error_get_errno(&error) == EIO);
+        assert_se(sd_bus_error_is_set(&error));
+
+        return 0;
+}