Re: [Qemu-devel] [PATCH v2 31/47] hw/pci*: Replace fprintf(stderr, "*\n" with error_report()

[Marcel Apfelbaum]

Hi,

On 30/09/2017 3:16, Alistair Francis wrote:
> Replace a large number of the fprintf(stderr, "*\n" calls with
> error_report(). The functions were renamed with these commands and then
> compiler issues where manually fixed.
>
> find ./* -type f -exec sed -i \
>      'N;N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N;N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N;N;N; {s|fprintf(stderr, 
> "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N;N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
> find ./* -type f -exec sed -i \
>      'N; {s|fprintf(stderr, "\(.*\)\\n"\(.*\));|error_report("\1"\2);|Ig}' \
>      {} +
>
> Some lines where then manually tweaked to pass checkpatch.

We are using coccinelle with great results until now and
we save the scripts in scripts/coccinelle so we can run
them later/improve them.
For your task it should be a really easy script, but
since your work is already done maybe is not worth it.
Anyway, here is the equivalent coccinelle script:

@
expression list ARGS;
@@
- fprintf(stderr, ARGS);
+ error_report(ARGS);

To run:
Install coccinelle: dnf install coccinelle
Save the above as:  <qemu_dir>/scripts/coccinelle/remove_fprintf.cocci
Run it to see the diff:
    spatch --sp-file <qemu_dir>scripts/coccinelle/remove_fprintf.cocci 
--dir <qemu_dir> --include-headers


Reviewed-by: Marcel Apfelbaum <address@hidden>

Thanks,
Marcel

> Signed-off-by: Alistair Francis <address@hidden>
> Cc: "Michael S. Tsirkin" <address@hidden>
> Cc: Marcel Apfelbaum <address@hidden>
> ---
> V2:
>   - Split hw patch into individual directories
>
>   hw/pci-host/bonito.c |  6 +++---
>   hw/pci/pci.c         | 10 +++++-----
>   2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 1767d2af61..48fef86c05 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -38,7 +38,7 @@
>    */
>   
>   #include "qemu/osdep.h"
> -
> +#include "qemu/error-report.h"
>   #include "hw/hw.h"
>   #include "hw/pci/pci.h"
>   #include "hw/i386/pc.h"
> @@ -449,8 +449,8 @@ static uint32_t bonito_sbridge_pciaddr(void *opaque, hwaddr 
> addr)
>       regno = (cfgaddr & BONITO_PCICONF_REG_MASK) >> BONITO_PCICONF_REG_OFFSET;
>   
>       if (idsel == 0) {
> -        fprintf(stderr, "error in bonito pci config address " TARGET_FMT_plx
> -            ",pcimap_cfg=%x\n", addr, s->regs[BONITO_PCIMAP_CFG]);
> +        error_report("error in bonito pci config address " TARGET_FMT_plx
> +            ",pcimap_cfg=%x", addr, s->regs[BONITO_PCIMAP_CFG]);
>           exit(1);
>       }
>       pciaddr = PCI_ADDR(pci_bus_num(phb->bus), devno, funno, regno);
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 1e6fb88eba..1c8395d319 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -515,7 +515,7 @@ static int get_pci_irq_state(QEMUFile *f, void *pv, size_t 
> size,
>       for (i = 0; i < PCI_NUM_PINS; ++i) {
>           irq_state[i] = qemu_get_be32(f);
>           if (irq_state[i] != 0x1 && irq_state[i] != 0) {
> -            fprintf(stderr, "irq state %d: must be 0 or 1.\n",
> +            error_report("irq state %d: must be 0 or 1.",
>                       irq_state[i]);
>               return -EINVAL;
>           }
> @@ -678,7 +678,7 @@ static PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root,
>       unsigned slot;
>   
>       if (!root) {
> -        fprintf(stderr, "No primary PCI bus\n");
> +        error_report("No primary PCI bus");
>           return NULL;
>       }
>   
> @@ -694,7 +694,7 @@ static PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root,
>       }
>   
>       if (dom != 0) {
> -        fprintf(stderr, "No support for non-zero PCI domains\n");
> +        error_report("No support for non-zero PCI domains");
>           return NULL;
>       }
>   
> @@ -1118,8 +1118,8 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
>       assert(region_num >= 0);
>       assert(region_num < PCI_NUM_REGIONS);
>       if (size & (size-1)) {
> -        fprintf(stderr, "ERROR: PCI region size must be pow2 "
> -                    "type=0x%x, size=0x%"FMT_PCIBUS"\n", type, size);
> +        error_report("ERROR: PCI region size must be pow2 "
> +                    "type=0x%x, size=0x%"FMT_PCIBUS"", type, size);
>           exit(1);
>       }
>