morgan/systemd
1
0
Fork 0
mirror of https://github.com/morgan9e/systemd synced 2026-04-15 08:56:15 +09:00
Code Issues Packages Projects Releases Wiki Activity

sd-varlink: change sd_varlink_error() to always return an error

Browse Source 
Let's make sure that sd_varlink_error() always returns an error code, so
that we can use it in a style "return sd_varlink_error(…);" everywhere,
which has two effects: return a good error reply to clients, and exit
the current stack frame with a failure code.

Interestingly sd_varlink_error_invalid_parameter() already worked like
this in some cases, but sd_varlink_error() itself didn't.

This is an alternative to the error handling tweak proposed in #34882,
but I think is a lot more generically useful, since it establishes a
pattern.

I checked our codebase, and this change should generally be OK without
breaking callsites, since the current callers (with exception of the
machined case from #34882) called sd_varlink_error() in the outermost
varlink method call dispatch stack frame, where this behaviour change
does not alter anything.

This is similar btw, how sd_bus_error_setf() and friends always return
error codes too, synthesized from its parameters.
This commit is contained in:
Lennart Poettering
2024-10-30 15:31:08 +01:00
committed by Yu Watanabe
parent 76a3af0630
commit d2ebf5cc1d
2 changed files with 35 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
src/libsystemd/sd-varlink/sd-varlink.c
View File

@@ -1361,7 +1361,11 @@ static int varlink_dispatch_method(sd_varlink *v) {
if (v->state == VARLINK_PROCESSING_METHOD) {
r = sd_varlink_error(v, SD_VARLINK_ERROR_EXPECTED_MORE, NULL);
if (r < 0)
/* If we didn't manage to enqueue an error response, then fail the
* connection completely. Otherwise ignore the error from
* sd_varlink_error() here, as it is synthesized from the function's
* parameters. */
if (r < 0 && VARLINK_STATE_WANTS_REPLY(v->state))
goto fail;
}
} else if (r < 0) {
@@ -1371,7 +1375,8 @@ static int varlink_dispatch_method(sd_varlink *v) {
if (VARLINK_STATE_WANTS_REPLY(v->state)) {
r = sd_varlink_error_invalid_parameter_name(v, bad_field);
if (r < 0)
/* If we didn't manage to enqueue an error response, then fail the connection completely. */
if (r < 0 && VARLINK_STATE_WANTS_REPLY(v->state))
goto fail;
}
}
@@ -1388,14 +1393,16 @@ static int varlink_dispatch_method(sd_varlink *v) {
* method call remains unanswered. */
if (VARLINK_STATE_WANTS_REPLY(v->state)) {
r = sd_varlink_error_errno(v, r);
if (r < 0)
/* If we didn't manage to enqueue an error response, then fail the connection completely. */
if (r < 0 && VARLINK_STATE_WANTS_REPLY(v->state))
goto fail;
}
}
}
} else if (VARLINK_STATE_WANTS_REPLY(v->state)) {
r = sd_varlink_errorbo(v, SD_VARLINK_ERROR_METHOD_NOT_FOUND, SD_JSON_BUILD_PAIR("method", SD_JSON_BUILD_STRING(method)));
if (r < 0)
/* If we didn't manage to enqueue an error response, then fail the connection completely. */
if (r < 0 && VARLINK_STATE_WANTS_REPLY(v->state))
goto fail;
}
@@ -2559,7 +2566,10 @@ _public_ int sd_varlink_error(sd_varlink *v, const char *error_id, sd_json_varia
} else
varlink_set_state(v, VARLINK_PROCESSED_METHOD);
return 1;
/* Everything worked. Let's now return the error we got passed as input as negative errno, so that
* programs can just do "return sd_varlink_error();" and get both: a friendly error reply to clients,
* and an error return from the current stack frame. */
return sd_varlink_error_to_errno(error_id, parameters);
}
_public_ int sd_varlink_errorb(sd_varlink *v, const char *error_id, ...) {

28
src/test/test-varlink.c
View File

@@ -33,14 +33,20 @@ static int method_something(sd_varlink *link, sd_json_variant *parameters, sd_va
int r;
a = sd_json_variant_by_key(parameters, "a");
if (!a)
return sd_varlink_error(link, "io.test.BadParameters", NULL);
if (!a) {
r = sd_varlink_error(link, "io.test.BadParameters", NULL);
assert_se(r == -EBADR);
return r;
}
x = sd_json_variant_integer(a);
b = sd_json_variant_by_key(parameters, "b");
if (!b)
return sd_varlink_error(link, "io.test.BadParameters", NULL);
if (!b) {
r = sd_varlink_error(link, "io.test.BadParameters", NULL);
assert_se(r == -EBADR);
return r;
}
y = sd_json_variant_integer(b);
@@ -105,8 +111,11 @@ static int method_passfd(sd_varlink *link, sd_json_variant *parameters, sd_varli
int r;
a = sd_json_variant_by_key(parameters, "fd");
if (!a)
return sd_varlink_error(link, "io.test.BadParameters", NULL);
if (!a) {
r = sd_varlink_error_invalid_parameter_name(link, "fd");
assert_se(r == -EINVAL);
return r;
}
ASSERT_STREQ(sd_json_variant_string(a), "whoop");
@@ -242,8 +251,7 @@ static void *thread(void *arg) {
_cleanup_(sd_json_variant_unrefp) sd_json_variant *i = NULL;
_cleanup_(sd_json_variant_unrefp) sd_json_variant *wrong = NULL;
sd_json_variant *o = NULL, *k = NULL, *j = NULL;
const char *error_id;
const char *e;
const char *error_id, *e;
int x = 0;
assert_se(sd_json_build(&i, SD_JSON_BUILD_OBJECT(SD_JSON_BUILD_PAIR("a", SD_JSON_BUILD_INTEGER(88)),
@@ -288,6 +296,7 @@ static void *thread(void *arg) {
assert_se(sd_varlink_push_fd(c, fd3) == 2);
assert_se(sd_varlink_callb(c, "io.test.PassFD", &o, &e, SD_JSON_BUILD_OBJECT(SD_JSON_BUILD_PAIR("fd", SD_JSON_BUILD_STRING("whoop")))) >= 0);
assert_se(!e);
int fd4 = sd_varlink_peek_fd(c, 0);
int fd5 = sd_varlink_peek_fd(c, 1);
@@ -298,6 +307,9 @@ static void *thread(void *arg) {
test_fd(fd4, "miau", 4);
test_fd(fd5, "wuff", 4);
assert_se(sd_varlink_callb(c, "io.test.PassFD", &o, &e, SD_JSON_BUILD_OBJECT(SD_JSON_BUILD_PAIR("fdx", SD_JSON_BUILD_STRING("whoopx")))) >= 0);
ASSERT_TRUE(sd_varlink_error_is_invalid_parameter(e, o, "fd"));
assert_se(sd_varlink_callb(c, "io.test.IDontExist", &o, &e, SD_JSON_BUILD_OBJECT(SD_JSON_BUILD_PAIR("x", SD_JSON_BUILD_REAL(5.5)))) >= 0);
ASSERT_STREQ(sd_json_variant_string(sd_json_variant_by_key(o, "method")), "io.test.IDontExist");
ASSERT_STREQ(e, SD_VARLINK_ERROR_METHOD_NOT_FOUND);