Re: [Qemu-devel] [PATCH V2 4/5] util: add new function message_printf()

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 23 May 2013 16:47:15 +0800
Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:

> This function takes an input parameter *output, which can be specified by
> caller as stderr, stdout or a monitor. error_vprintf() now calls message_vprintf(),
> which is a static function added in this patch.
> 
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>

This solution looks good to me:

Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  include/qemu/error-report.h |   13 +++++++++++++
>  util/qemu-error.c           |   28 ++++++++++++++++++++++++++--
>  2 files changed, 39 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
> index c902cc1..cdde78b 100644
> --- a/include/qemu/error-report.h
> +++ b/include/qemu/error-report.h
> @@ -14,6 +14,8 @@
>  #define QEMU_ERROR_H
>  
>  #include <stdarg.h>
> +#include <stdio.h>
> +#include "qemu/typedefs.h"
>  
>  typedef struct Location {
>      /* all members are private to qemu-error.c */
> @@ -32,6 +34,17 @@ void loc_set_none(void);
>  void loc_set_cmdline(char **argv, int idx, int cnt);
>  void loc_set_file(const char *fname, int lno);
>  
> +typedef struct QemuOutput {
> +    enum { OUTPUT_STREAM, OUTPUT_MONITOR } kind;
> +    union {
> +        FILE *stream;
> +        Monitor *monitor;
> +    };
> +} QemuOutput;
> +
> +void message_printf(const QemuOutput *output, const char *fmt, ...)
> +                    GCC_FMT_ATTR(2, 3);
> +
>  void error_vprintf(const char *fmt, va_list ap) GCC_FMT_ATTR(1, 0);
>  void error_printf(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  void error_printf_unless_qmp(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
> diff --git a/util/qemu-error.c b/util/qemu-error.c
> index 08a36f4..c7ff0a8 100644
> --- a/util/qemu-error.c
> +++ b/util/qemu-error.c
> @@ -13,6 +13,25 @@
>  #include <stdio.h>
>  #include "monitor/monitor.h"
>  
> +static GCC_FMT_ATTR(2, 0)
> +void message_vprintf(const QemuOutput *output, const char *fmt, va_list ap)
> +{
> +    if (output->kind == OUTPUT_STREAM) {
> +        vfprintf(output->stream, fmt, ap);
> +    } else if (output->kind == OUTPUT_MONITOR) {
> +        monitor_vprintf(output->monitor, fmt, ap);
> +    }
> +}
> +
> +void message_printf(const QemuOutput *output, const char *fmt, ...)
> +{
> +    va_list ap;
> +
> +    va_start(ap, fmt);
> +    message_vprintf(output, fmt, ap);
> +    va_end(ap);
> +}
> +
>  /*
>   * Print to current monitor if we have one, else to stderr.
>   * TODO should return int, so callers can calculate width, but that
> @@ -20,11 +39,16 @@
>   */
>  void error_vprintf(const char *fmt, va_list ap)
>  {
> +    QemuOutput output;
> +
>      if (cur_mon) {
> -        monitor_vprintf(cur_mon, fmt, ap);
> +        output.kind = OUTPUT_MONITOR;
> +        output.monitor = cur_mon;
>      } else {
> -        vfprintf(stderr, fmt, ap);
> +        output.kind = OUTPUT_STREAM;
> +        output.stream = stderr;
>      }
> +    message_vprintf(&output, fmt, ap);
>  }
>  
>  /*