diff mbox

[2/4] error: Clean up errors with embedded newlines (again), part 1

Message ID 1449768232-22924-3-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Dec. 10, 2015, 5:23 p.m. UTC
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.  Commit 474c213 cleaned up some, but they keep comint
back.  Offenders tracked down with the Coccinelle semantic patch from
commit 312fd5f.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/pc.c | 4 ++--
 kvm-all.c    | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Laszlo Ersek Dec. 10, 2015, 5:36 p.m. UTC | #1
On 12/10/15 18:23, Markus Armbruster 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.  Commit 474c213 cleaned up some, but they keep comint

s/comint/coming/, you've been reading too many spy stories! :)

> back.  Offenders tracked down with the Coccinelle semantic patch from
> commit 312fd5f.

My share of the blame is b86f46132 here... But you saw that patch! ;)

Anyway, this cleanup is consistent with your advice in
<http://thread.gmane.org/gmane.comp.emulators.qemu/374410/focus=374855>.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Pavel Fedin <p.fedin@samsung.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/i386/pc.c | 4 ++--
>  kvm-all.c    | 6 +++---
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5e20e07..2c89ed1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -418,8 +418,8 @@ static void pc_cmos_init_late(void *opaque)
>  
>      if (state.multiple) {
>          error_report("warning: multiple floppy disk controllers with "
> -                     "iobase=0x3f0 have been found;\n"
> -                     "the one being picked for CMOS setup might not reflect "
> +                     "iobase=0x3f0 have been found");
> +        error_printf("the one being picked for CMOS setup might not reflect "
>                       "your intent");
>      }
>      pc_cmos_init_floppy(s, state.floppy);
> diff --git a/kvm-all.c b/kvm-all.c
> index c648b81..9a7dd21 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -2020,9 +2020,9 @@ void kvm_device_access(int fd, int group, uint64_t attr,
>                             write ? KVM_SET_DEVICE_ATTR : KVM_GET_DEVICE_ATTR,
>                             &kvmattr);
>      if (err < 0) {
> -        error_report("KVM_%s_DEVICE_ATTR failed: %s\n"
> -                     "Group %d attr 0x%016" PRIx64, write ? "SET" : "GET",
> -                     strerror(-err), group, attr);
> +        error_report("KVM_%s_DEVICE_ATTR failed: %s",
> +                     write ? "SET" : "GET", strerror(-err));
> +        error_printf("Group %d attr 0x%016" PRIx64, group, attr);
>          abort();
>      }
>  }
>
Markus Armbruster Dec. 14, 2015, 9:27 a.m. UTC | #2
Laszlo Ersek <lersek@redhat.com> writes:

> On 12/10/15 18:23, Markus Armbruster 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.  Commit 474c213 cleaned up some, but they keep comint
>
> s/comint/coming/, you've been reading too many spy stories! :)

Unfortunately, my demand for spy stories has been fully satisfied by the
news.

>> back.  Offenders tracked down with the Coccinelle semantic patch from
>> commit 312fd5f.
>
> My share of the blame is b86f46132 here... But you saw that patch! ;)

Yes, but it never made it to the front of my review queue...

> Anyway, this cleanup is consistent with your advice in
> <http://thread.gmane.org/gmane.comp.emulators.qemu/374410/focus=374855>.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
diff mbox

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5e20e07..2c89ed1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -418,8 +418,8 @@  static void pc_cmos_init_late(void *opaque)
 
     if (state.multiple) {
         error_report("warning: multiple floppy disk controllers with "
-                     "iobase=0x3f0 have been found;\n"
-                     "the one being picked for CMOS setup might not reflect "
+                     "iobase=0x3f0 have been found");
+        error_printf("the one being picked for CMOS setup might not reflect "
                      "your intent");
     }
     pc_cmos_init_floppy(s, state.floppy);
diff --git a/kvm-all.c b/kvm-all.c
index c648b81..9a7dd21 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2020,9 +2020,9 @@  void kvm_device_access(int fd, int group, uint64_t attr,
                            write ? KVM_SET_DEVICE_ATTR : KVM_GET_DEVICE_ATTR,
                            &kvmattr);
     if (err < 0) {
-        error_report("KVM_%s_DEVICE_ATTR failed: %s\n"
-                     "Group %d attr 0x%016" PRIx64, write ? "SET" : "GET",
-                     strerror(-err), group, attr);
+        error_report("KVM_%s_DEVICE_ATTR failed: %s",
+                     write ? "SET" : "GET", strerror(-err));
+        error_printf("Group %d attr 0x%016" PRIx64, group, attr);
         abort();
     }
 }