[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: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH V2 4/5] util: add new function message_printf() |
Date: |
Thu, 23 May 2013 09:05:08 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130514 Thunderbird/17.0.6 |
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
> message_vprintf(),
> 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.
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH V2 0/5] qapi and snapshot code clean up in block layer, Wenchao Xia, 2013/05/23
- [Qemu-devel] [PATCH V2 1/5] block: drop bs_snapshots global variable, Wenchao Xia, 2013/05/23
- [Qemu-devel] [PATCH V2 2/5] block: move snapshot code in block.c to block/snapshot.c, Wenchao Xia, 2013/05/23
- [Qemu-devel] [PATCH V2 3/5] block: move qmp and info dump related code to block/qapi.c, Wenchao Xia, 2013/05/23
- [Qemu-devel] [PATCH V2 4/5] util: add new function message_printf(), Wenchao Xia, 2013/05/23
- [Qemu-devel] [PATCH V2 5/5] block: dump to specified output for bdrv_snapshot_dump() and bdrv_image_info_dump(), Wenchao Xia, 2013/05/23