[PATCH RESEND kvmtool 1/4] util: Make pr_err() return void

Alexandru Elisei
2023-06-30T13:31:59Z

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.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
include/kvm/util.h | 2 +-
util/parse-options.c | 28 ++++++++++++++++------------
util/util.c | 3 +--
3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/include/kvm/util.h b/include/kvm/util.h
index b49454876a83..f51f370d2b37 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 9a1bbee6c271..fb44b48d14c8 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 1877105e3c08..f59f26e1581c 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, ...)
--
2.41.0