Re: [Qemu-devel] [PATCH v2 1/8] qemu-error: introduce error_report_nolf

[Daniel P. Berrange]

On Mon, Jul 17, 2017 at 08:54:18AM +0200, Ladi Prosek wrote:
> On Fri, Jul 14, 2017 at 12:41 PM, Daniel P. Berrange
> <address@hidden> wrote:
> > On Thu, Jul 13, 2017 at 02:32:06PM +0100, Stefan Hajnoczi wrote:
> >> On Thu, Jul 13, 2017 at 01:02:30PM +0200, Ladi Prosek wrote:
> >> > +/*
> >> > + * Print an error message to current monitor if we have one, else to 
> >> > stderr.
> >> > + * Format arguments like sprintf().  The resulting message should be a
> >> > + * single phrase, with no trailing punctuation.  The no-LF version 
> >> > allows
> >> > + * additional text to be appended with error_printf() or 
> >> > error_vprintf().
> >> > + * Make sure to always close with a newline after all text is printed.
> >> > + * Prepends the current location.
> >> > + * It's wrong to call this in a QMP monitor.  Use error_setg() there.
> >> > + */
> >> > +void error_report_nolf(const char *fmt, ...)
> >> > +{
> >> > +    va_list ap;
> >> > +
> >> > +    va_start(ap, fmt);
> >> > +    error_vreport_nolf(fmt, ap);
> >> > +    va_end(ap);
> >> >  }
> >>
> >> Each call to this function prepends the timestamp, so it cannot really
> >> be used for a sequence of prints in a single line.
> >>
> >> It's a little ugly but I expected something along the lines of
> >> g_strdup_vprintf() in virtio_error():
> >>
> >>   char *msg;
> >>
> >>   va_start(ap, fmt);
> >>   msg = g_strdup_vprintf(fmt, ap);
> >>   va_end(ap);
> >>
> >>   error_report("%s: %s", DEVICE(vdev)->id, msg);
> >>
> >>   g_free(msg);
> >
> > You could get the same thing by turning virtio_Error into a macro with
> > a few games. Rename the current method to virtio_error_impl() and then
> > define:
> >
> >   #define virtio_error(dev, fmt, ...) \
> >      virtio_error_impl(dev, "%s: " fmt, DEVICE(dev)->id, __VA_ARGS__)
> 
> Neat! I think I'll stick with a function though. This doesn't allocate
> but it adds a little bit of code to each call site which has the
> potential of slowing down the fast no-error path (I have no data, just
> the general keeping-the-code-compact-is-good principle). Holler if you
> disagree!

IMHO that would be uneccessary optimization, particular since this is in
the error scenario and so is not performance critical to normal operation.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|