From 5256326261c1f8b4022133ff664a922ed111d6ab Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Wed, 21 Aug 2024 16:15:24 -0400 Subject: [PATCH 1/5] sysupdated: Fixup minor formatting issues --- src/sysupdate/sysupdated.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index e2c3d7e102..776211f74b 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -163,7 +163,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Job*, job_free); DEFINE_HASH_OPS_WITH_VALUE_DESTRUCTOR(job_hash_ops, uint64_t, uint64_hash_func, uint64_compare_func, Job, job_free); -static int job_new(JobType type, Target *t, sd_bus_message *msg, JobComplete complete_cb, Job **ret) { +static int job_new(JobType type, Target *t, sd_bus_message *msg, JobComplete complete_cb, Job **ret) { _cleanup_(job_freep) Job *j = NULL; int r; @@ -844,8 +844,8 @@ static int target_method_list_finish( static int target_method_list(sd_bus_message *msg, void *userdata, sd_bus_error *error) { Target *t = ASSERT_PTR(userdata); _cleanup_(job_freep) Job *j = NULL; - int r; uint64_t flags; + int r; assert(msg); @@ -906,8 +906,8 @@ static int target_method_describe(sd_bus_message *msg, void *userdata, sd_bus_er Target *t = ASSERT_PTR(userdata); _cleanup_(job_freep) Job *j = NULL; const char *version; - int r; uint64_t flags; + int r; assert(msg); From e0081f18a0bd980f005bd07b99766a1249d55ebd Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Wed, 21 Aug 2024 21:35:25 -0400 Subject: [PATCH 2/5] sysupdated: Fixup redundant constant name SD_ stands for systemd, so SD_SYSTEMD_* is SYSTEMD_SYSTEMD_* --- man/org.freedesktop.sysupdate1.xml | 8 ++++---- src/sysupdate/sysupdate-util.h | 2 +- src/sysupdate/sysupdated.c | 8 ++++---- src/sysupdate/updatectl.c | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/man/org.freedesktop.sysupdate1.xml b/man/org.freedesktop.sysupdate1.xml index ac0e9152a1..329fab8635 100644 --- a/man/org.freedesktop.sysupdate1.xml +++ b/man/org.freedesktop.sysupdate1.xml @@ -180,11 +180,11 @@ node /org/freedesktop/sysupdate1/target/host { Describe() returns all known information about a given version as a JSON object. The version argument is used to pass the version to be described. Additional options may be passed through the flags argument. The only supported value currently - is SD_SYSTEMD_SYSUPDATE_OFFLINE, which prevents the call from accessing the network + is SD_SYSUPDATE_OFFLINE, which prevents the call from accessing the network and restricts results to locally installed versions. This flag is defined as follows: -#define SD_SYSTEMD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) +#define SD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) The returned JSON object contains several known keys. More keys may be added in the future. The @@ -327,8 +327,8 @@ node /org/freedesktop/sysupdate1/target/host { class contains the class of the Target being acted upon, and name contains the name of the same Target. Additionally, each method exposes its arguments to the rule. Arguments containing flags are unwrapped into a variable-per-flag; for example, the - SD_SYSTEMD_SYSUPDATE_OFFLINE flag is exposed as a variable named - offline. + SD_SYSUPDATE_OFFLINE flag is exposed as a variable named offline. + diff --git a/src/sysupdate/sysupdate-util.h b/src/sysupdate/sysupdate-util.h index 56339a87b1..989e069329 100644 --- a/src/sysupdate/sysupdate-util.h +++ b/src/sysupdate/sysupdate-util.h @@ -4,4 +4,4 @@ int reboot_now(void); -#define SD_SYSTEMD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) +#define SD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index 776211f74b..83295fe0d9 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -856,7 +856,7 @@ static int target_method_list(sd_bus_message *msg, void *userdata, sd_bus_error const char *details[] = { "class", target_class_to_string(t->class), "name", t->name, - "offline", one_zero(FLAGS_SET(flags, SD_SYSTEMD_SYSUPDATE_OFFLINE)), + "offline", one_zero(FLAGS_SET(flags, SD_SYSUPDATE_OFFLINE)), NULL }; @@ -875,7 +875,7 @@ static int target_method_list(sd_bus_message *msg, void *userdata, sd_bus_error if (r < 0) return r; - j->offline = FLAGS_SET(flags, SD_SYSTEMD_SYSUPDATE_OFFLINE); + j->offline = FLAGS_SET(flags, SD_SYSUPDATE_OFFLINE); r = job_start(j); if (r < 0) @@ -922,7 +922,7 @@ static int target_method_describe(sd_bus_message *msg, void *userdata, sd_bus_er "class", target_class_to_string(t->class), "name", t->name, "version", version, - "offline", one_zero(FLAGS_SET(flags, SD_SYSTEMD_SYSUPDATE_OFFLINE)), + "offline", one_zero(FLAGS_SET(flags, SD_SYSUPDATE_OFFLINE)), NULL }; @@ -945,7 +945,7 @@ static int target_method_describe(sd_bus_message *msg, void *userdata, sd_bus_er if (!j->version) return log_oom(); - j->offline = FLAGS_SET(flags, SD_SYSTEMD_SYSUPDATE_OFFLINE); + j->offline = FLAGS_SET(flags, SD_SYSUPDATE_OFFLINE); r = job_start(j); if (r < 0) diff --git a/src/sysupdate/updatectl.c b/src/sysupdate/updatectl.c index 39957ea1a0..151fa9e1d7 100644 --- a/src/sysupdate/updatectl.c +++ b/src/sysupdate/updatectl.c @@ -430,7 +430,7 @@ static int list_versions(sd_bus *bus, const char *target_path) { &error, &reply, "t", - arg_offline ? SD_SYSTEMD_SYSUPDATE_OFFLINE : 0); + arg_offline ? SD_SYSUPDATE_OFFLINE : 0); if (r < 0) return log_bus_error(r, &error, NULL, "call List"); @@ -473,7 +473,7 @@ static int list_versions(sd_bus *bus, const char *target_path) { op, "st", *version, - arg_offline ? SD_SYSTEMD_SYSUPDATE_OFFLINE : 0); + arg_offline ? SD_SYSUPDATE_OFFLINE : 0); if (r < 0) return log_error_errno(r, "Failed to call Describe: %m"); TAKE_PTR(op); @@ -508,7 +508,7 @@ static int describe(sd_bus *bus, const char *target_path, const char *version) { &reply, "st", version, - arg_offline ? SD_SYSTEMD_SYSUPDATE_OFFLINE : 0); + arg_offline ? SD_SYSUPDATE_OFFLINE : 0); if (r < 0) return log_bus_error(r, &error, NULL, "call Describe"); @@ -707,7 +707,7 @@ static int check_finished(sd_bus_message *reply, void *userdata, sd_bus_error *r op, "st", new_version, - arg_offline ? SD_SYSTEMD_SYSUPDATE_OFFLINE : 0); + arg_offline ? SD_SYSUPDATE_OFFLINE : 0); if (r < 0) return log_error_errno(r, "Failed to call Describe: %m"); TAKE_PTR(op); From b1bcaa0eb147a934f4f3042af27bf7435e643574 Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Wed, 21 Aug 2024 21:36:25 -0400 Subject: [PATCH 3/5] sysupdated: Verify inputs more rigorously Also return better errors --- src/sysupdate/sysupdate-util.h | 1 + src/sysupdate/sysupdated.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/sysupdate/sysupdate-util.h b/src/sysupdate/sysupdate-util.h index 989e069329..8504fd24b7 100644 --- a/src/sysupdate/sysupdate-util.h +++ b/src/sysupdate/sysupdate-util.h @@ -5,3 +5,4 @@ int reboot_now(void); #define SD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) +#define SD_SYSUPDATE_FLAGS_ALL (SD_SYSUPDATE_OFFLINE) diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index 83295fe0d9..6405ff0e83 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -853,6 +853,9 @@ static int target_method_list(sd_bus_message *msg, void *userdata, sd_bus_error if (r < 0) return r; + if ((flags & ~SD_SYSUPDATE_FLAGS_ALL) != 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid flags specified"); + const char *details[] = { "class", target_class_to_string(t->class), "name", t->name, @@ -916,7 +919,10 @@ static int target_method_describe(sd_bus_message *msg, void *userdata, sd_bus_er return r; if (isempty(version)) - return -EINVAL; + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Version must be specified"); + + if ((flags & ~SD_SYSUPDATE_FLAGS_ALL) != 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid flags specified"); const char *details[] = { "class", target_class_to_string(t->class), @@ -1057,7 +1063,7 @@ static int target_method_update(sd_bus_message *msg, void *userdata, sd_bus_erro return r; if (flags != 0) - return sd_bus_error_set_errnof(error, SYNTHETIC_ERRNO(EINVAL), "Flags argument must be 0: %m"); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Flags must be 0"); if (isempty(version)) action = "org.freedesktop.sysupdate1.update"; From 31fc2fb039d468da4aaea11facab9d080942278c Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Wed, 21 Aug 2024 21:44:53 -0400 Subject: [PATCH 4/5] sysupdate: Simplify sysupdate_run_simple callsite Allows the caller to optionally pass in a target, instead of making everyone call target_get_argument at the call site. --- src/sysupdate/sysupdated.c | 39 +++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/sysupdate/sysupdated.c b/src/sysupdate/sysupdated.c index 6405ff0e83..4edfea937e 100644 --- a/src/sysupdate/sysupdated.c +++ b/src/sysupdate/sysupdated.c @@ -725,13 +725,20 @@ static int target_new(Manager *m, TargetClass class, const char *name, const cha return 0; } -static int sysupdate_run_simple(sd_json_variant **ret, ...) { +static int sysupdate_run_simple(sd_json_variant **ret, Target *t, ...) { _cleanup_close_pair_ int pipe[2] = EBADF_PAIR; _cleanup_(pidref_done_sigkill_wait) PidRef pid = PIDREF_NULL; _cleanup_fclose_ FILE *f = NULL; _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; + _cleanup_free_ char *target_arg = NULL; int r; + if (t) { + r = target_get_argument(t, &target_arg); + if (r < 0) + return r; + } + r = pipe2(pipe, O_CLOEXEC); if (r < 0) return -errno; @@ -760,7 +767,12 @@ static int sysupdate_run_simple(sd_json_variant **ret, ...) { _exit(EXIT_FAILURE); } - va_start(ap, ret); + if (target_arg && strv_extend(&args, target_arg) < 0) { + log_oom(); + _exit(EXIT_FAILURE); + } + + va_start(ap, t); while ((arg = va_arg(ap, char*))) { r = strv_extend(&args, arg); if (r < 0) @@ -1158,16 +1170,11 @@ static int target_method_vacuum(sd_bus_message *msg, void *userdata, sd_bus_erro static int target_method_get_version(sd_bus_message *msg, void *userdata, sd_bus_error *error) { Target *t = ASSERT_PTR(userdata); - _cleanup_free_ char *target_arg = NULL; _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; sd_json_variant *version_json; int r; - r = target_get_argument(t, &target_arg); - if (r < 0) - return r; - - r = sysupdate_run_simple(&v, "--offline", "list", target_arg, NULL); + r = sysupdate_run_simple(&v, t, "--offline", "list", NULL); if (r < 0) return r; @@ -1189,16 +1196,11 @@ static int target_method_get_version(sd_bus_message *msg, void *userdata, sd_bus } static int target_get_appstream(Target *t, char ***ret) { - _cleanup_free_ char *target_arg = NULL; _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL; sd_json_variant *appstream_url_json; int r; - r = target_get_argument(t, &target_arg); - if (r < 0) - return r; - - r = sysupdate_run_simple(&v, "--offline", "list", target_arg, NULL); + r = sysupdate_run_simple(&v, t, "--offline", "list", NULL); if (r < 0) return r; @@ -1241,18 +1243,11 @@ static int target_method_get_appstream(sd_bus_message *msg, void *userdata, sd_b static int target_list_components(Target *t, char ***ret_components, bool *ret_have_default) { _cleanup_(sd_json_variant_unrefp) sd_json_variant *json = NULL; _cleanup_strv_free_ char **components = NULL; - _cleanup_free_ char *target_arg = NULL; sd_json_variant *v; bool have_default; int r; - if (t) { - r = target_get_argument(t, &target_arg); - if (r < 0) - return r; - } - - r = sysupdate_run_simple(&json, "components", target_arg, NULL); + r = sysupdate_run_simple(&json, t, "components", NULL); if (r < 0) return r; From d470a6c22736f98748b918a74f141ea44a52298b Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Wed, 21 Aug 2024 22:10:49 -0400 Subject: [PATCH 5/5] sysupdate: man: Cleanup sections about flags --- man/org.freedesktop.sysupdate1.xml | 50 ++++++++++++++++-------------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/man/org.freedesktop.sysupdate1.xml b/man/org.freedesktop.sysupdate1.xml index 329fab8635..66c21fab3d 100644 --- a/man/org.freedesktop.sysupdate1.xml +++ b/man/org.freedesktop.sysupdate1.xml @@ -170,61 +170,60 @@ node /org/freedesktop/sysupdate1/target/host { Methods - List() returns a list of versions available for this target. The - flags argument can be used to pass additional options, with bit 0 mapping to - . When is true, this method returns only the versions - installed locally. Otherwise, this method pulls metadata from the network and returns all versions - available for this target. Use Describe() to query more information about each - version returned by this method. - - Describe() returns all known information about a given version as a JSON - object. The version argument is used to pass the version to be described. Additional - options may be passed through the flags argument. The only supported value currently - is SD_SYSUPDATE_OFFLINE, which prevents the call from accessing the network - and restricts results to locally installed versions. This flag is defined as follows: + List() returns a list of versions available for this target. Additional + options may be passed through the flags argument. Valid flags are defined as follows: + #define SD_SYSUPDATE_OFFLINE (UINT64_C(1) << 0) - The returned JSON object contains several known keys. More keys may be added in the future. The - currently known keys are as follows: + When SD_SYSUPDATE_OFFLINE is set, this method returns only the versions + installed locally. Otherwise, this method pulls metadata from the network and returns all versions + available for this target. Use Describe() to query more information about each + version returned by this method. + + Describe() returns all known information about a given version as a JSON + object. The version argument is used to pass the version to be described. Additional + options may be passed through the flags argument. This method supports the same flags + as List(). The returned JSON object contains several known keys. More keys may be + added in the future. The currently known keys are as follows: - version + version A string containing the version number. - newest + newest A boolean indicating whether this version is the latest available for the target. - available + available A boolean indicating whether this version is available for download. - installed + installed A boolean indicating whether this version is installed locally. - obsolete + obsolete A boolean indicating whether this version is considered obsolete by the service, and is therefore disallowed from being installed. - protected + protected A boolean indicating whether this version is exempt from deletion by a Vacuum() operation. - changelog_urls + changelog_urls A list of strings that contain user-presentable URLs to ChangeLogs associated with this version. @@ -326,9 +325,12 @@ node /org/freedesktop/sysupdate1/target/host { All methods called on this interface expose additional variables to the polkit rules. class contains the class of the Target being acted upon, and name contains the name of the same Target. Additionally, each method exposes its arguments to the - rule. Arguments containing flags are unwrapped into a variable-per-flag; for example, the - SD_SYSUPDATE_OFFLINE flag is exposed as a variable named offline. - + rule. Flags are mapped as follows: + + + SD_SYSUPDATE_OFFLINE → update + +