Modify mount_propagation_flags_from_string to return a normal int code

This means that callers can distiguish an error from flags==0,
and don't have to special-case the empty string.

6 files changed, 44 insertions, 46 deletions

diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c
index 8970050408..840e94a553 100644
--- a/src/basic/mount-util.c
+++ b/src/basic/mount-util.c
@@ -693,13 +693,12 @@ int umount_verbose(const char *what) {
 const char *mount_propagation_flags_to_string(unsigned long flags) {
 
         switch (flags & (MS_SHARED|MS_SLAVE|MS_PRIVATE)) {
-
+        case 0:
+                return "";
         case MS_SHARED:
                 return "shared";
-
         case MS_SLAVE:
                 return "slave";
-
         case MS_PRIVATE:
                 return "private";
         }
@@ -707,17 +706,18 @@ const char *mount_propagation_flags_to_string(unsigned long flags) {
         return NULL;
 }
 
-unsigned long mount_propagation_flags_from_string(const char *name) {
 
-        if (isempty(name))
-                return 0;
-
-        if (streq(name, "shared"))
-                return MS_SHARED;
-        if (streq(name, "slave"))
-                return MS_SLAVE;
-        if (streq(name, "private"))
-                return MS_PRIVATE;
+int mount_propagation_flags_from_string(const char *name, unsigned long *ret) {
 
+        if (isempty(name))
+                *ret = 0;
+        else if (streq(name, "shared"))
+                *ret = MS_SHARED;
+        else if (streq(name, "slave"))
+                *ret = MS_SLAVE;
+        else if (streq(name, "private"))
+                *ret = MS_PRIVATE;
+        else
+                return -EINVAL;
         return 0;
 }
diff --git a/src/basic/mount-util.h b/src/basic/mount-util.h
index c8049198d4..1615c94e9a 100644
--- a/src/basic/mount-util.h
+++ b/src/basic/mount-util.h
@@ -63,4 +63,4 @@ int mount_verbose(
 int umount_verbose(const char *where);
 
 const char *mount_propagation_flags_to_string(unsigned long flags);
-unsigned long mount_propagation_flags_from_string(const char *name);
+int mount_propagation_flags_from_string(const char *name, unsigned long *ret);
diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c
index b3fc0ff5c3..9960b7a811 100644
--- a/src/core/dbus-execute.c
+++ b/src/core/dbus-execute.c
@@ -1671,7 +1671,7 @@ int bus_exec_context_set_transient_property(
                 if (mode != UNIT_CHECK) {
                         c->mount_flags = flags;
 
-                        unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, strempty(mount_propagation_flags_to_string(flags)));
+                        unit_write_drop_in_private_format(u, mode, name, "%s=%s", name, mount_propagation_flags_to_string(flags));
                 }
 
                 return 1;
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index f325d853c6..bc0cf73d39 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -1278,25 +1278,17 @@ int config_parse_exec_mount_flags(
                 void *userdata) {
 
 
-        unsigned long flags;
         ExecContext *c = data;
+        int r;
 
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
-        if (isempty(rvalue))
-                flags = 0;
-        else {
-                flags = mount_propagation_flags_from_string(rvalue);
-                if (flags == 0) {
-                        log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse mount flag %s, ignoring.", rvalue);
-                        return 0;
-                }
-        }
-
-        c->mount_flags = flags;
+        r = mount_propagation_flags_from_string(rvalue, &c->mount_flags);
+        if (r < 0)
+                log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse mount flag %s, ignoring.", rvalue);
 
         return 0;
 }
diff --git a/src/shared/bus-unit-util.c b/src/shared/bus-unit-util.c
index b030b3b9d1..829be2c6da 100644
--- a/src/shared/bus-unit-util.c
+++ b/src/shared/bus-unit-util.c
@@ -579,15 +579,9 @@ int bus_append_unit_property_assignment(sd_bus_message *m, const char *assignmen
         else if (streq(field, "MountFlags")) {
                 unsigned long f;
 
-                if (isempty(eq))
-                        f = 0;
-                else {
-                        f = mount_propagation_flags_from_string(eq);
-                        if (f == 0) {
-                                log_error("Failed to parse mount propagation type: %s", eq);
-                                return -EINVAL;
-                        }
-                }
+                r = mount_propagation_flags_from_string(eq, &f);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to parse mount propagation flags: %s", eq);
 
                 r = sd_bus_message_append(m, "v", "t", f);
         } else if (STR_IN_SET(field, "BindPaths", "BindReadOnlyPaths")) {
diff --git a/src/test/test-mount-util.c b/src/test/test-mount-util.c
index da7f35623b..7c5929d009 100644
--- a/src/test/test-mount-util.c
+++ b/src/test/test-mount-util.c
@@ -23,23 +23,35 @@
 #include "mount-util.h"
 #include "string-util.h"
 
-static void test_mount_propagation_flags(const char *name, unsigned long f) {
-        assert(mount_propagation_flags_from_string(name) == f);
+static void test_mount_propagation_flags(const char *name, int ret, unsigned long expected) {
+        long unsigned flags;
 
-        if (f != 0)
-                assert_se(streq_ptr(mount_propagation_flags_to_string(f), name));
+        assert(mount_propagation_flags_from_string(name, &flags) == ret);
+
+        if (ret >= 0) {
+                const char *c;
+
+                assert_se(flags == expected);
+
+                c = mount_propagation_flags_to_string(flags);
+                if (isempty(name))
+                        assert_se(isempty(c));
+                else
+                        assert_se(streq(c, name));
+        }
 }
 
 int main(int argc, char *argv[]) {
 
         log_set_max_level(LOG_DEBUG);
 
-        test_mount_propagation_flags("shared", MS_SHARED);
-        test_mount_propagation_flags("slave", MS_SLAVE);
-        test_mount_propagation_flags("private", MS_PRIVATE);
-        test_mount_propagation_flags(NULL, 0);
-        test_mount_propagation_flags("", 0);
-        test_mount_propagation_flags("xxxx", 0);
+        test_mount_propagation_flags("shared", 0, MS_SHARED);
+        test_mount_propagation_flags("slave", 0, MS_SLAVE);
+        test_mount_propagation_flags("private", 0, MS_PRIVATE);
+        test_mount_propagation_flags(NULL, 0, 0);
+        test_mount_propagation_flags("", 0, 0);
+        test_mount_propagation_flags("xxxx", -EINVAL, 0);
+        test_mount_propagation_flags(" ", -EINVAL, 0);
 
         return 0;
 }