diff mbox

[v2,2/8] error: Functions to report warnings and informational messages

Message ID 96f42a10eea7c311f6a50391042c373ffb4247d1.1499453788.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis July 7, 2017, 11:21 p.m. UTC 
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 <alistair.francis@xilinx.com>
---
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(-)

Comments

Markus Armbruster July 10, 2017, 12:56 p.m. UTC | #1 
Alistair Francis <alistair.francis@xilinx.com> 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 <alistair.francis@xilinx.com>
> ---
> 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 <armbru@redhat.com>
diff mbox

Patch

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;
 
 	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.
  * 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.
+ */
+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
+ * 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.
  */
@@ -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);
 }