[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/8] error: Functions to report warnings and
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/8] error: Functions to report warnings and informational messages |
Date: |
Mon, 10 Jul 2017 14:56:59 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Alistair Francis <address@hidden> writes:
> Add warn_report(), warn_vreport() for reporting warnings, and
> info_report(), info_vreport() for informational messages.
>
> These are implemented them with a helper function factored out of
> error_vreport(), suitably generalized. This patch makes no changes
> to the output of the original error_report() function.
>
> Signed-off-by: Alistair Francis <address@hidden>
> ---
> v2:
> - Don't add *vreport() functions to checkpatch
> - Maintain original comments for the reporting functions
> - Don't change the error report output in this patch
> v1:
> - Don't expose the generic report and vreport() functions
> - Prefix error messages
> - Use vreport instead of qmsg_vreport()
> RFC V3:
> - Change the function and enum names to be more descriptive
> - Add wrapper functions for *_report() and *_vreport()
>
> include/qemu/error-report.h | 7 ++++
> scripts/checkpatch.pl | 5 ++-
> util/qemu-error.c | 97
> ++++++++++++++++++++++++++++++++++++++++++---
> 3 files changed, 102 insertions(+), 7 deletions(-)
>
> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> index 3001865896..e1c8ae1a52 100644
> --- a/include/qemu/error-report.h
> +++ b/include/qemu/error-report.h
> @@ -35,8 +35,15 @@ void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> void error_vprintf_unless_qmp(const char *fmt, va_list ap) GCC_FMT_ATTR(1,
> 0);
> void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> void error_set_progname(const char *argv0);
> +
> void error_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
> +void warn_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
> +void info_vreport(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
> +
> void error_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> +void warn_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> +void info_report(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> +
> const char *error_get_progname(void);
> extern bool enable_timestamp_msg;
>
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index 73efc927a9..b76fe30ad3 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2534,8 +2534,9 @@ sub process {
> error_set|
> error_prepend|
> error_reportf_err|
> - error_vreport|
> - error_report}x;
> + error_report|
> + warn_report|
> + info_report}x;
This check is about newlines in format strings. Both the FOO_report()
and the FOO_vreport() take format strings. Therefore, keep
error_vreport() and add warn_vreport(), info_vreport().
>
> if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
> ERROR("Error messages should not contain newlines\n" .
> $herecurr);
> diff --git a/util/qemu-error.c b/util/qemu-error.c
> index 1c5e35ecdb..b555247e24 100644
> --- a/util/qemu-error.c
> +++ b/util/qemu-error.c
> @@ -14,6 +14,16 @@
> #include "monitor/monitor.h"
> #include "qemu/error-report.h"
>
> +/*
> + * @report_type is the type of message: error, warning or
> + * informational.
> + */
> +typedef enum {
> + REPORT_TYPE_ERROR,
> + REPORT_TYPE_WARNING,
> + REPORT_TYPE_INFO,
> +} report_type;
> +
> void error_printf(const char *fmt, ...)
> {
> va_list ap;
> @@ -179,17 +189,28 @@ static void print_loc(void)
>
> bool enable_timestamp_msg;
> /*
> - * Print an error message to current monitor if we have one, else to stderr.
> + * Print a message to current monitor if we have one, else to stderr.
Need a sentence on @report_type right here. Perhaps
* @report_type is the type of message: error, warning or
* informational.
> * Format arguments like vsprintf(). The resulting message should be
> * a single phrase, with no newline or trailing punctuation.
> * Prepend the current location and append a newline.
> * It's wrong to call this in a QMP monitor. Use error_setg() there.
> */
> -void error_vreport(const char *fmt, va_list ap)
> +static void vreport(report_type type, const char *fmt, va_list ap)
> {
> GTimeVal tv;
> gchar *timestr;
>
> + switch (type) {
> + case REPORT_TYPE_ERROR:
> + break;
> + case REPORT_TYPE_WARNING:
> + error_printf("warning: ");
> + break;
> + case REPORT_TYPE_INFO:
> + error_printf("info: ");
> + break;
> + }
> +
> if (enable_timestamp_msg && !cur_mon) {
> g_get_current_time(&tv);
> timestr = g_time_val_to_iso8601(&tv);
> @@ -204,8 +225,41 @@ void error_vreport(const char *fmt, va_list ap)
>
> /*
> * Print an error message to current monitor if we have one, else to stderr.
> - * Format arguments like sprintf(). The resulting message should be a
> - * single phrase, with no newline or trailing punctuation.
> + * Format arguments like vsprintf(). The resulting message should be
> + * a single phrase, with no newline or trailing punctuation.
> + * Prepend the current location and append a newline.
> + * It's wrong to call this in a QMP monitor. Use error_setg() there.
> + */
> +void error_vreport(const char *fmt, va_list ap)
> +{
> + vreport(REPORT_TYPE_ERROR, fmt, ap);
> +}
> +
> +/*
> + * Print a warning message to current monitor if we have one, else to stderr.
> + */
You missed one :)
> +void warn_vreport(const char *fmt, va_list ap)
> +{
> + vreport(REPORT_TYPE_WARNING, fmt, ap);
> +}
> +
> +/*
> + * Print an information message to current monitor if we have one, else to
> + * stderr.
> + * Format arguments like vsprintf(). The resulting message should be
> + * a single phrase, with no newline or trailing punctuation.
> + * Prepend the current location and append a newline.
> + * It's wrong to call this in a QMP monitor. Use error_setg() there.
> + */
> +void info_vreport(const char *fmt, va_list ap)
> +{
> + vreport(REPORT_TYPE_INFO, fmt, ap);
> +}
> +
> +/*
> + * Print an error message to current monitor if we have one, else to stderr.
> + * Format arguments like vsprintf(). The resulting message should be
like sprintf()
> + * a single phrase, with no newline or trailing punctuation.
> * Prepend the current location and append a newline.
> * It's wrong to call this in a QMP monitor. Use error_setg() there.
> */
void error_report(const char *fmt, ...)
> @@ -214,6 +268,39 @@ void error_report(const char *fmt, ...)
> va_list ap;
>
> va_start(ap, fmt);
> - error_vreport(fmt, ap);
> + vreport(REPORT_TYPE_ERROR, fmt, ap);
> + va_end(ap);
> +}
> +
> +/*
> + * Print a warning message to current monitor if we have one, else to stderr.
> + * Format arguments like sprintf(). The resulting message should be a
> + * single phrase, with no newline or trailing punctuation.
> + * Prepend the current location and append a newline.
> + * It's wrong to call this in a QMP monitor. Use error_setg() there.
> + */
> +void warn_report(const char *fmt, ...)
> +{
> + va_list ap;
> +
> + va_start(ap, fmt);
> + vreport(REPORT_TYPE_WARNING, fmt, ap);
> + va_end(ap);
> +}
> +
> +/*
> + * Print an information message to current monitor if we have one, else to
> + * stderr.
> + * Format arguments like sprintf(). The resulting message should be a
> + * single phrase, with no newline or trailing punctuation.
> + * Prepend the current location and append a newline.
> + * It's wrong to call this in a QMP monitor. Use error_setg() there.
> + */
> +void info_report(const char *fmt, ...)
> +{
> + va_list ap;
> +
> + va_start(ap, fmt);
> + vreport(REPORT_TYPE_INFO, fmt, ap);
> va_end(ap);
> }
With these nits touched up:
Reviewed-by: Markus Armbruster <address@hidden>
- [Qemu-devel] [PATCH v2 0/8] Implement a warning_report function, Alistair Francis, 2017/07/07
- [Qemu-devel] [PATCH v2 1/8] util/qemu-error: Rename error_print_loc() to be more generic, Alistair Francis, 2017/07/07
- [Qemu-devel] [PATCH v2 2/8] error: Functions to report warnings and informational messages, Alistair Francis, 2017/07/07
- Re: [Qemu-devel] [PATCH v2 2/8] error: Functions to report warnings and informational messages,
Markus Armbruster <=
- [Qemu-devel] [PATCH v2 3/8] Convert error_report() to warn_report(), Alistair Francis, 2017/07/07
- [Qemu-devel] [PATCH v2 4/8] hw/i386: Improve some of the warning messages, Alistair Francis, 2017/07/07
- [Qemu-devel] [PATCH v2 5/8] char-socket: Report TCP socket waiting as information, Alistair Francis, 2017/07/07
- [Qemu-devel] [PATCH v2 7/8] Convert error_report*_err() to warn_report*_err(), Alistair Francis, 2017/07/07
- [Qemu-devel] [PATCH v2 6/8] error: Implement the warn and free Error functions, Alistair Francis, 2017/07/07