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

[Luiz Capitulino <lcapitulino@redhat.com>]

On Fri,  8 Feb 2013 17:17:07 +0100
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.
> 
> 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 <armbru@redhat.com>
> ---
>  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?

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

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

>              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 "
> -                     "(check dmesg).\n\tSkip option ROM probe with rombar=0, "
> -                     "or load from file with romfile=", rom_file);
> +        error_report("pci-assign: Cannot read from host %s", rom_file);
> +        error_printf("Device option ROM contents are probably invalid "
> +                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
> +                     "or load from file with romfile=\n");
>          memory_region_destroy(&dev->dev.rom);
>          goto close_rom;
>      }