On 12/18/15 16:35, Markus Armbruster wrote:
> The arguments of error_setg() & friends should yield a short error
> string without newlines.
>
> Two places try to append additional help to the error message by
> embedding newlines in the error string. That's nice, but let's do it
> the right way, with error_append_hint().
>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
> hw/i386/kvm/pci-assign.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
> index 0fd6923..eec1340 100644
> --- a/hw/i386/kvm/pci-assign.c
> +++ b/hw/i386/kvm/pci-assign.c
> @@ -770,7 +770,7 @@ static char *assign_failed_examine(const AssignedDevice
> *dev)
> "*** $ echo \"%04x:%02x:%02x.%x\" > /sys/bus/pci/drivers/"
> "pci-stub/bind\n"
> "*** $ echo \"%04x %04x\" >
> /sys/bus/pci/drivers/pci-stub/remove_id\n"
> - "***",
> + "***\n",
> ns, dev->host.domain, dev->host.bus, dev->host.slot,
> dev->host.function, vendor_id, device_id,
> dev->host.domain, dev->host.bus, dev->host.slot, dev->host.function,
> @@ -778,7 +778,7 @@ static char *assign_failed_examine(const AssignedDevice
> *dev)
> dev->host.function, vendor_id, device_id);
>
> fail:
> - return g_strdup("Couldn't find out why.");
> + return g_strdup("Couldn't find out why.\n");
> }
>
> static void assign_device(AssignedDevice *dev, Error **errp)
> @@ -812,8 +812,9 @@ static void assign_device(AssignedDevice *dev, Error
> **errp)
> char *cause;
>
> cause = assign_failed_examine(dev);
> - error_setg_errno(errp, -r, "Failed to assign device \"%s\"\n%s",
> - dev->dev.qdev.id, cause);
> + error_setg_errno(errp, -r, "Failed to assign device \"%s\"",
> + dev->dev.qdev.id);
> + error_append_hint(errp, "%s", cause);
> g_free(cause);
> break;
> }
> @@ -912,11 +913,10 @@ retry:
> dev->features |= ASSIGNED_DEVICE_PREFER_MSI_MASK;
> goto retry;
> }
> - error_setg_errno(errp, -r,
> - "Failed to assign irq for \"%s\"\n"
> - "Perhaps you are assigning a device "
> - "that shares an IRQ with another device?",
> + error_setg_errno(errp, -r, "Failed to assign irq for \"%s\"",
> dev->dev.qdev.id);
> + error_append_hint(errp, "Perhaps you are assigning a device "
> + "that shares an IRQ with another device?\n");
> return r;
> }
>
>
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
