[Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Markus Armbruster]

Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they
keep coming back.  Tracked down with the Coccinelle semantic patch
from commit 312fd5f.

Cc: Fam Zheng <famz@redhat.com>
Cc: Peter Crosthwaite <crosthwaitepeter@gmail.com>
Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Changchun Ouyang <changchun.ouyang@intel.com>
Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>
Cc: Pavel Fedin <p.fedin@samsung.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/vmdk.c              |  4 ++--
 hw/arm/xlnx-zynqmp.c      |  2 +-
 hw/ppc/spapr.c            |  3 ++-
 hw/s390x/ipl.c            |  8 ++++----
 hw/s390x/s390-skeys-kvm.c |  2 +-
 hw/s390x/s390-skeys.c     | 16 ++++++++--------
 hw/tpm/tpm_tis.c          |  2 +-
 migration/ram.c           |  2 +-
 migration/savevm.c        |  4 ++--
 net/vhost-user.c          |  6 +++---
 qga/commands-posix.c      |  2 +-
 target-arm/cpu.c          |  2 +-
 target-arm/machine.c      |  4 ++--
 13 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 6f819e4..b4a224e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1494,8 +1494,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
 
     if (sector_num > bs->total_sectors) {
         error_report("Wrong offset: sector_num=0x%" PRIx64
-                " total_sectors=0x%" PRIx64 "\n",
-                sector_num, bs->total_sectors);
+                     " total_sectors=0x%" PRIx64,
+                     sector_num, bs->total_sectors);
         return -EIO;
     }
 
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 87553bb..20a3b2b 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -227,7 +227,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     }
 
     if (!s->boot_cpu_ptr) {
-        error_setg(errp, "ZynqMP Boot cpu %s not found\n", boot_cpu);
+        error_setg(errp, "ZynqMP Boot cpu %s not found", boot_cpu);
         return;
     }
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 6bfb908..6ba68d7 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1831,7 +1831,8 @@ static void ppc_spapr_init(MachineState *machine)
         ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
 
         if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
-            error_report("Specified number of memory slots %"PRIu64" exceeds max supported %d\n",
+            error_report("Specified number of memory slots %" PRIu64
+                         " exceeds max supported %d",
                          machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
             exit(EXIT_FAILURE);
         }
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index b91fcc6..e100428 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -94,7 +94,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
 
         bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
         if (bios_filename == NULL) {
-            error_setg(&l_err, "could not find stage1 bootloader\n");
+            error_setg(&l_err, "could not find stage1 bootloader");
             goto error;
         }
 
@@ -113,7 +113,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
         g_free(bios_filename);
 
         if (bios_size == -1) {
-            error_setg(&l_err, "could not load bootloader '%s'\n", bios_name);
+            error_setg(&l_err, "could not load bootloader '%s'", bios_name);
             goto error;
         }
 
@@ -128,7 +128,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
             kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
         }
         if (kernel_size < 0) {
-            error_setg(&l_err, "could not load kernel '%s'\n", ipl->kernel);
+            error_setg(&l_err, "could not load kernel '%s'", ipl->kernel);
             goto error;
         }
         /*
@@ -156,7 +156,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
             initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
                                               ram_size - initrd_offset);
             if (initrd_size == -1) {
-                error_setg(&l_err, "could not load initrd '%s'\n", ipl->initrd);
+                error_setg(&l_err, "could not load initrd '%s'", ipl->initrd);
                 goto error;
             }
 
diff --git a/hw/s390x/s390-skeys-kvm.c b/hw/s390x/s390-skeys-kvm.c
index 682949a..eaa37ba 100644
--- a/hw/s390x/s390-skeys-kvm.c
+++ b/hw/s390x/s390-skeys-kvm.c
@@ -21,7 +21,7 @@ static int kvm_s390_skeys_enabled(S390SKeysState *ss)
 
     r = skeyclass->get_skeys(ss, 0, 1, &single_key);
     if (r != 0 && r != KVM_S390_GET_SKEYS_NONE) {
-        error_report("S390_GET_KEYS error %d\n", r);
+        error_report("S390_GET_KEYS error %d", r);
     }
     return (r == 0);
 }
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 539ef6d..c9acc18 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -192,8 +192,8 @@ static int qemu_s390_skeys_set(S390SKeysState *ss, uint64_t start_gfn,
     /* Check for uint64 overflow and access beyond end of key data */
     if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
         error_report("Error: Setting storage keys for page beyond the end "
-                "of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
-                count);
+                     "of memory: gfn=%" PRIx64 " count=%" PRId64,
+                     start_gfn, count);
         return -EINVAL;
     }
 
@@ -212,8 +212,8 @@ static int qemu_s390_skeys_get(S390SKeysState *ss, uint64_t start_gfn,
     /* Check for uint64 overflow and access beyond end of key data */
     if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
         error_report("Error: Getting storage keys for page beyond the end "
-                "of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
-                count);
+                     "of memory: gfn=%" PRIx64 " count=%" PRId64,
+                     start_gfn, count);
         return -EINVAL;
     }
 
@@ -257,7 +257,7 @@ static void s390_storage_keys_save(QEMUFile *f, void *opaque)
 
     buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
     if (!buf) {
-        error_report("storage key save could not allocate memory\n");
+        error_report("storage key save could not allocate memory");
         goto end_stream;
     }
 
@@ -277,7 +277,7 @@ static void s390_storage_keys_save(QEMUFile *f, void *opaque)
                  * use S390_SKEYS_SAVE_FLAG_ERROR to indicate failure to the
                  * reading side.
                  */
-                error_report("S390_GET_KEYS error %d\n", error);
+                error_report("S390_GET_KEYS error %d", error);
                 memset(buf, 0, S390_SKEYS_BUFFER_SIZE);
                 eos = S390_SKEYS_SAVE_FLAG_ERROR;
             }
@@ -315,7 +315,7 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
             uint8_t *buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
 
             if (!buf) {
-                error_report("storage key load could not allocate memory\n");
+                error_report("storage key load could not allocate memory");
                 ret = -ENOMEM;
                 break;
             }
@@ -327,7 +327,7 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
 
                 ret = skeyclass->set_skeys(ss, cur_gfn, cur_count, buf);
                 if (ret < 0) {
-                    error_report("S390_SET_KEYS error %d\n", ret);
+                    error_report("S390_SET_KEYS error %d", ret);
                     break;
                 }
                 handled_count += cur_count;
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index ff073d5..95fc66e 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -1051,7 +1051,7 @@ static void tpm_tis_realizefn(DeviceState *dev, Error **errp)
 
     if (tis->irq_num > 15) {
         error_setg(errp, "tpm_tis: IRQ %d for TPM TIS is outside valid range "
-                   "of 0 to 15.\n", tis->irq_num);
+                   "of 0 to 15", tis->irq_num);
         return;
     }
 
diff --git a/migration/ram.c b/migration/ram.c
index 1eb155a..d1611cb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2314,7 +2314,7 @@ static int ram_load_postcopy(QEMUFile *f)
             } else {
                 /* not the 1st TP within the HP */
                 if (host != (last_host + TARGET_PAGE_SIZE)) {
-                    error_report("Non-sequential target page %p/%p\n",
+                    error_report("Non-sequential target page %p/%p",
                                   host, last_host);
                     ret = -EINVAL;
                     break;
diff --git a/migration/savevm.c b/migration/savevm.c
index 0ad1b93..f7d12e1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1563,8 +1563,8 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
     ret = qemu_get_buffer(mis->from_src_file, buffer, (int)length);
     if (ret != length) {
         g_free(buffer);
-        error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d\n",
-                ret, length);
+        error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d",
+                     ret, length);
         return (ret < 0) ? ret : -EAGAIN;
     }
     trace_loadvm_handle_cmd_packaged_received(ret);
diff --git a/net/vhost-user.c b/net/vhost-user.c
index b368a90..e4dd089 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -82,15 +82,15 @@ static int vhost_user_start(int queues, NetClientState *ncs[])
         options.opaque      = s->chr;
         s->vhost_net = vhost_net_init(&options);
         if (!s->vhost_net) {
-            error_report("failed to init vhost_net for queue %d\n", i);
+            error_report("failed to init vhost_net for queue %d", i);
             goto err;
         }
 
         if (i == 0) {
             max_queues = vhost_net_get_max_queues(s->vhost_net);
             if (queues > max_queues) {
-                error_report("you are asking more queues than "
-                             "supported: %d\n", max_queues);
+                error_report("you are asking more queues than supported: %d",
+                             max_queues);
                 goto err;
             }
         }
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index c2ff970..ec0e997 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2247,7 +2247,7 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
          */
         if (errno != ENOENT) {
             error_setg_errno(errp, errno, "Can't open directory"
-                             "\"/sys/devices/system/memory/\"\n");
+                             "\"/sys/devices/system/memory/\"");
         }
         return NULL;
     }
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 30739fc..cac27f9 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -649,7 +649,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         uint32_t nr = cpu->pmsav7_dregion;
 
         if (nr > 0xff) {
-            error_setg(errp, "PMSAv7 MPU #regions invalid %" PRIu32 "\n", nr);
+            error_setg(errp, "PMSAv7 MPU #regions invalid %" PRIu32, nr);
             return;
         }
 
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 36a0d15..b1e1418 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -337,11 +337,11 @@ const char *gicv3_class_name(void)
         return "kvm-arm-gicv3";
 #else
         error_report("KVM GICv3 acceleration is not supported on this "
-                     "platform\n");
+                     "platform");
 #endif
     } else {
         /* TODO: Software emulation is not implemented yet */
-        error_report("KVM is currently required for GICv3 emulation\n");
+        error_report("KVM is currently required for GICv3 emulation");
     }
 
     exit(1);
-- 
2.4.3

Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they
> keep coming back.  Tracked down with the Coccinelle semantic patch
> from commit 312fd5f.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> Cc: Fam Zheng <famz@redhat.com>
> Cc: Peter Crosthwaite <crosthwaitepeter@gmail.com>
> Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
> Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Changchun Ouyang <changchun.ouyang@intel.com>
> Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Cc: Pavel Fedin <p.fedin@samsung.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/vmdk.c              |  4 ++--
>  hw/arm/xlnx-zynqmp.c      |  2 +-
>  hw/ppc/spapr.c            |  3 ++-
>  hw/s390x/ipl.c            |  8 ++++----
>  hw/s390x/s390-skeys-kvm.c |  2 +-
>  hw/s390x/s390-skeys.c     | 16 ++++++++--------
>  hw/tpm/tpm_tis.c          |  2 +-
>  migration/ram.c           |  2 +-
>  migration/savevm.c        |  4 ++--
>  net/vhost-user.c          |  6 +++---
>  qga/commands-posix.c      |  2 +-
>  target-arm/cpu.c          |  2 +-
>  target-arm/machine.c      |  4 ++--
>  13 files changed, 29 insertions(+), 28 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 6f819e4..b4a224e 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1494,8 +1494,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
>  
>      if (sector_num > bs->total_sectors) {
>          error_report("Wrong offset: sector_num=0x%" PRIx64
> -                " total_sectors=0x%" PRIx64 "\n",
> -                sector_num, bs->total_sectors);
> +                     " total_sectors=0x%" PRIx64,
> +                     sector_num, bs->total_sectors);
>          return -EIO;
>      }
>  
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 87553bb..20a3b2b 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -227,7 +227,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>      }
>  
>      if (!s->boot_cpu_ptr) {
> -        error_setg(errp, "ZynqMP Boot cpu %s not found\n", boot_cpu);
> +        error_setg(errp, "ZynqMP Boot cpu %s not found", boot_cpu);
>          return;
>      }
>  
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 6bfb908..6ba68d7 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1831,7 +1831,8 @@ static void ppc_spapr_init(MachineState *machine)
>          ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
>  
>          if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
> -            error_report("Specified number of memory slots %"PRIu64" exceeds max supported %d\n",
> +            error_report("Specified number of memory slots %" PRIu64
> +                         " exceeds max supported %d",
>                           machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
>              exit(EXIT_FAILURE);
>          }
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index b91fcc6..e100428 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -94,7 +94,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>  
>          bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>          if (bios_filename == NULL) {
> -            error_setg(&l_err, "could not find stage1 bootloader\n");
> +            error_setg(&l_err, "could not find stage1 bootloader");
>              goto error;
>          }
>  
> @@ -113,7 +113,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>          g_free(bios_filename);
>  
>          if (bios_size == -1) {
> -            error_setg(&l_err, "could not load bootloader '%s'\n", bios_name);
> +            error_setg(&l_err, "could not load bootloader '%s'", bios_name);
>              goto error;
>          }
>  
> @@ -128,7 +128,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>              kernel_size = load_image_targphys(ipl->kernel, 0, ram_size);
>          }
>          if (kernel_size < 0) {
> -            error_setg(&l_err, "could not load kernel '%s'\n", ipl->kernel);
> +            error_setg(&l_err, "could not load kernel '%s'", ipl->kernel);
>              goto error;
>          }
>          /*
> @@ -156,7 +156,7 @@ static void s390_ipl_realize(DeviceState *dev, Error **errp)
>              initrd_size = load_image_targphys(ipl->initrd, initrd_offset,
>                                                ram_size - initrd_offset);
>              if (initrd_size == -1) {
> -                error_setg(&l_err, "could not load initrd '%s'\n", ipl->initrd);
> +                error_setg(&l_err, "could not load initrd '%s'", ipl->initrd);
>                  goto error;
>              }
>  
> diff --git a/hw/s390x/s390-skeys-kvm.c b/hw/s390x/s390-skeys-kvm.c
> index 682949a..eaa37ba 100644
> --- a/hw/s390x/s390-skeys-kvm.c
> +++ b/hw/s390x/s390-skeys-kvm.c
> @@ -21,7 +21,7 @@ static int kvm_s390_skeys_enabled(S390SKeysState *ss)
>  
>      r = skeyclass->get_skeys(ss, 0, 1, &single_key);
>      if (r != 0 && r != KVM_S390_GET_SKEYS_NONE) {
> -        error_report("S390_GET_KEYS error %d\n", r);
> +        error_report("S390_GET_KEYS error %d", r);
>      }
>      return (r == 0);
>  }
> diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
> index 539ef6d..c9acc18 100644
> --- a/hw/s390x/s390-skeys.c
> +++ b/hw/s390x/s390-skeys.c
> @@ -192,8 +192,8 @@ static int qemu_s390_skeys_set(S390SKeysState *ss, uint64_t start_gfn,
>      /* Check for uint64 overflow and access beyond end of key data */
>      if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
>          error_report("Error: Setting storage keys for page beyond the end "
> -                "of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
> -                count);
> +                     "of memory: gfn=%" PRIx64 " count=%" PRId64,
> +                     start_gfn, count);
>          return -EINVAL;
>      }
>  
> @@ -212,8 +212,8 @@ static int qemu_s390_skeys_get(S390SKeysState *ss, uint64_t start_gfn,
>      /* Check for uint64 overflow and access beyond end of key data */
>      if (start_gfn + count > skeydev->key_count || start_gfn + count < count) {
>          error_report("Error: Getting storage keys for page beyond the end "
> -                "of memory: gfn=%" PRIx64 " count=%" PRId64 "\n", start_gfn,
> -                count);
> +                     "of memory: gfn=%" PRIx64 " count=%" PRId64,
> +                     start_gfn, count);
>          return -EINVAL;
>      }
>  
> @@ -257,7 +257,7 @@ static void s390_storage_keys_save(QEMUFile *f, void *opaque)
>  
>      buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
>      if (!buf) {
> -        error_report("storage key save could not allocate memory\n");
> +        error_report("storage key save could not allocate memory");
>          goto end_stream;
>      }
>  
> @@ -277,7 +277,7 @@ static void s390_storage_keys_save(QEMUFile *f, void *opaque)
>                   * use S390_SKEYS_SAVE_FLAG_ERROR to indicate failure to the
>                   * reading side.
>                   */
> -                error_report("S390_GET_KEYS error %d\n", error);
> +                error_report("S390_GET_KEYS error %d", error);
>                  memset(buf, 0, S390_SKEYS_BUFFER_SIZE);
>                  eos = S390_SKEYS_SAVE_FLAG_ERROR;
>              }
> @@ -315,7 +315,7 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
>              uint8_t *buf = g_try_malloc(S390_SKEYS_BUFFER_SIZE);
>  
>              if (!buf) {
> -                error_report("storage key load could not allocate memory\n");
> +                error_report("storage key load could not allocate memory");
>                  ret = -ENOMEM;
>                  break;
>              }
> @@ -327,7 +327,7 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id)
>  
>                  ret = skeyclass->set_skeys(ss, cur_gfn, cur_count, buf);
>                  if (ret < 0) {
> -                    error_report("S390_SET_KEYS error %d\n", ret);
> +                    error_report("S390_SET_KEYS error %d", ret);
>                      break;
>                  }
>                  handled_count += cur_count;
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index ff073d5..95fc66e 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -1051,7 +1051,7 @@ static void tpm_tis_realizefn(DeviceState *dev, Error **errp)
>  
>      if (tis->irq_num > 15) {
>          error_setg(errp, "tpm_tis: IRQ %d for TPM TIS is outside valid range "
> -                   "of 0 to 15.\n", tis->irq_num);
> +                   "of 0 to 15", tis->irq_num);
>          return;
>      }
>  
> diff --git a/migration/ram.c b/migration/ram.c
> index 1eb155a..d1611cb 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -2314,7 +2314,7 @@ static int ram_load_postcopy(QEMUFile *f)
>              } else {
>                  /* not the 1st TP within the HP */
>                  if (host != (last_host + TARGET_PAGE_SIZE)) {
> -                    error_report("Non-sequential target page %p/%p\n",
> +                    error_report("Non-sequential target page %p/%p",
>                                    host, last_host);
>                      ret = -EINVAL;
>                      break;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 0ad1b93..f7d12e1 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1563,8 +1563,8 @@ static int loadvm_handle_cmd_packaged(MigrationIncomingState *mis)
>      ret = qemu_get_buffer(mis->from_src_file, buffer, (int)length);
>      if (ret != length) {
>          g_free(buffer);
> -        error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d\n",
> -                ret, length);
> +        error_report("CMD_PACKAGED: Buffer receive fail ret=%d length=%d",
> +                     ret, length);
>          return (ret < 0) ? ret : -EAGAIN;
>      }
>      trace_loadvm_handle_cmd_packaged_received(ret);
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index b368a90..e4dd089 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -82,15 +82,15 @@ static int vhost_user_start(int queues, NetClientState *ncs[])
>          options.opaque      = s->chr;
>          s->vhost_net = vhost_net_init(&options);
>          if (!s->vhost_net) {
> -            error_report("failed to init vhost_net for queue %d\n", i);
> +            error_report("failed to init vhost_net for queue %d", i);
>              goto err;
>          }
>  
>          if (i == 0) {
>              max_queues = vhost_net_get_max_queues(s->vhost_net);
>              if (queues > max_queues) {
> -                error_report("you are asking more queues than "
> -                             "supported: %d\n", max_queues);
> +                error_report("you are asking more queues than supported: %d",
> +                             max_queues);
>                  goto err;
>              }
>          }
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index c2ff970..ec0e997 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -2247,7 +2247,7 @@ GuestMemoryBlockList *qmp_guest_get_memory_blocks(Error **errp)
>           */
>          if (errno != ENOENT) {
>              error_setg_errno(errp, errno, "Can't open directory"
> -                             "\"/sys/devices/system/memory/\"\n");
> +                             "\"/sys/devices/system/memory/\"");
>          }
>          return NULL;
>      }
> diff --git a/target-arm/cpu.c b/target-arm/cpu.c
> index 30739fc..cac27f9 100644
> --- a/target-arm/cpu.c
> +++ b/target-arm/cpu.c
> @@ -649,7 +649,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>          uint32_t nr = cpu->pmsav7_dregion;
>  
>          if (nr > 0xff) {
> -            error_setg(errp, "PMSAv7 MPU #regions invalid %" PRIu32 "\n", nr);
> +            error_setg(errp, "PMSAv7 MPU #regions invalid %" PRIu32, nr);
>              return;
>          }
>  
> diff --git a/target-arm/machine.c b/target-arm/machine.c
> index 36a0d15..b1e1418 100644
> --- a/target-arm/machine.c
> +++ b/target-arm/machine.c
> @@ -337,11 +337,11 @@ const char *gicv3_class_name(void)
>          return "kvm-arm-gicv3";
>  #else
>          error_report("KVM GICv3 acceleration is not supported on this "
> -                     "platform\n");
> +                     "platform");
>  #endif
>      } else {
>          /* TODO: Software emulation is not implemented yet */
> -        error_report("KVM is currently required for GICv3 emulation\n");
> +        error_report("KVM is currently required for GICv3 emulation");
>      }
>  
>      exit(1);
> -- 
> 2.4.3
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Cornelia Huck]

On Thu, 10 Dec 2015 18:23:49 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they
> keep coming back.  Tracked down with the Coccinelle semantic patch
> from commit 312fd5f.
> 
> Cc: Fam Zheng <famz@redhat.com>
> Cc: Peter Crosthwaite <crosthwaitepeter@gmail.com>
> Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
> Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Changchun Ouyang <changchun.ouyang@intel.com>
> Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Cc: Pavel Fedin <p.fedin@samsung.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/vmdk.c              |  4 ++--
>  hw/arm/xlnx-zynqmp.c      |  2 +-
>  hw/ppc/spapr.c            |  3 ++-
>  hw/s390x/ipl.c            |  8 ++++----
>  hw/s390x/s390-skeys-kvm.c |  2 +-
>  hw/s390x/s390-skeys.c     | 16 ++++++++--------
>  hw/tpm/tpm_tis.c          |  2 +-
>  migration/ram.c           |  2 +-
>  migration/savevm.c        |  4 ++--
>  net/vhost-user.c          |  6 +++---
>  qga/commands-posix.c      |  2 +-
>  target-arm/cpu.c          |  2 +-
>  target-arm/machine.c      |  4 ++--
>  13 files changed, 29 insertions(+), 28 deletions(-)
> 

s390x parts:

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>

Can somebody with Perl-fu extend checkpatch.pl if these keep coming
back?

Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Bharata B Rao]

On Thu, Dec 10, 2015 at 06:23:49PM +0100, Markus Armbruster wrote:
> Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they
> keep coming back.  Tracked down with the Coccinelle semantic patch
> from commit 312fd5f.
> 
> Cc: Fam Zheng <famz@redhat.com>
> Cc: Peter Crosthwaite <crosthwaitepeter@gmail.com>
> Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
> Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
> Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
> Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Changchun Ouyang <changchun.ouyang@intel.com>
> Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>
> Cc: Pavel Fedin <p.fedin@samsung.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  block/vmdk.c              |  4 ++--
>  hw/arm/xlnx-zynqmp.c      |  2 +-
>  hw/ppc/spapr.c            |  3 ++-

For spapr bits, Acked-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Fam Zheng]

On Thu, 12/10 18:23, Markus Armbruster wrote:
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 6f819e4..b4a224e 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1494,8 +1494,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
>  
>      if (sector_num > bs->total_sectors) {
>          error_report("Wrong offset: sector_num=0x%" PRIx64
> -                " total_sectors=0x%" PRIx64 "\n",
> -                sector_num, bs->total_sectors);
> +                     " total_sectors=0x%" PRIx64,
> +                     sector_num, bs->total_sectors);
>          return -EIO;
>      }
>  

For VMDK change:

Acked-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Hailiang Zhang]

On 2015/12/11 2:16, Cornelia Huck wrote:
> On Thu, 10 Dec 2015 18:23:49 +0100
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Commit 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but they
>> keep coming back.  Tracked down with the Coccinelle semantic patch
>> from commit 312fd5f.
>>
>> Cc: Fam Zheng <famz@redhat.com>
>> Cc: Peter Crosthwaite <crosthwaitepeter@gmail.com>
>> Cc: Bharata B Rao <bharata@linux.vnet.ibm.com>
>> Cc: Dominik Dingel <dingel@linux.vnet.ibm.com>
>> Cc: David Hildenbrand <dahi@linux.vnet.ibm.com>
>> Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
>> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Cc: Changchun Ouyang <changchun.ouyang@intel.com>
>> Cc: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> Cc: Pavel Fedin <p.fedin@samsung.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   block/vmdk.c              |  4 ++--
>>   hw/arm/xlnx-zynqmp.c      |  2 +-
>>   hw/ppc/spapr.c            |  3 ++-
>>   hw/s390x/ipl.c            |  8 ++++----
>>   hw/s390x/s390-skeys-kvm.c |  2 +-
>>   hw/s390x/s390-skeys.c     | 16 ++++++++--------
>>   hw/tpm/tpm_tis.c          |  2 +-
>>   migration/ram.c           |  2 +-
>>   migration/savevm.c        |  4 ++--
>>   net/vhost-user.c          |  6 +++---
>>   qga/commands-posix.c      |  2 +-
>>   target-arm/cpu.c          |  2 +-
>>   target-arm/machine.c      |  4 ++--
>>   13 files changed, 29 insertions(+), 28 deletions(-)
>>
>
> s390x parts:
>
> Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
>
> Can somebody with Perl-fu extend checkpatch.pl if these keep coming
> back?
>
>

Ha, that's really a good idea ...

> .
>

[Qemu-devel] [PATCH 0/4] error: Newline fixes

[Markus Armbruster]

Markus Armbruster (4):
  error: Strip trailing '\n' from error string arguments (again)
  checkpatch: Fix newline detection in error_setg() & friends
  vfio: Use error_report() instead of error_printf() for errors
  error: Fix error_printf() calls lacking newlines

 hw/i386/pc.c                             |  2 +-
 hw/mips/cps.c                            |  2 +-
 hw/nvram/fw_cfg.c                        |  2 +-
 hw/ppc/spapr_cpu_core.c                  |  2 +-
 hw/vfio/platform.c                       |  2 +-
 kvm-all.c                                |  2 +-
 qemu-img.c                               |  4 ++--
 scripts/checkpatch.pl                    |  5 ++++-
 scripts/coccinelle/err-bad-newline.cocci | 29 +++++++++++++++++++++++++++++
 slirp/slirp.c                            |  8 ++++----
 target-i386/kvm.c                        |  2 +-
 ui/vnc.c                                 |  2 +-
 12 files changed, 47 insertions(+), 15 deletions(-)
 create mode 100644 scripts/coccinelle/err-bad-newline.cocci

-- 
2.5.5

[Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Markus Armbruster]

Commit 9af9e0f, 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but
they keep coming back.  checkpatch.pl tries to flag them since commit
5d596c2, but it's not very good at it.  Offenders tracked down with
Coccinelle script scripts/coccinelle/err-bad-newline.cocci, an updated
version of the script from commit 312fd5f.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/mips/cps.c                            |  2 +-
 hw/nvram/fw_cfg.c                        |  2 +-
 hw/ppc/spapr_cpu_core.c                  |  2 +-
 qemu-img.c                               |  4 ++--
 scripts/coccinelle/err-bad-newline.cocci | 29 +++++++++++++++++++++++++++++
 slirp/slirp.c                            |  8 ++++----
 target-i386/kvm.c                        |  2 +-
 7 files changed, 39 insertions(+), 10 deletions(-)
 create mode 100644 scripts/coccinelle/err-bad-newline.cocci

diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index 77c6217..4ef337d 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -73,7 +73,7 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < s->num_vp; i++) {
         cpu = cpu_mips_init(s->cpu_model);
         if (cpu == NULL) {
-            error_setg(errp, "%s: CPU initialization failed\n",  __func__);
+            error_setg(errp, "%s: CPU initialization failed",  __func__);
             return;
         }
 
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 2873030..4fe52f6 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -744,7 +744,7 @@ static int get_fw_cfg_order(FWCfgState *s, const char *name)
     }
 
     /* Stick unknown stuff at the end. */
-    error_report("warning: Unknown firmware file in legacy mode: %s\n", name);
+    error_report("warning: Unknown firmware file in legacy mode: %s", name);
     return FW_CFG_ORDER_OVERRIDE_LAST;
 }
 
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index ec81ee6..23e6c82 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -241,7 +241,7 @@ void spapr_core_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
     }
 
     if (cc->core_id % smp_threads) {
-        error_setg(&local_err, "invalid core id %d\n", cc->core_id);
+        error_setg(&local_err, "invalid core id %d", cc->core_id);
         goto out;
     }
 
diff --git a/qemu-img.c b/qemu-img.c
index 2e40e1f..d2865a5 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3490,7 +3490,7 @@ typedef struct BenchData {
 static void bench_undrained_flush_cb(void *opaque, int ret)
 {
     if (ret < 0) {
-        error_report("Failed flush request: %s\n", strerror(-ret));
+        error_report("Failed flush request: %s", strerror(-ret));
         exit(EXIT_FAILURE);
     }
 }
@@ -3501,7 +3501,7 @@ static void bench_cb(void *opaque, int ret)
     BlockAIOCB *acb;
 
     if (ret < 0) {
-        error_report("Failed request: %s\n", strerror(-ret));
+        error_report("Failed request: %s", strerror(-ret));
         exit(EXIT_FAILURE);
     }
 
diff --git a/scripts/coccinelle/err-bad-newline.cocci b/scripts/coccinelle/err-bad-newline.cocci
new file mode 100644
index 0000000..1316cc8
--- /dev/null
+++ b/scripts/coccinelle/err-bad-newline.cocci
@@ -0,0 +1,29 @@
+// Error messages should not contain newlines.  This script finds
+// messages that do.  Fixing them is manual.
+@r@
+expression errp, eno, cls, fmt;
+position p;
+@@
+(
+error_report(fmt, ...)@p
+|
+error_setg(errp, fmt, ...)@p
+|
+error_setg_errno(errp, eno, fmt, ...)@p
+|
+error_setg_win32(errp, eno, cls, fmt, ...)@p
+|
+error_prepend(errp, fmt, ...)@p
+|
+error_setg_file_open(errp, eno, cls, fmt, ...)@p
+|
+error_reportf_err(errp, fmt, ...)@p
+|
+error_set(errp, cls, fmt, ...)@p
+)
+@script:python@
+fmt << r.fmt;
+p << r.p;
+@@
+if "\\n" in str(fmt):
+    print "%s:%s:%s:%s" % (p[0].file, p[0].line, p[0].column, fmt)
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 7eb183d..47a1652 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -1197,8 +1197,8 @@ static void slirp_socket_save(QEMUFile *f, struct socket *so)
         qemu_put_be16(f, so->so_fport);
         break;
     default:
-        error_report(
-                "so_ffamily unknown, unable to save so_faddr and so_fport\n");
+        error_report("so_ffamily unknown, unable to save so_faddr and"
+                     " so_fport");
     }
     qemu_put_be16(f, so->so_lfamily);
     switch (so->so_lfamily) {
@@ -1207,8 +1207,8 @@ static void slirp_socket_save(QEMUFile *f, struct socket *so)
         qemu_put_be16(f, so->so_lport);
         break;
     default:
-        error_report(
-                "so_ffamily unknown, unable to save so_laddr and so_lport\n");
+        error_report("so_ffamily unknown, unable to save so_laddr and"
+                     " so_lport");
     }
     qemu_put_byte(f, so->so_iptos);
     qemu_put_byte(f, so->so_emu);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 9697e16..0b2016a 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -3250,7 +3250,7 @@ int kvm_arch_irqchip_create(MachineState *ms, KVMState *s)
     if (machine_kernel_irqchip_split(ms)) {
         ret = kvm_vm_enable_cap(s, KVM_CAP_SPLIT_IRQCHIP, 0, 24);
         if (ret) {
-            error_report("Could not enable split irqchip mode: %s\n",
+            error_report("Could not enable split irqchip mode: %s",
                          strerror(-ret));
             exit(1);
         } else {
-- 
2.5.5

[Qemu-devel] [PATCH 2/4] checkpatch: Fix newline detection in error_setg() & friends

[Markus Armbruster]

Commit 5d596c2's regexp assumes the error message string is the first
argument.  Correct for error_report(), wrong for all the others.
Relax the regexp to match newline in anywhere.  This might cause
additional false positives.

While there, update the list of error_reporting functions.

Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/checkpatch.pl | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index afa7f79..c2ca999 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2514,11 +2514,14 @@ sub process {
 	my $qemu_error_funcs = qr{error_setg|
 				error_setg_errno|
 				error_setg_win32|
+				error_setg_file_open|
 				error_set|
+				error_prepend|
+				error_reportf_err|
 				error_vreport|
 				error_report}x;
 
-	if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(\s*\".*\\n/) {
+	if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
 		WARN("Error messages should not contain newlines\n" . $herecurr);
 	}
 
-- 
2.5.5

[Qemu-devel] [PATCH 3/4] vfio: Use error_report() instead of error_printf() for errors

[Markus Armbruster]

Cc: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/vfio/platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 1798a00..a559e7b 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -496,7 +496,7 @@ static int vfio_populate_device(VFIODevice *vbasedev)
         irq.index = i;
         ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
         if (ret) {
-            error_printf("vfio: error getting device %s irq info",
+            error_report("vfio: error getting device %s irq info",
                          vbasedev->name);
             goto irq_err;
         } else {
-- 
2.5.5

[Qemu-devel] [PATCH 4/4] error: Fix error_printf() calls lacking newlines

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/pc.c | 2 +-
 kvm-all.c    | 2 +-
 ui/vnc.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 47593b7..022dd1b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -381,7 +381,7 @@ ISADevice *pc_find_fdc0(void)
         error_report("warning: multiple floppy disk controllers with "
                      "iobase=0x3f0 have been found");
         error_printf("the one being picked for CMOS setup might not reflect "
-                     "your intent");
+                     "your intent\n");
     }
 
     return state.floppy;
diff --git a/kvm-all.c b/kvm-all.c
index ef81ca5..aaa4cc6 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2135,7 +2135,7 @@ void kvm_device_access(int fd, int group, uint64_t attr,
     if (err < 0) {
         error_report("KVM_%s_DEVICE_ATTR failed: %s",
                      write ? "SET" : "GET", strerror(-err));
-        error_printf("Group %d attr 0x%016" PRIx64, group, attr);
+        error_printf("Group %d attr 0x%016" PRIx64 "\n", group, attr);
         abort();
     }
 }
diff --git a/ui/vnc.c b/ui/vnc.c
index 3ce3a5b..bad5874 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3193,7 +3193,7 @@ int vnc_display_password(const char *id, const char *password)
     }
     if (vs->auth == VNC_AUTH_NONE) {
         error_printf_unless_qmp("If you want use passwords please enable "
-                                "password auth using '-vnc ${dpy},password'.");
+                                "password auth using '-vnc ${dpy},password'.\n");
         return -EINVAL;
     }
 
-- 
2.5.5

Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

On 08/03/2016 05:37 AM, Markus Armbruster wrote:
> Commit 9af9e0f, 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but
> they keep coming back.  checkpatch.pl tries to flag them since commit
> 5d596c2, but it's not very good at it.  Offenders tracked down with
> Coccinelle script scripts/coccinelle/err-bad-newline.cocci, an updated
> version of the script from commit 312fd5f.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

> +++ b/scripts/coccinelle/err-bad-newline.cocci
> @@ -0,0 +1,29 @@
> +// Error messages should not contain newlines.  This script finds
> +// messages that do.  Fixing them is manual.
> +@r@
> +expression errp, eno, cls, fmt;
> +position p;
> +@@
> +(
> +error_report(fmt, ...)@p

So you use Coccinelle to find error messages...

> +@script:python@
> +fmt << r.fmt;
> +p << r.p;
> +@@
> +if "\\n" in str(fmt):
> +    print "%s:%s:%s:%s" % (p[0].file, p[0].line, p[0].column, fmt)

...and python to then parse the format string and flag violations.
Slick.  Does it still work across formats like "foo %" PRIdMAX "\n"?

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 2/4] checkpatch: Fix newline detection in error_setg() & friends

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

On 08/03/2016 05:37 AM, Markus Armbruster wrote:
> Commit 5d596c2's regexp assumes the error message string is the first
> argument.  Correct for error_report(), wrong for all the others.
> Relax the regexp to match newline in anywhere.  This might cause
> additional false positives.

Might, but those are still rare enough that we don't have to worry about
it unless someone actually hits such a false positive.

> 
> While there, update the list of error_reporting functions.
> 
> Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/checkpatch.pl | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 3/4] vfio: Use error_report() instead of error_printf() for errors

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 995 bytes --]

On 08/03/2016 05:37 AM, Markus Armbruster wrote:
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/vfio/platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
> index 1798a00..a559e7b 100644
> --- a/hw/vfio/platform.c
> +++ b/hw/vfio/platform.c
> @@ -496,7 +496,7 @@ static int vfio_populate_device(VFIODevice *vbasedev)
>          irq.index = i;
>          ret = ioctl(vbasedev->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq);
>          if (ret) {
> -            error_printf("vfio: error getting device %s irq info",
> +            error_report("vfio: error getting device %s irq info",
>                           vbasedev->name);
>              goto irq_err;
>          } else {
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 4/4] error: Fix error_printf() calls lacking newlines

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On 08/03/2016 05:37 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/i386/pc.c | 2 +-
>  kvm-all.c    | 2 +-
>  ui/vnc.c     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 

I'm guessing these were found with a slight tweak to the Coccinelle
script in 1/4? Worth checking that in, or at least leaving a comment
bread-crumb?

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH 1/4] error: Strip trailing '\n' from error string arguments (again)

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 08/03/2016 05:37 AM, Markus Armbruster wrote:
>> Commit 9af9e0f, 6daf194d, be62a2eb and 312fd5f got rid of a bunch, but
>> they keep coming back.  checkpatch.pl tries to flag them since commit
>> 5d596c2, but it's not very good at it.  Offenders tracked down with
>> Coccinelle script scripts/coccinelle/err-bad-newline.cocci, an updated
>> version of the script from commit 312fd5f.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
>> +++ b/scripts/coccinelle/err-bad-newline.cocci
>> @@ -0,0 +1,29 @@
>> +// Error messages should not contain newlines.  This script finds
>> +// messages that do.  Fixing them is manual.
>> +@r@
>> +expression errp, eno, cls, fmt;
>> +position p;
>> +@@
>> +(
>> +error_report(fmt, ...)@p
>
> So you use Coccinelle to find error messages...
>
>> +@script:python@
>> +fmt << r.fmt;
>> +p << r.p;
>> +@@
>> +if "\\n" in str(fmt):
>> +    print "%s:%s:%s:%s" % (p[0].file, p[0].line, p[0].column, fmt)
>
> ...and python to then parse the format string and flag violations.
> Slick.  Does it still work across formats like "foo %" PRIdMAX "\n"?

Testing... yes, it does.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!

Re: [Qemu-devel] [PATCH 4/4] error: Fix error_printf() calls lacking newlines

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 08/03/2016 05:37 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/i386/pc.c | 2 +-
>>  kvm-all.c    | 2 +-
>>  ui/vnc.c     | 2 +-
>>  3 files changed, 3 insertions(+), 3 deletions(-)
>> 
>
> I'm guessing these were found with a slight tweak to the Coccinelle
> script in 1/4? Worth checking that in, or at least leaving a comment
> bread-crumb?

I simply examined all error_printf() calls.  Only 49.  If the problem
persists, we may want to add a script to catch it.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!