[for-1.4,1/6] error: Clean up error strings with embedded newlines

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 <armbru@redhat.com>
---
 hw/kvm/pci-assign.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

On 8 February 2013 12:15, Markus Armbruster <armbru@redhat.com> 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 <armbru@redhat.com>
> ---
>  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"

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

(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.)

-- PMM

Peter Maydell <peter.maydell@linaro.org> writes:

> On 8 February 2013 12:15, Markus Armbruster <armbru@redhat.com> 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 <armbru@redhat.com>
>> ---
>>  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!