On Sun, Oct 1, 2017 at 1:11 AM, Marcel Apfelbaum <marcel@redhat.com> wrote:
> 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
Thanks,
I have already decided that coccinelle is the way to go here, this
ended up much more complex then I was hoping.
That's good to know we save the previous scripts.
Thanks,
Alistair
>
>
> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
>
> Thanks,
> Marcel
>
>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Marcel Apfelbaum <marcel@redhat.com>
>> ---
>> 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);
>> }
>>
>
>