From 592fd144ae313855f48d0ca52a103013b41e5d59 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 6 Nov 2014 00:49:44 +0100 Subject: [PATCH] condition: properly allow passing back errors from condition checks --- src/core/condition.c | 38 +++++++++++++++++++++------------ src/shared/architecture.c | 8 +++---- src/shared/architecture.h | 10 ++++----- src/shared/condition-util.c | 39 +++++++++++++++++----------------- src/shared/condition-util.h | 21 ++++++++++++------ src/test/test-architecture.c | 3 +-- src/test/test-condition-util.c | 4 ++-- 7 files changed, 70 insertions(+), 53 deletions(-) diff --git a/src/core/condition.c b/src/core/condition.c index 8e2e3118d..32135ec5b 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 dc45f3589..34c5a53fa 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 353d49bed..f1fef23cf 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 026b6a871..33752c8f9 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 047fdbfd8..fbb815008 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 24217ad36..30bdec45e 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 1c792446a..7a247fbdb 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); -- 2.30.2