From: Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Date: Thu, 26 Dec 2013 20:30:22 +0000 (-0500)
Subject: Use enums to make it obvious what boolean params mean
X-Git-Tag: v209~627
X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=f78e6385dc4cee0a1f399c4c89ebf823c108d447;ds=sidebyside

Use enums to make it obvious what boolean params mean

Suggested-by: Russ Allbery <rra@debian.org>
---

diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c
index 4d3e3cc9e..37c1e8cbf 100644
--- a/src/core/dbus-unit.c
+++ b/src/core/dbus-unit.c
@@ -813,7 +813,7 @@ static int bus_unit_set_transient_property(
                 if (r < 0)
                         return r;
 
-                if (!unit_name_is_valid(s, false) || !endswith(s, ".slice"))
+                if (!unit_name_is_valid(s, TEMPLATE_INVALID) || !endswith(s, ".slice"))
                         return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid slice name %s", s);
 
                 if (isempty(s)) {
@@ -865,7 +865,7 @@ static int bus_unit_set_transient_property(
                         return r;
 
                 while ((r = sd_bus_message_read(message, "s", &other)) > 0) {
-                        if (!unit_name_is_valid(other, false))
+                        if (!unit_name_is_valid(other, TEMPLATE_INVALID))
                                 return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid unit name %s", other);
 
                         if (mode != UNIT_CHECK) {
diff --git a/src/core/device.c b/src/core/device.c
index d3976c9d4..c7bc1e25e 100644
--- a/src/core/device.c
+++ b/src/core/device.c
@@ -268,7 +268,7 @@ static int device_update_unit(Manager *m, struct udev_device *dev, const char *p
                                 memcpy(e, w, l);
                                 e[l] = 0;
 
-                                n = unit_name_mangle(e, false);
+                                n = unit_name_mangle(e, MANGLE_NOGLOB);
                                 if (!n) {
                                         r = -ENOMEM;
                                         goto fail;
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 183c43d58..7a2d32ddb 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -2433,7 +2433,7 @@ static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {
                  * unit name. */
                 name = basename(*filename);
 
-                if (unit_name_is_valid(name, true)) {
+                if (unit_name_is_valid(name, TEMPLATE_VALID)) {
 
                         id = set_get(names, name);
                         if (!id) {
diff --git a/src/core/manager.c b/src/core/manager.c
index 6b33c1828..f69ae079d 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1115,7 +1115,7 @@ int manager_load_unit_prepare(
 
         t = unit_name_to_type(name);
 
-        if (t == _UNIT_TYPE_INVALID || !unit_name_is_valid(name, false))
+        if (t == _UNIT_TYPE_INVALID || !unit_name_is_valid(name, TEMPLATE_INVALID))
                 return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Unit name %s is not valid.", name);
 
         ret = manager_get_unit(m, name);
diff --git a/src/core/snapshot.c b/src/core/snapshot.c
index 21e89ac99..d914af20e 100644
--- a/src/core/snapshot.c
+++ b/src/core/snapshot.c
@@ -200,7 +200,7 @@ int snapshot_create(Manager *m, const char *name, bool cleanup, sd_bus_error *e,
         assert(_s);
 
         if (name) {
-                if (!unit_name_is_valid(name, false))
+                if (!unit_name_is_valid(name, TEMPLATE_INVALID))
                         return sd_bus_error_setf(e, SD_BUS_ERROR_INVALID_ARGS, "Unit name %s is not valid.", name);
 
                 if (unit_name_to_type(name) != UNIT_SNAPSHOT)
diff --git a/src/core/unit.c b/src/core/unit.c
index 160a346d8..e3b620603 100644
--- a/src/core/unit.c
+++ b/src/core/unit.c
@@ -119,7 +119,7 @@ int unit_add_name(Unit *u, const char *text) {
         if (!s)
                 return -ENOMEM;
 
-        if (!unit_name_is_valid(s, false)) {
+        if (!unit_name_is_valid(s, TEMPLATE_INVALID)) {
                 r = -EINVAL;
                 goto fail;
         }
diff --git a/src/dbus1-generator/dbus1-generator.c b/src/dbus1-generator/dbus1-generator.c
index d7ecd7f9a..5470bc94c 100644
--- a/src/dbus1-generator/dbus1-generator.c
+++ b/src/dbus1-generator/dbus1-generator.c
@@ -190,7 +190,7 @@ static int add_dbus(const char *path, const char *fname, const char *type) {
         }
 
         if (service) {
-                if (!unit_name_is_valid(service, false)) {
+                if (!unit_name_is_valid(service, TEMPLATE_INVALID)) {
                         log_warning("Unit name %s is not valid, ignoring.", service);
                         return 0;
                 }
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
index fdee9d4e6..42c16f67e 100644
--- a/src/journal/journalctl.c
+++ b/src/journal/journalctl.c
@@ -991,7 +991,7 @@ static int add_units(sd_journal *j) {
         assert(j);
 
         STRV_FOREACH(i, arg_system_units) {
-                u = unit_name_mangle(*i, false);
+                u = unit_name_mangle(*i, MANGLE_NOGLOB);
                 if (!u)
                         return log_oom();
                 r = add_matches_for_unit(j, u);
@@ -1003,7 +1003,7 @@ static int add_units(sd_journal *j) {
         }
 
         STRV_FOREACH(i, arg_user_units) {
-                u = unit_name_mangle(*i, false);
+                u = unit_name_mangle(*i, MANGLE_NOGLOB);
                 if (!u)
                         return log_oom();
 
diff --git a/src/run/run.c b/src/run/run.c
index ef2015fe3..1b14e40e4 100644
--- a/src/run/run.c
+++ b/src/run/run.c
@@ -208,7 +208,7 @@ static int message_start_transient_unit_new(sd_bus *bus, const char *name, sd_bu
         if (!isempty(arg_slice)) {
                 _cleanup_free_ char *slice;
 
-                slice = unit_name_mangle_with_suffix(arg_slice, false, ".slice");
+                slice = unit_name_mangle_with_suffix(arg_slice, MANGLE_NOGLOB, ".slice");
                 if (!slice)
                         return -ENOMEM;
 
@@ -255,7 +255,7 @@ static int start_transient_service(
         int r;
 
         if (arg_unit)
-                name = unit_name_mangle_with_suffix(arg_unit, false, ".service");
+                name = unit_name_mangle_with_suffix(arg_unit, MANGLE_NOGLOB, ".service");
         else
                 asprintf(&name, "run-%lu.service", (unsigned long) getpid());
         if (!name)
@@ -342,7 +342,7 @@ static int start_transient_scope(
         assert(bus);
 
         if (arg_unit)
-                name = unit_name_mangle_with_suffix(arg_unit, false, ".scope");
+                name = unit_name_mangle_with_suffix(arg_unit, MANGLE_NOGLOB, ".scope");
         else
                 asprintf(&name, "run-%lu.scope", (unsigned long) getpid());
         if (!name)
diff --git a/src/shared/cgroup-util.c b/src/shared/cgroup-util.c
index 27eee8eac..f2af8dcfd 100644
--- a/src/shared/cgroup-util.c
+++ b/src/shared/cgroup-util.c
@@ -1152,7 +1152,7 @@ int cg_path_decode_unit(const char *cgroup, char **unit){
         c = strndupa(cgroup, e - cgroup);
         c = cg_unescape(c);
 
-        if (!unit_name_is_valid(c, false))
+        if (!unit_name_is_valid(c, TEMPLATE_INVALID))
                 return -EINVAL;
 
         s = strdup(c);
@@ -1566,7 +1566,7 @@ int cg_slice_to_path(const char *unit, char **ret) {
         assert(unit);
         assert(ret);
 
-        if (!unit_name_is_valid(unit, false))
+        if (!unit_name_is_valid(unit, TEMPLATE_INVALID))
                 return -EINVAL;
 
         if (!endswith(unit, ".slice"))
@@ -1583,7 +1583,7 @@ int cg_slice_to_path(const char *unit, char **ret) {
 
                 strcpy(stpncpy(n, p, dash - p), ".slice");
 
-                if (!unit_name_is_valid(n, false))
+                if (!unit_name_is_valid(n, TEMPLATE_INVALID))
                         return -EINVAL;
 
                 escaped = cg_escape(n);
diff --git a/src/shared/install.c b/src/shared/install.c
index 233d64b6e..62151d99d 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -575,7 +575,7 @@ int unit_file_mask(
         STRV_FOREACH(i, files) {
                 _cleanup_free_ char *path = NULL;
 
-                if (!unit_name_is_valid(*i, true)) {
+                if (!unit_name_is_valid(*i, TEMPLATE_VALID)) {
                         if (r == 0)
                                 r = -EINVAL;
                         continue;
@@ -643,7 +643,7 @@ int unit_file_unmask(
         STRV_FOREACH(i, files) {
                 char *path;
 
-                if (!unit_name_is_valid(*i, true)) {
+                if (!unit_name_is_valid(*i, TEMPLATE_VALID)) {
                         if (r == 0)
                                 r = -EINVAL;
                         continue;
@@ -719,7 +719,7 @@ int unit_file_link(
                 fn = basename(*i);
 
                 if (!path_is_absolute(*i) ||
-                    !unit_name_is_valid(fn, true)) {
+                    !unit_name_is_valid(fn, TEMPLATE_VALID)) {
                         if (r == 0)
                                 r = -EINVAL;
                         continue;
@@ -861,7 +861,7 @@ static int install_info_add(
         if (!name)
                 name = basename(path);
 
-        if (!unit_name_is_valid(name, true))
+        if (!unit_name_is_valid(name, TEMPLATE_VALID))
                 return -EINVAL;
 
         if (hashmap_get(c->have_installed, name) ||
@@ -1235,7 +1235,7 @@ static int install_info_symlink_wants(
                 if (q < 0)
                         return q;
 
-                if (!unit_name_is_valid(dst, true)) {
+                if (!unit_name_is_valid(dst, TEMPLATE_VALID)) {
                         r = -EINVAL;
                         continue;
                 }
@@ -1272,7 +1272,7 @@ static int install_info_symlink_requires(
                 if (q < 0)
                         return q;
 
-                if (!unit_name_is_valid(dst, true)) {
+                if (!unit_name_is_valid(dst, TEMPLATE_VALID)) {
                         r = -EINVAL;
                         continue;
                 }
@@ -1675,7 +1675,7 @@ UnitFileState unit_file_get_state(
         if (root_dir && scope != UNIT_FILE_SYSTEM)
                 return -EINVAL;
 
-        if (!unit_name_is_valid(name, true))
+        if (!unit_name_is_valid(name, TEMPLATE_VALID))
                 return -EINVAL;
 
         r = lookup_paths_init_from_scope(&paths, scope);
@@ -1845,7 +1845,7 @@ int unit_file_preset(
 
         STRV_FOREACH(i, files) {
 
-                if (!unit_name_is_valid(*i, true))
+                if (!unit_name_is_valid(*i, TEMPLATE_VALID))
                         return -EINVAL;
 
                 r = unit_file_query_preset(scope, *i);
@@ -1949,7 +1949,7 @@ int unit_file_get_list(
                         if (ignore_file(de->d_name))
                                 continue;
 
-                        if (!unit_name_is_valid(de->d_name, true))
+                        if (!unit_name_is_valid(de->d_name, TEMPLATE_VALID))
                                 continue;
 
                         if (hashmap_get(h, de->d_name))
diff --git a/src/shared/unit-name.c b/src/shared/unit-name.c
index 832b92681..408323b8f 100644
--- a/src/shared/unit-name.c
+++ b/src/shared/unit-name.c
@@ -62,7 +62,7 @@ static const char* const unit_load_state_table[_UNIT_LOAD_STATE_MAX] = {
 
 DEFINE_STRING_TABLE_LOOKUP(unit_load_state, UnitLoadState);
 
-bool unit_name_is_valid(const char *n, bool template_ok) {
+bool unit_name_is_valid(const char *n, enum template_valid template_ok) {
         const char *e, *i, *at;
 
         /* Valid formats:
@@ -72,6 +72,7 @@ bool unit_name_is_valid(const char *n, bool template_ok) {
          */
 
         assert(n);
+        assert(IN_SET(template_ok, TEMPLATE_VALID, TEMPLATE_INVALID));
 
         if (strlen(n) >= UNIT_NAME_MAX)
                 return false;
@@ -96,7 +97,7 @@ bool unit_name_is_valid(const char *n, bool template_ok) {
                 if (at == n)
                         return false;
 
-                if (!template_ok && at+1 == e)
+                if (!template_ok == TEMPLATE_VALID && at+1 == e)
                         return false;
         }
 
@@ -186,7 +187,7 @@ char *unit_name_change_suffix(const char *n, const char *suffix) {
         size_t a, b;
 
         assert(n);
-        assert(unit_name_is_valid(n, true));
+        assert(unit_name_is_valid(n, TEMPLATE_VALID));
         assert(suffix);
         assert(suffix[0] == '.');
 
@@ -486,12 +487,13 @@ int unit_name_from_dbus_path(const char *path, char **name) {
  *  Try to turn a string that might not be a unit name into a
  *  sensible unit name.
  */
-char *unit_name_mangle(const char *name, bool allow_globs) {
+char *unit_name_mangle(const char *name, enum unit_name_mangle allow_globs) {
         char *r, *t;
         const char *f;
-        const char* valid_chars = allow_globs ? "@" VALID_CHARS "[]!-*?" : "@" VALID_CHARS;
+        const char* valid_chars = allow_globs == MANGLE_GLOB ? "@" VALID_CHARS "[]!-*?" : "@" VALID_CHARS;
 
         assert(name);
+        assert(IN_SET(allow_globs, MANGLE_GLOB, MANGLE_NOGLOB));
 
         if (is_device_path(name))
                 return unit_name_from_path(name, ".device");
@@ -528,7 +530,7 @@ char *unit_name_mangle(const char *name, bool allow_globs) {
  *  Similar to unit_name_mangle(), but is called when we know
  *  that this is about a specific unit type.
  */
-char *unit_name_mangle_with_suffix(const char *name, bool allow_globs, const char *suffix) {
+char *unit_name_mangle_with_suffix(const char *name, enum unit_name_mangle allow_globs, const char *suffix) {
         char *r, *t;
         const char *f;
 
diff --git a/src/shared/unit-name.h b/src/shared/unit-name.h
index 362ff0c00..d06d2b235 100644
--- a/src/shared/unit-name.h
+++ b/src/shared/unit-name.h
@@ -69,7 +69,12 @@ int unit_name_to_instance(const char *n, char **instance);
 char* unit_name_to_prefix(const char *n);
 char* unit_name_to_prefix_and_instance(const char *n);
 
-bool unit_name_is_valid(const char *n, bool template_ok) _pure_;
+enum template_valid {
+        TEMPLATE_INVALID,
+        TEMPLATE_VALID,
+};
+
+bool unit_name_is_valid(const char *n, enum template_valid template_ok) _pure_;
 bool unit_prefix_is_valid(const char *p) _pure_;
 bool unit_instance_is_valid(const char *i) _pure_;
 
@@ -98,7 +103,12 @@ char *unit_name_to_path(const char *name);
 char *unit_dbus_path_from_name(const char *name);
 int unit_name_from_dbus_path(const char *path, char **name);
 
-char *unit_name_mangle(const char *name, bool allow_globs);
-char *unit_name_mangle_with_suffix(const char *name, bool allow_globs, const char *suffix);
+enum unit_name_mangle {
+        MANGLE_NOGLOB,
+        MANGLE_GLOB,
+};
+
+char *unit_name_mangle(const char *name, enum unit_name_mangle allow_globs);
+char *unit_name_mangle_with_suffix(const char *name, enum unit_name_mangle allow_globs, const char *suffix);
 
 int build_subslice(const char *slice, const char*name, char **subslice);
diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c
index 03b9dd9c2..3a9bca90c 100644
--- a/src/systemctl/systemctl.c
+++ b/src/systemctl/systemctl.c
@@ -1375,7 +1375,7 @@ static int list_dependencies(sd_bus *bus, char **args) {
         assert(bus);
 
         if (args[1]) {
-                unit = unit_name_mangle(args[1], false);
+                unit = unit_name_mangle(args[1], MANGLE_NOGLOB);
                 if (!unit)
                         return log_oom();
                 u = unit;
@@ -1475,7 +1475,7 @@ static int set_default(sd_bus *bus, char **args) {
         unsigned n_changes = 0;
         int r;
 
-        unit = unit_name_mangle_with_suffix(args[1], false, ".target");
+        unit = unit_name_mangle_with_suffix(args[1], MANGLE_NOGLOB, ".target");
         if (!unit)
                 return log_oom();
 
@@ -1924,7 +1924,7 @@ static int check_one_unit(sd_bus *bus, const char *name, const char *good_states
 
         assert(name);
 
-        n = unit_name_mangle(name, false);
+        n = unit_name_mangle(name, MANGLE_NOGLOB);
         if (!n)
                 return log_oom();
 
@@ -1981,7 +1981,7 @@ static int check_triggering_units(
         char **i;
         int r;
 
-        n = unit_name_mangle(name, false);
+        n = unit_name_mangle(name, MANGLE_NOGLOB);
         if (!n)
                 return log_oom();
 
@@ -2111,9 +2111,9 @@ static int expand_names(sd_bus *bus, char **names, const char* suffix, char ***r
                 char *t;
 
                 if (suffix)
-                        t = unit_name_mangle_with_suffix(*name, true, suffix);
+                        t = unit_name_mangle_with_suffix(*name, MANGLE_GLOB, suffix);
                 else
-                        t = unit_name_mangle(*name, true);
+                        t = unit_name_mangle(*name, MANGLE_GLOB);
                 if (!t)
                         return log_oom();
 
@@ -4117,7 +4117,7 @@ static int set_property(sd_bus *bus, char **args) {
         if (r < 0)
                 return bus_log_create_error(r);
 
-        n = unit_name_mangle(args[1], false);
+        n = unit_name_mangle(args[1], MANGLE_NOGLOB);
         if (!n)
                 return log_oom();
 
@@ -4164,7 +4164,7 @@ static int snapshot(sd_bus *bus, char **args) {
         int r;
 
         if (strv_length(args) > 1)
-                n = unit_name_mangle_with_suffix(args[1], false, ".snapshot");
+                n = unit_name_mangle_with_suffix(args[1], MANGLE_NOGLOB, ".snapshot");
         else
                 n = strdup("");
         if (!n)
@@ -4619,7 +4619,7 @@ static int mangle_names(char **original_names, char ***mangled_names) {
                 if (is_path(*name))
                         *i = strdup(*name);
                 else
-                        *i = unit_name_mangle(*name, false);
+                        *i = unit_name_mangle(*name, MANGLE_NOGLOB);
 
                 if (!*i) {
                         strv_free(l);
diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c
index ad664bc54..c10ec608a 100644
--- a/src/test/test-unit-name.c
+++ b/src/test/test-unit-name.c
@@ -63,7 +63,7 @@ static void test_replacements(void) {
                 puts(t);                                           \
                 k = unit_name_to_path(t);                          \
                 puts(k);                                           \
-                assert(streq(k, expected ? expected : path));     \
+                assert(streq(k, expected ? expected : path));      \
         }
 
         expect("/waldo", ".mount", NULL);
@@ -74,12 +74,12 @@ static void test_replacements(void) {
 
         puts("-------------------------------------------------");
 #undef expect
-#define expect(pattern, path, suffix, expected)                         \
-        {                                                               \
-                _cleanup_free_ char *t =                                \
+#define expect(pattern, path, suffix, expected)                              \
+        {                                                                    \
+                _cleanup_free_ char *t =                                     \
                         unit_name_from_path_instance(pattern, path, suffix); \
-                puts(t);                                                \
-                assert(streq(t, expected));                             \
+                puts(t);                                                     \
+                assert(streq(t, expected));                                  \
         }
 
         expect("waldo", "/waldo", ".mount", "waldo@waldo.mount");
@@ -89,13 +89,13 @@ static void test_replacements(void) {
 
         puts("-------------------------------------------------");
 #undef expect
-#define expect(pattern)                                                 \
-        {                                                               \
-                _cleanup_free_ char *k, *t;                             \
-                assert_se(t = unit_name_mangle(pattern, false));        \
-                assert_se(k = unit_name_mangle(t, false));              \
-                puts(t);                                                \
-                assert_se(streq(t, k));                                 \
+#define expect(pattern)                                                     \
+        {                                                                   \
+                _cleanup_free_ char *k, *t;                                 \
+                assert_se(t = unit_name_mangle(pattern, MANGLE_NOGLOB));    \
+                assert_se(k = unit_name_mangle(t, MANGLE_NOGLOB));          \
+                puts(t);                                                    \
+                assert_se(streq(t, k));                                     \
         }
 
         expect("/home");