Message ID | 1470224274-31522-2-git-send-email-armbru@redhat.com |
---|---|
State | New |
Headers | show |
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> 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!
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 {
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