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

[Peter Maydell]

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"

>              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