[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5] Add timestamp to error_report()
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH v5] Add timestamp to error_report() |
Date: |
Tue, 2 Jul 2013 11:09:26 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Jul 01, 2013 at 02:54:07PM -0400, Seiji Aguchi wrote:
> diff --git a/qemu-options.hx b/qemu-options.hx
> index ca6fdf6..a6dac1a 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -3102,3 +3102,15 @@ HXCOMM This is the last statement. Insert new options
> before this line!
> STEXI
> @end table
> ETEXI
> +
> +DEF("msg", HAS_ARG, QEMU_OPTION_msg,
> + "-msg [timestamp=on|off]\n"
> + " change the format of messages\n"
> + " timestamp=on|off enables leading timestamps (default:on)\n",
> + QEMU_ARCH_ALL)
> +STEXI
> address@hidden -msg timestamp=on|off
> address@hidden -msg
> +prepend a timestamp to each log message.
> +(disabled by default)
> +ETEXI
I am confused. If the user specifies -msg then enable_timestamp_msg is
on by default. If the user does not specify -msg then
enable_timestmap_msg is off. Did I get that right?
This means that the default behavior of QEMU does not change but you can
add -msg to enable timestamps.
I'm happy with this but find the documentation confusing.
> diff --git a/util/qemu-time.c b/util/qemu-time.c
> new file mode 100644
> index 0000000..3862788
> --- /dev/null
> +++ b/util/qemu-time.c
> @@ -0,0 +1,26 @@
> +/*
> + * Time handling
> + *
> + * Copyright (C) 2013 Hitachi Data Systems Corp.
> + *
> + * Authors:
> + * Seiji Aguchi <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#include "qemu/time.h"
> +
> +void qemu_get_timestamp_str(char *timestr, size_t len)
> +{
> + GTimeVal tv;
> + gchar *tmp_str = NULL;
> +
> + /* Size of len should be at least TIMESTAMP_LEN to avoid truncation */
> + assert(len >= TIMESTAMP_LEN);
> +
> + g_get_current_time(&tv);
> + tmp_str = g_time_val_to_iso8601(&tv);
> + g_strlcpy((gchar *)timestr, tmp_str, len);
> + g_free(tmp_str);
> +}
Why strcpy into a fixed-size buffer when glib gives us a heap-allocated
string?
This patch would be simpler by dropping qemu-time.c/time.h and simply
doing:
if (enable_timestamp_msg) {
GTimeVal tv;
gchar *timestamp;
g_get_current_time(&tv);
timestamp = g_time_val_to_iso8601(&tv);
error_printf("%s ", timestamp);
g_free(timestamp);
}