diff options
| author | Alexandru Elisei <alexandru.elisei@arm.com> | 2023-07-07 16:11:16 +0100 |
|---|---|---|
| committer | Will Deacon <will@kernel.org> | 2023-07-12 17:11:56 +0100 |
| commit | 2cc4929cc6b9a382a0c866cf5bbe849e3c0ec271 (patch) | |
| tree | edf3142a90344c8aa9d89bc178d4d6e97e4198e9 | |
| parent | 0b5e55fc032d1c6394b8ec7fe02d842813c903df (diff) | |
| download | kvmtool-2cc4929cc6b9a382a0c866cf5bbe849e3c0ec271.tar.gz | |
util: Make pr_err() return void
Of all the pr_* functions, pr_err() is the only function that returns a
value, which is -1. The code in parse_options is the only code that relies
on pr_err() returning a value, and that value must be exactly -1, because
it is being treated differently than the other return values.
This makes the code opaque, because it's not immediately obvious where that
value comes from, and fragile, as a change in the return value of pr_err
would break it.
Make pr_err() more like the other functions and don't return a value.
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20230707151119.81208-2-alexandru.elisei@arm.com
Signed-off-by: Will Deacon <will@kernel.org>
| -rw-r--r-- | include/kvm/util.h | 2 | ||||
| -rw-r--r-- | util/parse-options.c | 28 | ||||
| -rw-r--r-- | util/util.c | 3 |
3 files changed, 18 insertions, 15 deletions
diff --git a/include/kvm/util.h b/include/kvm/util.h index b4945487..f51f370d 100644 --- a/include/kvm/util.h +++ b/include/kvm/util.h @@ -39,7 +39,7 @@ extern bool do_debug_print; extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1, 2))); extern void die_perror(const char *s) NORETURN; -extern int pr_err(const char *err, ...) __attribute__((format (printf, 1, 2))); +extern void pr_err(const char *err, ...) __attribute__((format (printf, 1, 2))); extern void pr_warning(const char *err, ...) __attribute__((format (printf, 1, 2))); extern void pr_info(const char *err, ...) __attribute__((format (printf, 1, 2))); extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN); diff --git a/util/parse-options.c b/util/parse-options.c index 9a1bbee6..d353256e 100644 --- a/util/parse-options.c +++ b/util/parse-options.c @@ -17,10 +17,13 @@ static int opterror(const struct option *opt, const char *reason, int flags) { if (flags & OPT_SHORT) - return pr_err("switch `%c' %s", opt->short_name, reason); - if (flags & OPT_UNSET) - return pr_err("option `no-%s' %s", opt->long_name, reason); - return pr_err("option `%s' %s", opt->long_name, reason); + pr_err("switch `%c' %s", opt->short_name, reason); + else if (flags & OPT_UNSET) + pr_err("option `no-%s' %s", opt->long_name, reason); + else + pr_err("option `%s' %s", opt->long_name, reason); + + return -1; } static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt, @@ -429,14 +432,15 @@ is_abbreviated: return get_value(p, options, flags); } - if (ambiguous_option) - return pr_err("Ambiguous option: %s " - "(could be --%s%s or --%s%s)", - arg, - (ambiguous_flags & OPT_UNSET) ? "no-" : "", - ambiguous_option->long_name, - (abbrev_flags & OPT_UNSET) ? "no-" : "", - abbrev_option->long_name); + if (ambiguous_option) { + pr_err("Ambiguous option: %s (could be --%s%s or --%s%s)", + arg, + (ambiguous_flags & OPT_UNSET) ? "no-" : "", + ambiguous_option->long_name, + (abbrev_flags & OPT_UNSET) ? "no-" : "", + abbrev_option->long_name); + return -1; + } if (abbrev_option) return get_value(p, abbrev_option, abbrev_flags); return -2; diff --git a/util/util.c b/util/util.c index 1877105e..f59f26e1 100644 --- a/util/util.c +++ b/util/util.c @@ -47,14 +47,13 @@ void die(const char *err, ...) va_end(params); } -int pr_err(const char *err, ...) +void pr_err(const char *err, ...) { va_list params; va_start(params, err); error_builtin(err, params); va_end(params); - return -1; } void pr_warning(const char *warn, ...) |