qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [PATCH for-1.4 v2 1/6] error: Clean up error strings


From: Markus Armbruster
Subject: Re: [Qemu-trivial] [PATCH for-1.4 v2 1/6] error: Clean up error strings with embedded newlines
Date: Fri, 08 Feb 2013 19:56:17 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux)

Luiz Capitulino <address@hidden> writes:

> On Fri,  8 Feb 2013 17:17:07 +0100
> Markus Armbruster <address@hidden> wrote:
>
>> The arguments of error_report() should yield a short error string
>> without newlines.
>> 
>> A few places try to print additional help after the error message by
>> embedding newlines in the error string.  That's nice, but let's do it
>> the right way.
>> 
>> Since I'm touching these lines anyway, drop a stray preposition and
>> some tabs.  We don't use tabs for similar messages elsewhere.
>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  hw/kvm/pci-assign.c | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>> 
>> diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
>> index 896cfe8..da64b5b 100644
>> --- a/hw/kvm/pci-assign.c
>> +++ b/hw/kvm/pci-assign.c
>> @@ -936,8 +936,8 @@ retry:
>>              /* Retry with host-side MSI. There might be an IRQ conflict and
>>               * either the kernel or the device doesn't support sharing. */
>>              error_report("Host-side INTx sharing not supported, "
>> -                         "using MSI instead.\n"
>> -                         "Some devices do not to work properly in this 
>> mode.");
>> +                         "using MSI instead");
>> +            error_printf("Some devices do not work properly in this 
>> mode.\n");
>
> This is fixing command-line, right?

Whatever made assign_intx() run.  I'm not familiar with this code, and I
don't know how to trigger the error.

Hmm, one call chain is via PCIDeviceClass init method assigned_initfn().
So it could also be device_add.

> I honestly don't know which is less worse, the current code or having
> to call two different functions in the correct order to report an
> error :(

You call one function to report the error.  In the rare case that you
want to add some explanation or hint to the error message, you use
another function to print to the error destination.  Big deal :)

Explanations and hints are *not* error messages.  Sticking them into the
error string like the code does before my patch happens to work due to
the way error_report() formats the error message.  Relying on that is
unclean.  Besides, error_report()'s function comment clearly stipulates
"no newlines".

> I'd say the best solution for this would be to propagate errors, but this
> will loose LOC support.

I'm unwilling to degrade error message quality even further.



reply via email to

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