condition: propagate errors from condition_test_list()
Make condition_test_list() return negative errno on condition evaluation errors instead of coercing them to false. This allows callers to distinguish between a condition that evaluated to false and a condition that failed to evaluate. Callers that only need a boolean result explicitly check for a positive return value, preserving their current behavior. Add comprehensive unit tests covering error propagation with trigger and non-trigger conditions. Tests use uname_architecture() to get the runtime architecture instead of native_architecture(), ensuring they work correctly in environments where binaries run under architecture emulation.
This commit is contained in:
@@ -125,11 +125,11 @@ static int verify_conditions(char **lines, RuntimeScope scope, const char *unit,
|
||||
}
|
||||
|
||||
condition_test_logger_t logger = arg_quiet ? NULL : log_helper;
|
||||
r = condition_test_list(u->asserts, environ, assert_type_to_string, logger, u);
|
||||
r = condition_test_list(u->asserts, environ, assert_type_to_string, logger, u) > 0;
|
||||
if (u->asserts)
|
||||
log_full(arg_quiet ? LOG_DEBUG : LOG_NOTICE, "Asserts %s.", r > 0 ? "succeeded" : "failed");
|
||||
|
||||
q = condition_test_list(u->conditions, environ, condition_type_to_string, logger, u);
|
||||
q = condition_test_list(u->conditions, environ, condition_type_to_string, logger, u) > 0;
|
||||
if (u->conditions)
|
||||
log_full(arg_quiet ? LOG_DEBUG : LOG_NOTICE, "Conditions %s.", q > 0 ? "succeeded" : "failed");
|
||||
|
||||
|
||||
+2
-2
@@ -1819,7 +1819,7 @@ static bool unit_test_condition(Unit *u) {
|
||||
env,
|
||||
condition_type_to_string,
|
||||
log_unit_internal,
|
||||
u);
|
||||
u) > 0;
|
||||
|
||||
unit_add_to_dbus_queue(u);
|
||||
return u->condition_result;
|
||||
@@ -1843,7 +1843,7 @@ static bool unit_test_assert(Unit *u) {
|
||||
env,
|
||||
assert_type_to_string,
|
||||
log_unit_internal,
|
||||
u);
|
||||
u) > 0;
|
||||
|
||||
unit_add_to_dbus_queue(u);
|
||||
return u->assert_result;
|
||||
|
||||
+25
-9
@@ -28,6 +28,7 @@
|
||||
#include "efivars.h"
|
||||
#include "env-file.h"
|
||||
#include "env-util.h"
|
||||
#include "errno-util.h"
|
||||
#include "extract-word.h"
|
||||
#include "fd-util.h"
|
||||
#include "fileio.h"
|
||||
@@ -1420,7 +1421,7 @@ int condition_test(Condition *c, char **env) {
|
||||
return condition_test_impl(c, env, table);
|
||||
}
|
||||
|
||||
static bool condition_test_list_impl(
|
||||
static int condition_test_list_impl(
|
||||
Condition *first,
|
||||
char **env,
|
||||
condition_to_string_t to_string,
|
||||
@@ -1428,7 +1429,8 @@ static bool condition_test_list_impl(
|
||||
condition_test_func_t tester,
|
||||
void *userdata) {
|
||||
|
||||
int triggered = -1;
|
||||
int non_trigger_error = 0, trigger_error = 0;
|
||||
bool has_trigger = false, trigger_success = false;
|
||||
|
||||
/* If the condition list is empty, then it is true */
|
||||
if (!first)
|
||||
@@ -1440,12 +1442,14 @@ static bool condition_test_list_impl(
|
||||
LIST_FOREACH(conditions, c, first) {
|
||||
int r;
|
||||
|
||||
has_trigger = has_trigger || c->trigger;
|
||||
|
||||
r = tester(c, env);
|
||||
|
||||
if (logger) {
|
||||
if (r < 0)
|
||||
logger(userdata, LOG_WARNING, r, PROJECT_FILE, __LINE__, __func__,
|
||||
"Couldn't determine result for %s=%s%s%s, assuming failed: %m",
|
||||
"Couldn't determine result for %s=%s%s%s: %m",
|
||||
to_string(c->type),
|
||||
c->trigger ? "|" : "",
|
||||
c->negate ? "!" : "",
|
||||
@@ -1460,14 +1464,26 @@ static bool condition_test_list_impl(
|
||||
condition_result_to_string(c->result));
|
||||
}
|
||||
|
||||
if (!c->trigger && r <= 0)
|
||||
if (r < 0) {
|
||||
if (c->trigger)
|
||||
RET_GATHER(trigger_error, r);
|
||||
else
|
||||
RET_GATHER(non_trigger_error, r);
|
||||
|
||||
continue;
|
||||
}
|
||||
|
||||
if (!c->trigger && r == 0)
|
||||
return false;
|
||||
|
||||
if (c->trigger && triggered <= 0)
|
||||
triggered = r > 0;
|
||||
if (c->trigger && r > 0)
|
||||
trigger_success = true;
|
||||
}
|
||||
|
||||
return triggered != 0;
|
||||
if (!has_trigger || trigger_success)
|
||||
return non_trigger_error < 0 ? non_trigger_error : true;
|
||||
|
||||
return trigger_error < 0 ? trigger_error : false;
|
||||
}
|
||||
|
||||
bool condition_test_list_net(
|
||||
@@ -1477,10 +1493,10 @@ bool condition_test_list_net(
|
||||
condition_test_logger_t logger,
|
||||
void *userdata) {
|
||||
|
||||
return condition_test_list_impl(first, env, to_string, logger, condition_test_net, userdata);
|
||||
return condition_test_list_impl(first, env, to_string, logger, condition_test_net, userdata) > 0;
|
||||
}
|
||||
|
||||
bool condition_test_list(
|
||||
int condition_test_list(
|
||||
Condition *first,
|
||||
char **env,
|
||||
condition_to_string_t to_string,
|
||||
|
||||
@@ -86,7 +86,7 @@ int condition_test(Condition *c, char **env);
|
||||
typedef int (*condition_test_logger_t)(void *userdata, int level, int error, const char *file, int line, const char *func, const char *format, ...) _printf_(7, 8);
|
||||
typedef const char* (*condition_to_string_t)(ConditionType t) _const_;
|
||||
bool condition_test_list_net(Condition *first, char **env, condition_to_string_t to_string, condition_test_logger_t logger, void *userdata);
|
||||
bool condition_test_list(Condition *first, char **env, condition_to_string_t to_string, condition_test_logger_t logger, void *userdata);
|
||||
int condition_test_list(Condition *first, char **env, condition_to_string_t to_string, condition_test_logger_t logger, void *userdata);
|
||||
|
||||
void condition_dump(Condition *c, FILE *f, const char *prefix, condition_to_string_t to_string);
|
||||
void condition_dump_list(Condition *first, FILE *f, const char *prefix, condition_to_string_t to_string);
|
||||
|
||||
@@ -44,6 +44,78 @@
|
||||
#include "user-util.h"
|
||||
#include "virt.h"
|
||||
|
||||
TEST(condition_test_list) {
|
||||
Condition *condition, *sibling;
|
||||
Architecture arch, other_arch;
|
||||
|
||||
ASSERT_OK(arch = uname_architecture());
|
||||
|
||||
ASSERT_OK_POSITIVE(condition_test_list(NULL, environ, NULL, NULL, NULL));
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", false, false)));
|
||||
ASSERT_ERROR(condition_test_list(condition, environ, NULL, NULL, NULL), EINVAL);
|
||||
condition_free(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", true, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_ARCHITECTURE, architecture_to_string(arch), true, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_OK_POSITIVE(condition_test_list(condition, environ, NULL, NULL, NULL));
|
||||
condition_free_list(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_ARCHITECTURE, architecture_to_string(arch), true, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", true, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_OK_POSITIVE(condition_test_list(condition, environ, NULL, NULL, NULL));
|
||||
condition_free_list(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", true, false)));
|
||||
ASSERT_ERROR(condition_test_list(condition, environ, NULL, NULL, NULL), EINVAL);
|
||||
condition_free(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", true, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_PATH_EXISTS, "/thiscertainlywontexist", false, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_OK_ZERO(condition_test_list(condition, environ, NULL, NULL, NULL));
|
||||
condition_free_list(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", true, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_PATH_EXISTS, "/bin/sh", false, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_ERROR(condition_test_list(condition, environ, NULL, NULL, NULL), EINVAL);
|
||||
condition_free_list(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_PATH_EXISTS, "/thiscertainlywontexist", false, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", false, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_OK_ZERO(condition_test_list(condition, environ, NULL, NULL, NULL));
|
||||
condition_free_list(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", false, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_PATH_EXISTS, "/thiscertainlywontexist", false, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_OK_ZERO(condition_test_list(condition, environ, NULL, NULL, NULL));
|
||||
condition_free_list(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_PATH_EXISTS, "/bin/sh", false, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_ARCHITECTURE, architecture_to_string(arch), false, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_OK_POSITIVE(condition_test_list(condition, environ, NULL, NULL, NULL));
|
||||
condition_free_list(condition);
|
||||
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", false, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_ARCHITECTURE, architecture_to_string(arch), true, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_ERROR(condition_test_list(condition, environ, NULL, NULL, NULL), EINVAL);
|
||||
condition_free_list(condition);
|
||||
|
||||
other_arch = arch == ARCHITECTURE_X86_64 ? ARCHITECTURE_X86 : ARCHITECTURE_X86_64;
|
||||
ASSERT_NOT_NULL((condition = condition_new(CONDITION_FIRMWARE, "smbios-field(malformed)", false, false)));
|
||||
ASSERT_NOT_NULL((sibling = condition_new(CONDITION_ARCHITECTURE, architecture_to_string(other_arch), true, false)));
|
||||
LIST_APPEND(conditions, condition, sibling);
|
||||
ASSERT_OK_ZERO(condition_test_list(condition, environ, NULL, NULL, NULL));
|
||||
condition_free_list(condition);
|
||||
}
|
||||
|
||||
TEST(condition_test_path) {
|
||||
Condition *condition;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user