qemu-devel
[Top][All Lists]
Advanced

[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: Wed, 3 Jul 2013 11:10:48 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

On Tue, Jul 02, 2013 at 02:09:24PM +0000, Seiji Aguchi wrote:
> > > 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 
> 
> I plan to introduce timestamp to monitor_printf() and fprintf()
> near future.
> 
> In this case, it is better from a maintenance POV to keep time-handling
>  functionality  in a file that's separate from the qemu error file, so it can 
> be reused
> elsewhere in QEMU if needed.
> 
> It is suggested by Daniel.
> http://marc.info/?l=qemu-devel&m=136741113218944&w=2

Daniel's statement was about the code you copied from libvirt.  It was
not about using the glib function, which simplifies things greatly and
avoids the need for a test suite.

Patches need to make sense today, please do not add extra code with
potential future use in mind:

1. Other developers must be able to read and modify the current codebase
   on its own.  They do not know what potential future changes you were
   thinking about.

2. You may never end up submitting or getting the future stuff upstream.
   Then we'd be left with extra layers that are never used.

Stefan



reply via email to

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