condition: properly allow passing back errors from condition checks

7 files changed, 70 insertions, 53 deletions

diff --git a/src/core/condition.c b/src/core/condition.c
index 8e2e3118d7..32135ec5b2 100644
--- a/src/core/condition.c
+++ b/src/core/condition.c
@@ -40,7 +40,7 @@
 #include "selinux-util.h"
 #include "audit.h"
 
-static bool condition_test_security(Condition *c) {
+static int condition_test_security(Condition *c) {
         assert(c);
         assert(c->parameter);
         assert(c->type == CONDITION_SECURITY);
@@ -59,7 +59,7 @@ static bool condition_test_security(Condition *c) {
         return c->negate;
 }
 
-static bool condition_test_capability(Condition *c) {
+static int condition_test_capability(Condition *c) {
         _cleanup_fclose_ FILE *f = NULL;
         cap_value_t value;
         char line[LINE_MAX];
@@ -72,14 +72,14 @@ static bool condition_test_capability(Condition *c) {
         /* If it's an invalid capability, we don't have it */
 
         if (cap_from_name(c->parameter, &value) < 0)
-                return c->negate;
+                return -EINVAL;
 
         /* If it's a valid capability we default to assume
          * that we have it */
 
         f = fopen("/proc/self/status", "re");
         if (!f)
-                return !c->negate;
+                return -errno;
 
         while (fgets(line, sizeof(line), f)) {
                 truncate_nl(line);
@@ -132,12 +132,12 @@ static bool condition_test_first_boot(Condition *c) {
 
         r = parse_boolean(c->parameter);
         if (r < 0)
-                return c->negate;
+                return r;
 
         return ((access("/run/systemd/first-boot", F_OK) >= 0) == !!r) == !c->negate;
 }
 
-static bool condition_test(Condition *c) {
+static int condition_test(Condition *c) {
         assert(c);
 
         switch(c->type) {
@@ -242,25 +242,35 @@ bool condition_test_list(const char *unit, Condition *first) {
          * if any of the trigger conditions apply (unless there are
          * none) we return true */
         LIST_FOREACH(conditions, c, first) {
-                bool b;
-
-                b = condition_test(c);
-                if (unit)
+                int r;
+
+                r = condition_test(c);
+                if (r < 0)
+                        log_warning_unit(unit,
+                                         "Couldn't determine result for %s=%s%s%s for %s, assuming failed: %s",
+                                         condition_type_to_string(c->type),
+                                         c->trigger ? "|" : "",
+                                         c->negate ? "!" : "",
+                                         c->parameter,
+                                         unit,
+                                         strerror(-r));
+                else
                         log_debug_unit(unit,
                                        "%s=%s%s%s %s for %s.",
                                        condition_type_to_string(c->type),
                                        c->trigger ? "|" : "",
                                        c->negate ? "!" : "",
                                        c->parameter,
-                                       b ? "succeeded" : "failed",
+                                       r > 0 ? "succeeded" : "failed",
                                        unit);
-                c->state = b ? 1 : -1;
 
-                if (!c->trigger && !b)
+                c->state = r > 0 ? CONDITION_STATE_SUCCEEDED : CONDITION_STATE_FAILED;
+
+                if (!c->trigger && r <= 0)
                         return false;
 
                 if (c->trigger && triggered <= 0)
-                        triggered = b;
+                        triggered = r > 0;
         }
 
         return triggered != 0;
diff --git a/src/shared/architecture.c b/src/shared/architecture.c
index dc45f3589d..34c5a53fa9 100644
--- a/src/shared/architecture.c
+++ b/src/shared/architecture.c
@@ -23,7 +23,7 @@
 
 #include "architecture.h"
 
-Architecture uname_architecture(void) {
+int uname_architecture(void) {
 
         /* Return a sanitized enum identifying the architecture we are
          * running on. This is based on uname(), and the user may
@@ -41,7 +41,7 @@ Architecture uname_architecture(void) {
 
         static const struct {
                 const char *machine;
-                Architecture arch;
+                int arch;
         } arch_map[] = {
 #if defined(__x86_64__) || defined(__i386__)
                 { "x86_64",     ARCHITECTURE_X86_64   },
@@ -121,7 +121,7 @@ Architecture uname_architecture(void) {
 #endif
         };
 
-        static Architecture cached = _ARCHITECTURE_INVALID;
+        static int cached = _ARCHITECTURE_INVALID;
         struct utsname u;
         unsigned i;
 
@@ -168,4 +168,4 @@ static const char *const architecture_table[_ARCHITECTURE_MAX] = {
         [ARCHITECTURE_CRIS] = "cris",
 };
 
-DEFINE_STRING_TABLE_LOOKUP(architecture, Architecture);
+DEFINE_STRING_TABLE_LOOKUP(architecture, int);
diff --git a/src/shared/architecture.h b/src/shared/architecture.h
index 353d49bedf..f1fef23cf2 100644
--- a/src/shared/architecture.h
+++ b/src/shared/architecture.h
@@ -30,7 +30,7 @@
  * focus on general family, and distuignish word width and
  * endianness. */
 
-typedef enum Architecture {
+enum {
         ARCHITECTURE_X86 = 0,
         ARCHITECTURE_X86_64,
         ARCHITECTURE_PPC,
@@ -60,9 +60,9 @@ typedef enum Architecture {
         ARCHITECTURE_CRIS,
         _ARCHITECTURE_MAX,
         _ARCHITECTURE_INVALID = -1
-} Architecture;
+};
 
-Architecture uname_architecture(void);
+int uname_architecture(void);
 
 /*
  * LIB_ARCH_TUPLE should resolve to the local library path
@@ -188,5 +188,5 @@ Architecture uname_architecture(void);
 #error "Please register your architecture here!"
 #endif
 
-const char *architecture_to_string(Architecture a) _const_;
-Architecture architecture_from_string(const char *s) _pure_;
+const char *architecture_to_string(int a) _const_;
+int architecture_from_string(const char *s) _pure_;
diff --git a/src/shared/condition-util.c b/src/shared/condition-util.c
index 026b6a8712..33752c8f91 100644
--- a/src/shared/condition-util.c
+++ b/src/shared/condition-util.c
@@ -73,7 +73,7 @@ void condition_free_list(Condition *first) {
                 condition_free(c);
 }
 
-bool condition_test_kernel_command_line(Condition *c) {
+int condition_test_kernel_command_line(Condition *c) {
         _cleanup_free_ char *line = NULL;
         const char *p;
         bool equal;
@@ -85,8 +85,8 @@ bool condition_test_kernel_command_line(Condition *c) {
 
         r = proc_cmdline(&line);
         if (r < 0)
-                log_warning("Failed to read /proc/cmdline, ignoring: %s", strerror(-r));
-        if (r <= 0)
+                return r;
+        if (r == 0)
                 return c->negate;
 
         equal = !!strchr(c->parameter, '=');
@@ -97,7 +97,9 @@ bool condition_test_kernel_command_line(Condition *c) {
                 bool found;
 
                 r = unquote_first_word(&p, &word);
-                if (r <= 0)
+                if (r < 0)
+                        return r;
+                if (r == 0)
                         return c->negate;
 
                 if (equal)
@@ -116,7 +118,7 @@ bool condition_test_kernel_command_line(Condition *c) {
         return c->negate;
 }
 
-bool condition_test_virtualization(Condition *c) {
+int condition_test_virtualization(Condition *c) {
         int b, v;
         const char *id;
 
@@ -125,10 +127,8 @@ bool condition_test_virtualization(Condition *c) {
         assert(c->type == CONDITION_VIRTUALIZATION);
 
         v = detect_virtualization(&id);
-        if (v < 0) {
-                log_warning("Failed to detect virtualization, ignoring: %s", strerror(-v));
-                return c->negate;
-        }
+        if (v < 0)
+                return v;
 
         /* First, compare with yes/no */
         b = parse_boolean(c->parameter);
@@ -150,8 +150,8 @@ bool condition_test_virtualization(Condition *c) {
         return (v > 0 && streq(c->parameter, id)) == !c->negate;
 }
 
-bool condition_test_architecture(Condition *c) {
-        Architecture a, b;
+int condition_test_architecture(Condition *c) {
+        int a, b;
 
         assert(c);
         assert(c->parameter);
@@ -159,20 +159,19 @@ bool condition_test_architecture(Condition *c) {
 
         a = uname_architecture();
         if (a < 0)
-                return c->negate;
+                return a;
 
         if (streq(c->parameter, "native"))
                 b = native_architecture();
         else
                 b = architecture_from_string(c->parameter);
-
         if (b < 0)
-                return c->negate;
+                return b;
 
         return (a == b) == !c->negate;
 }
 
-bool condition_test_host(Condition *c) {
+int condition_test_host(Condition *c) {
         _cleanup_free_ char *h = NULL;
         sd_id128_t x, y;
         int r;
@@ -185,19 +184,19 @@ bool condition_test_host(Condition *c) {
 
                 r = sd_id128_get_machine(&y);
                 if (r < 0)
-                        return c->negate;
+                        return r;
 
                 return sd_id128_equal(x, y) == !c->negate;
         }
 
         h = gethostname_malloc();
         if (!h)
-                return c->negate;
+                return -ENOMEM;
 
         return (fnmatch(c->parameter, h, FNM_CASEFOLD) == 0) == !c->negate;
 }
 
-bool condition_test_ac_power(Condition *c) {
+int condition_test_ac_power(Condition *c) {
         int r;
 
         assert(c);
@@ -206,7 +205,7 @@ bool condition_test_ac_power(Condition *c) {
 
         r = parse_boolean(c->parameter);
         if (r < 0)
-                return !c->negate;
+                return r;
 
         return ((on_ac_power() != 0) == !!r) == !c->negate;
 }
@@ -225,7 +224,7 @@ void condition_dump(Condition *c, FILE *f, const char *prefix) {
                 c->trigger ? "|" : "",
                 c->negate ? "!" : "",
                 c->parameter,
-                c->state < 0 ? "failed" : c->state > 0 ? "succeeded" : "untested");
+                CONDITION_STATE_IS_FAILED(c->state) ? "failed" : CONDITION_STATE_IS_SUCCEEDED(c->state) ? "succeeded" : "untested");
 }
 
 void condition_dump_list(Condition *first, FILE *f, const char *prefix) {
diff --git a/src/shared/condition-util.h b/src/shared/condition-util.h
index 047fdbfd86..fbb8150083 100644
--- a/src/shared/condition-util.h
+++ b/src/shared/condition-util.h
@@ -51,6 +51,16 @@ typedef enum ConditionType {
         _CONDITION_TYPE_INVALID = -1
 } ConditionType;
 
+#define CONDITION_STATE_IS_SUCCEEDED(state) ((state) > 0)
+#define CONDITION_STATE_IS_UNKNOWN(state) ((state) == 0)
+#define CONDITION_STATE_IS_FAILED(state) ((state) < 0)
+
+enum {
+        CONDITION_STATE_SUCCEEDED = -1,
+        CONDITION_STATE_UNKNOWN = 0,
+        CONDITION_STATE_FAILED = 1
+};
+
 typedef struct Condition {
         ConditionType type;
 
@@ -58,7 +68,6 @@ typedef struct Condition {
         bool negate:1;
 
         char *parameter;
-
         int state;
 
         LIST_FIELDS(struct Condition, conditions);
@@ -68,11 +77,11 @@ Condition* condition_new(ConditionType type, const char *parameter, bool trigger
 void condition_free(Condition *c);
 void condition_free_list(Condition *c);
 
-bool condition_test_kernel_command_line(Condition *c);
-bool condition_test_virtualization(Condition *c);
-bool condition_test_architecture(Condition *c);
-bool condition_test_host(Condition *c);
-bool condition_test_ac_power(Condition *c);
+int condition_test_kernel_command_line(Condition *c);
+int condition_test_virtualization(Condition *c);
+int condition_test_architecture(Condition *c);
+int condition_test_host(Condition *c);
+int condition_test_ac_power(Condition *c);
 
 void condition_dump(Condition *c, FILE *f, const char *prefix);
 void condition_dump_list(Condition *c, FILE *f, const char *prefix);
diff --git a/src/test/test-architecture.c b/src/test/test-architecture.c
index 24217ad369..30bdec45e5 100644
--- a/src/test/test-architecture.c
+++ b/src/test/test-architecture.c
@@ -25,9 +25,8 @@
 #include "log.h"
 
 int main(int argc, char *argv[]) {
-        Architecture a;
-        int v;
         const char *id = NULL;
+        int a, v;
 
         v = detect_virtualization(&id);
         if (v == -EPERM || v == -EACCES)
diff --git a/src/test/test-condition-util.c b/src/test/test-condition-util.c
index 1c792446af..7a247fbdbd 100644
--- a/src/test/test-condition-util.c
+++ b/src/test/test-condition-util.c
@@ -73,8 +73,8 @@ static void test_condition_test_host(void) {
 
 static void test_condition_test_architecture(void) {
         Condition *condition;
-        Architecture a;
         const char *sa;
+        int a;
 
         a = uname_architecture();
         assert_se(a >= 0);
@@ -87,7 +87,7 @@ static void test_condition_test_architecture(void) {
         condition_free(condition);
 
         condition = condition_new(CONDITION_ARCHITECTURE, "garbage value", false, false);
-        assert_se(!condition_test_architecture(condition));
+        assert_se(condition_test_architecture(condition) < 0);
         condition_free(condition);
 
         condition = condition_new(CONDITION_ARCHITECTURE, sa, false, true);