Re: [Qemu-devel] [PATCH RFC 12/48] error: New error_printf() and error_vprintf()

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> On Wed, 24 Feb 2010 18:55:24 +0100
> Markus Armbruster <address@hidden> wrote:
>
>> FIXME They should return int, so callers can calculate width.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  qemu-error.c |   49 ++++++++++++++++++++++++++++++++++++++++++-------
>>  qemu-error.h |   14 ++++++++++++++
>>  2 files changed, 56 insertions(+), 7 deletions(-)
>> 
>> diff --git a/qemu-error.c b/qemu-error.c
>> index 63bcdcf..76c660a 100644
>> --- a/qemu-error.c
>> +++ b/qemu-error.c
>> @@ -1,18 +1,53 @@
>> +/*
>> + * Error reporting
>> + *
>> + * Copyright (C) 2010 Red Hat Inc.
>> + *
>> + * Authors:
>> + *  Markus Armbruster <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 <stdio.h>
>>  #include "monitor.h"
>>  #include "sysemu.h"
>>  
>> -void qemu_error(const char *fmt, ...)
>> +/*
>> + * Print to current monitor if we have one, else to stderr.
>> + * FIXME should return int, so callers can calculate width, but that
>> + * requires surgery to monitor_printf().  Left for another day.
>> + */
>> +void error_vprintf(const char *fmt, va_list ap)
>>  {
>> -    va_list args;
>> -
>> -    va_start(args, fmt);
>>      if (cur_mon) {
>> -        monitor_vprintf(cur_mon, fmt, args);
>> +        monitor_vprintf(cur_mon, fmt, ap);
>>      } else {
>> -        vfprintf(stderr, fmt, args);
>> +        vfprintf(stderr, fmt, ap);
>>      }
>> -    va_end(args);
>> +}
>
>  This can be static.

Yes.  But why would that be useful?  It's neither a name space pollution
nor does it poke a hole into an abstraction.

>> +
>> +/*
>> + * Print to current monitor if we have one, else to stderr.
>> + * FIXME just like error_vprintf()
>> + */
>> +void error_printf(const char *fmt, ...)
>> +{
>> +    va_list ap;
>> +
>> +    va_start(ap, fmt);
>> +    error_vprintf(fmt, ap);
>> +    va_end(ap);
>> +}
>
>  This function's name is inconsistent with qemu_error() and
> qemu_error_new().

I'm fond of prepending qemu_ to random symbols left and right.  Yes, I
know I'm reading QEMU source code, thank you :)

If the names here are really important: What about stripping qemu_ from
qemu_error() & friends?