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

[Ladi Prosek]

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!

> 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 :|