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
> <berra...@redhat.com> 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 :|
