[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Wenchao Xia
Subject: Re: [Qemu-devel] [PATCH V2 4/5] util: add new function message_printf()
Date: Fri, 24 May 2013 09:41:23 +0800
User-agent: Mozilla/5.0 (Windows NT 5.1; rv:17.0) Gecko/20130509 Thunderbird/17.0.6

于 2013-5-23 23:05, Eric Blake 写道:
On 05/23/2013 02:47 AM, Wenchao Xia wrote:
This function takes an input parameter *output, which can be specified by
caller as stderr, stdout or a monitor. error_vprintf() now calls 
which is a static function added in this patch.

Signed-off-by: Wenchao Xia <address@hidden>
  include/qemu/error-report.h |   13 +++++++++++++
  util/qemu-error.c           |   28 ++++++++++++++++++++++++++--
  2 files changed, 39 insertions(+), 2 deletions(-)

+++ 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);
+    }

Should you use a switch statement here, instead of open coding all
possible enum values?  But that's cosmetic.

More importantly, I think this function should return an int, whose
value is the value of vfprintf.  On the monitor_vfprintf arm, it could
return 0 for now (or, you could unravel THAT problem and fix
monitor_vfprintf to return an output count, but that sounds like a
bigger task).

+void message_printf(const QemuOutput *output, const char *fmt, ...)
+    va_list ap;
+    va_start(ap, fmt);
+    message_vprintf(output, fmt, ap);
+    va_end(ap);

This function should also return int.

   * Print to current monitor if we have one, else to stderr.
   * TODO should return int, so callers can calculate width, but that

And by fixing the underlying function to return int, you could finally
get rid of this TODO.

Given that the int return problem is pre-existing, and probably deserves
its own series, I'm fine with taking this patch as-is.

  It may not have much meaning of returning int now, since underlining
function do not support it so no caller can benefit from it. I think a
series later for that is better, thanks for your reviewing.

Reviewed-by: Eric Blake <address@hidden>

Best Regards

Wenchao Xia

reply via email to

[Prev in Thread] Current Thread [Next in Thread]