[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH for-1.4 1/6] error: Clean up erro
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH for-1.4 1/6] error: Clean up error strings with embedded newlines |
Date: |
Fri, 08 Feb 2013 14:50:55 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) |
Peter Maydell <address@hidden> writes:
> On 8 February 2013 12:15, 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.
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> hw/kvm/pci-assign.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
>> index 896cfe8..c31a7f2 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 to work properly in this
>> mode.");
>
> "do not work properly"
Since I'm touching the line anyway...
>> dev->features |= ASSIGNED_DEVICE_PREFER_MSI_MASK;
>> goto retry;
>> }
>> @@ -1903,10 +1903,10 @@ static void
>> assigned_dev_load_option_rom(AssignedDevice *dev)
>> memset(ptr, 0xff, st.st_size);
>>
>> if (!fread(ptr, 1, st.st_size, fp)) {
>> - error_report("pci-assign: Cannot read from host %s\n"
>> - "\tDevice option ROM contents are probably invalid "
>> + error_report("pci-assign: Cannot read from host %s", rom_file);
>> + error_printf("\tDevice option ROM contents are probably invalid "
>> "(check dmesg).\n\tSkip option ROM probe with
>> rombar=0, "
>> - "or load from file with romfile=", rom_file);
>> + "or load from file with romfile=\n");
>
> So should error_printf() strings end with \n or not? This one does,
> the one in the hunk above doesn't...
The hunk above is a bug. Will fix.
> (also we should probably either drop those \t indents or have
> error_printf() add them automatically or otherwise indicate that
> this is followon additional info about a previous error.)
error_printf() is just like printf(), except it prints to wherever error
messages go. In fact, error_report() uses error_printf() to print.
We could have an API to for emitting helpful text after an error
message. Its implementation could print the text indented, if we so
desire. I doubt it's really worthwhile.
Since this is the only place that indents text following an error
message, let's just drop the tabs.
Thanks!
- [Qemu-trivial] [PATCH for-1.4 0/6] Error reporting fixes, Markus Armbruster, 2013/02/08
- [Qemu-trivial] [PATCH for-1.4 2/6] error: Clean up abuse of error_report() for help, Markus Armbruster, 2013/02/08
- [Qemu-trivial] [PATCH for-1.4 6/6] vl: Exit unsuccessfully on option argument syntax error, Markus Armbruster, 2013/02/08
- [Qemu-trivial] [PATCH for-1.4 5/6] vl: Drop redundant "parse error" reports, Markus Armbruster, 2013/02/08
- [Qemu-trivial] [PATCH for-1.4 1/6] error: Clean up error strings with embedded newlines, Markus Armbruster, 2013/02/08
- [Qemu-trivial] [PATCH for-1.4 4/6] qemu-option: Disable two helpful messages that got broken recently, Markus Armbruster, 2013/02/08
- [Qemu-trivial] [PATCH for-1.4 3/6] error: Strip trailing '\n' from error string arguments (again), Markus Armbruster, 2013/02/08