Function-like macro with the same name as a typedef confuses Coccinelle

Function-like macro with the same name as a typedef confuses Coccinelle

[Markus Armbruster]

I discovered that Vladimir's auto-propagated-errp.cocci leaves
hw/arm/armsse.c unchanged, even though it clearly should change it.
Running spatch with --debug prints (among lots of other things)

    let's go
    -----------------------------------------------------------------------
    -----------------------------------------------------------------------
    parse error 
     = error in hw/arm/armsse.c; set verbose_parsing for more info
    badcount: 7
    bad: }
    bad: 
    bad: static void nsccfg_handler(void *opaque, int n, int level)
    bad: {
    BAD:!!!!!     ARMSSE *s = ARMSSE(opaque);
    bad: 
    bad:     s->nsccfg = level;
    bad: }
    parse error 

Alright, what's wrong with this?  ARMSSE is both a function-like macro
and a typedef:

    #define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), TYPE_ARMSSE)

    typedef struct ARMSSE {
        ...
    } ARMSSE;

This appears to spook Coccinelle badly enough to skip code using ARMSSE.

If I rename the macro to ARMSSE_() just for testing, it produces the
transformation I'd expect.

Grepping for typedef names is bothersome, so I used ctags -x to build a
cross reference table, and searched that for function-like macros
clashing with typedefs.  Result:

include/hw/arm/armsse.h:#define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), TYPE_ARMSSE)
include/hw/arm/armsse.h:} ARMSSE;
include/hw/block/swim.h:#define SWIM(obj) OBJECT_CHECK(SWIM, (obj), TYPE_SWIM)
include/hw/block/swim.h:} SWIM;
target/rx/cpu-qom.h:#define RXCPU(obj) \
target/rx/cpu.h:typedef struct RXCPU RXCPU;
target/s390x/translate.c:#define BD(N, BB, BD) { BB,  4, 0, FLD_C_b##N, FLD_O_b##N }, \
hw/audio/ac97.c:} BD;

The last one is a name clash in unrelated files; should not bother
Coccinelle.

The first three are due to QOM.  By convention, the name of the
function-like macro to convert to the QOM type is the QOM type in
ALL_CAPS.  Clash when the QOM type is ALL_CAPS already.

Clearly, Coccinelle is getting spooked to easily.  Regardless, three
questions:

1. Are ALL_CAPS typedef names a good idea?  We shout macros to tell
readers "beware, possibly magic".  Shouting other stuff as well
undermines that.

2. The compiler is quite cool with us using the same name for a
function-like macro and a not-function-like non-macro.  But is it a good
idea?

3. Do we want to work around Coccinelle's parsing flaw?  Having it skip
so much code is clearly suboptimal, and potentially dangerous.  The
obvious work-around is to rename the three problematic types so they
contain at least one lower-case letter.

Re: Function-like macro with the same name as a typedef confuses Coccinelle

[Peter Maydell]

On Thu, 2 Apr 2020 at 13:06, Markus Armbruster <armbru@redhat.com> wrote:
>
> I discovered that Vladimir's auto-propagated-errp.cocci leaves
> hw/arm/armsse.c unchanged, even though it clearly should change it.
> Running spatch with --debug prints (among lots of other things)

> Clearly, Coccinelle is getting spooked to easily.

Is it worth asking on the coccinelle mailing list about whether
coccinelle could be made to be less picky in this area ?

> Regardless, three questions:
>
> 1. Are ALL_CAPS typedef names a good idea?  We shout macros to tell
> readers "beware, possibly magic".  Shouting other stuff as well
> undermines that.
>
> 2. The compiler is quite cool with us using the same name for a
> function-like macro and a not-function-like non-macro.  But is it a good
> idea?

Probably not a great idea, and if we really only do it 3 times
it's not too hard to change I suppose. I think this basically
arises when the natural name for the struct happens to be all
uppercase already because the device name is an acronym. We
don't usually titlecase acronyms in structure names (eg
we say 'SCSIBus', not 'ScsiBus'), and (legacy exceptions aside)
we don't usually tack on a trailing 'State' or 'Device'
to the main device state struct these days -- so if your device's
natural name is an acronym then the struct ends up all-caps.
If we don't like all-caps struct names then ideally we'd
adjust one or the other of those conventions so we have a
consistent way to avoid them.

For 'ARMSSE' we could I suppose rename it 'ArmSSE', which would
be in line with current corporate branding but out of line with
most of the other places we use 'ARM' in a struct name :-)

Q: how many all-upper-case typedefs do we have in total (whether
they have a clashing macro or not)? Your argument 1 would
suggest we should look to change them all, not merely the ones
Coccinelle trips over.

thanks
-- PMM

Re: Function-like macro with the same name as a typedef confuses Coccinelle

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Thu, 2 Apr 2020 at 13:06, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> I discovered that Vladimir's auto-propagated-errp.cocci leaves
>> hw/arm/armsse.c unchanged, even though it clearly should change it.
>> Running spatch with --debug prints (among lots of other things)
>
>> Clearly, Coccinelle is getting spooked to easily.
>
> Is it worth asking on the coccinelle mailing list about whether
> coccinelle could be made to be less picky in this area ?

I guess we owe them the feedback.  I'll look into minimizing the
reproducer.

>> Regardless, three questions:
>>
>> 1. Are ALL_CAPS typedef names a good idea?  We shout macros to tell
>> readers "beware, possibly magic".  Shouting other stuff as well
>> undermines that.
>>
>> 2. The compiler is quite cool with us using the same name for a
>> function-like macro and a not-function-like non-macro.  But is it a good
>> idea?
>
> Probably not a great idea, and if we really only do it 3 times
> it's not too hard to change I suppose. I think this basically
> arises when the natural name for the struct happens to be all
> uppercase already because the device name is an acronym. We
> don't usually titlecase acronyms in structure names (eg
> we say 'SCSIBus', not 'ScsiBus'), and (legacy exceptions aside)
> we don't usually tack on a trailing 'State' or 'Device'
> to the main device state struct these days -- so if your device's
> natural name is an acronym then the struct ends up all-caps.

Plausible.

> If we don't like all-caps struct names then ideally we'd
> adjust one or the other of those conventions so we have a
> consistent way to avoid them.

I don't like the State suffix, it's four characters carrying zero bits
of information.  The Device suffix at least tells me it's (supposedly)
an instance of TYPE_DEVICE.

I'd prefer title-casing acronyms when needed to avoid confusion with
macros.

> For 'ARMSSE' we could I suppose rename it 'ArmSSE', which would
> be in line with current corporate branding but out of line with
> most of the other places we use 'ARM' in a struct name :-)

Pick your poison :)

> Q: how many all-upper-case typedefs do we have in total (whether
> they have a clashing macro or not)? Your argument 1 would
> suggest we should look to change them all, not merely the ones
> Coccinelle trips over.

$ egrep -c '^[A-Z_]+\s+typedef' cxref
116
$ egrep '^[A-Z_]+\s+typedef' cxref 
ACPIGPE          typedef     107 include/hw/acpi/acpi.h typedef struct ACPIGPE ACPIGPE;
ACPIREGS         typedef     108 include/hw/acpi/acpi.h typedef struct ACPIREGS ACPIREGS;
AES_KEY          typedef      11 include/crypto/aes.h typedef struct aes_key_st AES_KEY;
AHCI_SG          typedef     292 hw/ide/ahci_internal.h } QEMU_PACKED AHCI_SG;
ARMCPU           typedef      57 target/arm/cpu-qom.h typedef struct ARMCPU ARMCPU;
ARMSSE           typedef     218 include/hw/arm/armsse.h } ARMSSE;
ARMSSECPUID      typedef      39 include/hw/misc/armsse-cpuid.h } ARMSSECPUID;
ARMSSEMHU        typedef      42 include/hw/misc/armsse-mhu.h } ARMSSEMHU;
BD               typedef     147 hw/audio/ac97.c  } BD;
BIOS_TABLES_TEST typedef      77 tests/uefi-test-tools/UefiTestToolsPkg/Include/Guid/BiosTablesTest.h } BIOS_TABLES_TEST;
CCID_BULK_IN     typedef     190 hw/usb/dev-smartcard-reader.c } CCID_BULK_IN;
CD               typedef     490 hw/arm/smmuv3-internal.h } CD;
CHS              typedef     515 tests/qtest/hd-geo-test.c } CHS;
CHST             typedef      49 tests/qtest/hd-geo-test.c } CHST;
CIW              typedef      40 include/hw/s390x/css.h } QEMU_PACKED CIW;
CMB              typedef      64 include/hw/s390x/css.h } QEMU_PACKED CMB;
CMBE             typedef      77 include/hw/s390x/css.h } QEMU_PACKED CMBE;
CMSDKAPBTIMER    typedef      37 include/hw/timer/cmsdk-apb-timer.h } CMSDKAPBTIMER;
CMSDKAPBUART     typedef      45 include/hw/char/cmsdk-apb-uart.h } CMSDKAPBUART;
CMS_VTOC         typedef     188 pc-bios/s390-ccw/bootmap.h } __attribute__ ((packed)) CMS_VTOC;
CORE_ADDR        typedef    1647 disas/hppa.c     typedef unsigned int CORE_ADDR;
CPUTLB           typedef     222 include/exec/cpu-defs.h } CPUTLB;
CPUTLB           typedef     230 include/exec/cpu-defs.h typedef struct CPUTLB { } CPUTLB;
CRISCPU          typedef      53 target/cris/cpu-qom.h typedef struct CRISCPU CRISCPU;
CRW              typedef     201 include/hw/s390x/ioinst.h } CRW;
CURLAIOCB        typedef      97 block/curl.c     } CURLAIOCB;
DMAAIOCB         typedef      85 dma-helpers.c    } DMAAIOCB;
EHCI_STATES      typedef      69 hw/usb/hcd-ehci.c } EHCI_STATES;
FIS              typedef     355 tests/qtest/libqos/ahci.h } __attribute__((__packed__)) FIS;
FM_OPL           typedef      90 hw/audio/fmopl.h } FM_OPL;
FPCR             typedef      41 linux-user/arm/nwfpe/fpsr.h typedef unsigned int FPCR; /* type for floating point control register */
FPREG            typedef      56 linux-user/arm/nwfpe/fpa11.h } FPREG;
FPSR             typedef      40 linux-user/arm/nwfpe/fpsr.h typedef unsigned int FPSR; /* type for floating point status register */
GA_WTSINFOA      typedef    1955 qga/commands-win32.c } GA_WTSINFOA;
GO               typedef     110 include/hw/s390x/event-facility.h } QEMU_PACKED GO;
GUID             typedef      18 contrib/elf2dmp/pdb.h } GUID;
HPPACPU          typedef      50 target/hppa/cpu-qom.h typedef struct HPPACPU HPPACPU;
IDEDMA           typedef      24 include/hw/ide/internal.h typedef struct IDEDMA IDEDMA;
IMAGE_DATA_DIRECTORY typedef      48 contrib/elf2dmp/pe.h } __attribute__ ((packed)) IMAGE_DATA_DIRECTORY;
IMAGE_DEBUG_DIRECTORY typedef     100 contrib/elf2dmp/pe.h } __attribute__ ((packed)) IMAGE_DEBUG_DIRECTORY;
IMAGE_DOS_HEADER typedef      33 contrib/elf2dmp/pe.h } __attribute__ ((packed)) IMAGE_DOS_HEADER;
IMAGE_FILE_HEADER typedef      43 contrib/elf2dmp/pe.h } __attribute__ ((packed)) IMAGE_FILE_HEADER;
IPMIBT           typedef      66 include/hw/ipmi/ipmi_bt.h } IPMIBT;
IPMIKCS          typedef      69 include/hw/ipmi/ipmi_kcs.h } IPMIKCS;
IRB              typedef     132 include/hw/s390x/ioinst.h } IRB;
IRQMP            typedef      64 hw/intc/grlib_irqmp.c } IRQMP;
LDL_VTOC         typedef     156 pc-bios/s390-ccw/bootmap.h } __attribute__ ((packed)) LDL_VTOC;
LI               typedef      91 include/libdecnumber/decNumberLocal.h typedef long int LI; /* for printf arguments only */
MDB              typedef     126 include/hw/s390x/event-facility.h } QEMU_PACKED MDB;
MDBO             typedef     121 include/hw/s390x/event-facility.h } QEMU_PACKED MDBO;
MDMSU            typedef     157 include/hw/s390x/event-facility.h } QEMU_PACKED MDMSU;
MIPSCPU          typedef      55 target/mips/cpu-qom.h typedef struct MIPSCPU MIPSCPU;
MSGUID           typedef      81 block/vhdx.h     } MSGUID;
MTO              typedef      97 include/hw/s390x/event-facility.h } QEMU_PACKED MTO;
NCQFIS           typedef     451 tests/qtest/libqos/ahci.h } __attribute__((__packed__)) NCQFIS;
NFSRPC           typedef      76 block/nfs.c      } NFSRPC;
OPL_CH           typedef      54 hw/audio/fmopl.h } OPL_CH;
OPL_SLOT         typedef      38 hw/audio/fmopl.h }OPL_SLOT;
OPL_TIMERHANDLER typedef       5 hw/audio/fmopl.h typedef void (*OPL_TIMERHANDLER)(void *param, int channel, double interval_Sec);
ORB              typedef     142 include/hw/s390x/ioinst.h } ORB;
PDB_DS_HEADER    typedef      34 contrib/elf2dmp/pdb.h } PDB_DS_HEADER;
PDB_DS_ROOT      typedef      48 contrib/elf2dmp/pdb.h } PDB_DS_ROOT;
PDB_DS_TOC       typedef      39 contrib/elf2dmp/pdb.h } PDB_DS_TOC;
PDB_STREAM_INDEXES typedef     193 contrib/elf2dmp/pdb.h } PDB_STREAM_INDEXES;
PDB_STREAM_INDEXES_OLD typedef     179 contrib/elf2dmp/pdb.h } PDB_STREAM_INDEXES_OLD;
PDB_SYMBOLS      typedef     170 contrib/elf2dmp/pdb.h } PDB_SYMBOLS;
PDB_SYMBOLS_OLD  typedef     149 contrib/elf2dmp/pdb.h } PDB_SYMBOLS_OLD;
PDB_SYMBOL_FILE  typedef     110 contrib/elf2dmp/pdb.h } PDB_SYMBOL_FILE;
PDB_SYMBOL_FILE_EX typedef     124 contrib/elf2dmp/pdb.h } PDB_SYMBOL_FILE_EX;
PDB_SYMBOL_IMPORT typedef     138 contrib/elf2dmp/pdb.h } PDB_SYMBOL_IMPORT;
PDB_SYMBOL_RANGE typedef      85 contrib/elf2dmp/pdb.h } PDB_SYMBOL_RANGE;
PDB_SYMBOL_RANGE_EX typedef      97 contrib/elf2dmp/pdb.h } PDB_SYMBOL_RANGE_EX;
PDB_SYMBOL_SOURCE typedef     130 contrib/elf2dmp/pdb.h } PDB_SYMBOL_SOURCE;
PDB_TYPES        typedef      75 contrib/elf2dmp/pdb.h } PDB_TYPES;
PDB_TYPES_OLD    typedef      57 contrib/elf2dmp/pdb.h } PDB_TYPES_OLD;
PMCW             typedef      98 include/hw/s390x/ioinst.h } PMCW;
PRD              typedef     473 tests/qtest/libqos/ahci.h } __attribute__((__packed__)) PRD;
PSW              typedef      17 pc-bios/s390-ccw/s390-arch.h } __attribute__ ((aligned(8))) PSW;
PSW              typedef      46 target/s390x/cpu.h } PSW;
PTR              typedef      12 include/disas/dis-asm.h typedef void *PTR;
QEDAIOCB         typedef     150 block/qed.h      } QEDAIOCB;
QEMUBH           typedef      97 include/qemu/typedefs.h typedef struct QEMUBH QEMUBH;
QEMUFIFO         typedef      68 ui/console.c     } QEMUFIFO;
QFWCFG           typedef      18 tests/qtest/libqos/fw_cfg.h typedef struct QFWCFG QFWCFG;
QJSON            typedef     109 include/qemu/typedefs.h typedef struct QJSON QJSON;
QOHCI_PCI        typedef      17 tests/qtest/usb-hcd-ohci-test.c typedef struct QOHCI_PCI QOHCI_PCI;
QSDHCI           typedef      25 tests/qtest/libqos/sdhci.h typedef struct QSDHCI QSDHCI;
QSDHCI_PCI       typedef      27 tests/qtest/libqos/sdhci.h typedef struct QSDHCI_PCI QSDHCI_PCI;
QSDHCI_PCI       typedef      28 tests/qtest/libqos/x86_64_pc-machine.c typedef struct QSDHCI_PCI QSDHCI_PCI;
RADOSCB          typedef      99 block/rbd.c      } RADOSCB;
RBDAIOCB         typedef      91 block/rbd.c      } RBDAIOCB;
RING_IDX         typedef      52 include/hw/xen/interface/io/ring.h typedef unsigned int RING_IDX;
RISCVCPU         typedef     275 target/riscv/cpu.h } RISCVCPU;
RXCPU            typedef     118 target/rx/cpu.h  typedef struct RXCPU RXCPU;
SCCB             typedef      65 pc-bios/s390-ccw/sclp.h } __attribute__((packed)) SCCB;
SCCB             typedef     181 include/hw/s390x/sclp.h } QEMU_PACKED SCCB;
SCHIB            typedef     124 include/hw/s390x/ioinst.h } QEMU_PACKED SCHIB;
SCSW             typedef      28 include/hw/s390x/ioinst.h } SCSW;
SDBFIS           typedef     389 hw/ide/ahci_internal.h } QEMU_PACKED SDBFIS;
SPARCCPU         typedef      56 target/sparc/cpu-qom.h typedef struct SPARCCPU SPARCCPU;
STE              typedef     485 hw/arm/smmuv3-internal.h } STE;
SWIM             typedef      75 include/hw/block/swim.h } SWIM;
TCR              typedef     164 target/arm/cpu.h } TCR;
TPMPPI           typedef      21 hw/tpm/tpm_ppi.h } TPMPPI;
TZMPC            typedef      43 include/hw/misc/tz-mpc.h typedef struct TZMPC TZMPC;
TZMSC            typedef      77 include/hw/misc/tz-msc.h } TZMSC;
TZPPC            typedef      75 include/hw/misc/tz-ppc.h typedef struct TZPPC TZPPC;
UART             typedef      94 hw/char/grlib_apbuart.c } UART;
UHCI_QH          typedef     156 hw/usb/hcd-uhci.c } UHCI_QH;
UHCI_TD          typedef     151 hw/usb/hcd-uhci.c } UHCI_TD;
URI              typedef      77 include/qemu/uri.h } URI;
VFIOBAR          typedef      56 hw/vfio/pci.h    } VFIOBAR;
VFIOVGA          typedef      69 hw/vfio/pci.h    } VFIOVGA;
VXHSAIOCB        typedef      47 block/vxhs.c     } VXHSAIOCB;
XENCONS_RING_IDX typedef      30 include/hw/xen/interface/io/console.h typedef uint32_t XENCONS_RING_IDX;
XHCITRB          typedef     144 hw/usb/hcd-xhci.c } XHCITRB;

Re: Function-like macro with the same name as a typedef confuses Coccinelle

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Peter Maydell <peter.maydell@linaro.org> writes:
>
>> On Thu, 2 Apr 2020 at 13:06, Markus Armbruster <armbru@redhat.com> wrote:
>>>
>>> I discovered that Vladimir's auto-propagated-errp.cocci leaves
>>> hw/arm/armsse.c unchanged, even though it clearly should change it.
>>> Running spatch with --debug prints (among lots of other things)
>>
>>> Clearly, Coccinelle is getting spooked to easily.
>>
>> Is it worth asking on the coccinelle mailing list about whether
>> coccinelle could be made to be less picky in this area ?
>
> I guess we owe them the feedback.  I'll look into minimizing the
> reproducer.

https://systeme.lip6.fr/pipermail/cocci/2020-April/007097.html

Re: Function-like macro with the same name as a typedef confuses Coccinelle

[Eric Blake]

On 4/7/20 6:58 AM, Markus Armbruster wrote:
> Markus Armbruster <armbru@redhat.com> writes:
> 
>> Peter Maydell <peter.maydell@linaro.org> writes:
>>
>>> On Thu, 2 Apr 2020 at 13:06, Markus Armbruster <armbru@redhat.com> wrote:
>>>>
>>>> I discovered that Vladimir's auto-propagated-errp.cocci leaves
>>>> hw/arm/armsse.c unchanged, even though it clearly should change it.
>>>> Running spatch with --debug prints (among lots of other things)
>>>
>>>> Clearly, Coccinelle is getting spooked to easily.
>>>
>>> Is it worth asking on the coccinelle mailing list about whether
>>> coccinelle could be made to be less picky in this area ?
>>
>> I guess we owe them the feedback.  I'll look into minimizing the
>> reproducer.
> 
> https://systeme.lip6.fr/pipermail/cocci/2020-April/007097.html

Wow - firefox refused to connect to that site until I downgraded to 
permitting TLS 1.1.  We should also tell them about their configuration 
bug in not supporting TLS 1.2 for their archives.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: Function-like macro with the same name as a typedef confuses Coccinelle

[Philippe Mathieu-Daudé]

On 4/2/20 2:06 PM, Markus Armbruster wrote:
> I discovered that Vladimir's auto-propagated-errp.cocci leaves
> hw/arm/armsse.c unchanged, even though it clearly should change it.
> Running spatch with --debug prints (among lots of other things)
> 
>      let's go
>      -----------------------------------------------------------------------
>      -----------------------------------------------------------------------
>      parse error
>       = error in hw/arm/armsse.c; set verbose_parsing for more info
>      badcount: 7
>      bad: }
>      bad:
>      bad: static void nsccfg_handler(void *opaque, int n, int level)
>      bad: {
>      BAD:!!!!!     ARMSSE *s = ARMSSE(opaque);
>      bad:
>      bad:     s->nsccfg = level;
>      bad: }
>      parse error
> 
> Alright, what's wrong with this?  ARMSSE is both a function-like macro
> and a typedef:
> 
>      #define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), TYPE_ARMSSE)
> 
>      typedef struct ARMSSE {
>          ...
>      } ARMSSE;
> 
> This appears to spook Coccinelle badly enough to skip code using ARMSSE.
> 
> If I rename the macro to ARMSSE_() just for testing, it produces the
> transformation I'd expect.
> 
> Grepping for typedef names is bothersome, so I used ctags -x to build a
> cross reference table, and searched that for function-like macros
> clashing with typedefs.  Result:
> 
> include/hw/arm/armsse.h:#define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), TYPE_ARMSSE)
> include/hw/arm/armsse.h:} ARMSSE;
> include/hw/block/swim.h:#define SWIM(obj) OBJECT_CHECK(SWIM, (obj), TYPE_SWIM)
> include/hw/block/swim.h:} SWIM;
> target/rx/cpu-qom.h:#define RXCPU(obj) \
> target/rx/cpu.h:typedef struct RXCPU RXCPU;
> target/s390x/translate.c:#define BD(N, BB, BD) { BB,  4, 0, FLD_C_b##N, FLD_O_b##N }, \
> hw/audio/ac97.c:} BD;
> 
> The last one is a name clash in unrelated files; should not bother
> Coccinelle.
> 
> The first three are due to QOM.  By convention, the name of the
> function-like macro to convert to the QOM type is the QOM type in
> ALL_CAPS.  Clash when the QOM type is ALL_CAPS already.

To add to this list, another problem I'm having is with QOM interfaces.

For example this line:

   isa_bus_dma(bus, ISADMA(isa1), ISADMA(isa2));

The definitions are:

#define ISADMA(obj) INTERFACE_CHECK(IsaDma, (obj), TYPE_ISADMA)
typedef struct IsaDma IsaDma;

This is used as opaque pointer, so it compiles fine, but coccinelle is 
confused because the actual 'struct IsaDma' is never defined.

The only 'documentation' I found about this is commit 00ed3da9b5 which 
describes this as 'common practice'.

> 
> Clearly, Coccinelle is getting spooked to easily.  Regardless, three
> questions:
> 
> 1. Are ALL_CAPS typedef names a good idea?  We shout macros to tell
> readers "beware, possibly magic".  Shouting other stuff as well
> undermines that.
> 
> 2. The compiler is quite cool with us using the same name for a
> function-like macro and a not-function-like non-macro.  But is it a good
> idea?
> 
> 3. Do we want to work around Coccinelle's parsing flaw?  Having it skip
> so much code is clearly suboptimal, and potentially dangerous.  The
> obvious work-around is to rename the three problematic types so they
> contain at least one lower-case letter.

Re: Function-like macro with the same name as a typedef confuses Coccinelle

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 4/2/20 2:06 PM, Markus Armbruster wrote:
>> I discovered that Vladimir's auto-propagated-errp.cocci leaves
>> hw/arm/armsse.c unchanged, even though it clearly should change it.
>> Running spatch with --debug prints (among lots of other things)
>>
>>      let's go
>>      -----------------------------------------------------------------------
>>      -----------------------------------------------------------------------
>>      parse error
>>       = error in hw/arm/armsse.c; set verbose_parsing for more info
>>      badcount: 7
>>      bad: }
>>      bad:
>>      bad: static void nsccfg_handler(void *opaque, int n, int level)
>>      bad: {
>>      BAD:!!!!!     ARMSSE *s = ARMSSE(opaque);
>>      bad:
>>      bad:     s->nsccfg = level;
>>      bad: }
>>      parse error
>>
>> Alright, what's wrong with this?  ARMSSE is both a function-like macro
>> and a typedef:
>>
>>      #define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), TYPE_ARMSSE)
>>
>>      typedef struct ARMSSE {
>>          ...
>>      } ARMSSE;
>>
>> This appears to spook Coccinelle badly enough to skip code using ARMSSE.
>>
>> If I rename the macro to ARMSSE_() just for testing, it produces the
>> transformation I'd expect.
>>
>> Grepping for typedef names is bothersome, so I used ctags -x to build a
>> cross reference table, and searched that for function-like macros
>> clashing with typedefs.  Result:
>>
>> include/hw/arm/armsse.h:#define ARMSSE(obj) OBJECT_CHECK(ARMSSE, (obj), TYPE_ARMSSE)
>> include/hw/arm/armsse.h:} ARMSSE;
>> include/hw/block/swim.h:#define SWIM(obj) OBJECT_CHECK(SWIM, (obj), TYPE_SWIM)
>> include/hw/block/swim.h:} SWIM;
>> target/rx/cpu-qom.h:#define RXCPU(obj) \
>> target/rx/cpu.h:typedef struct RXCPU RXCPU;
>> target/s390x/translate.c:#define BD(N, BB, BD) { BB,  4, 0, FLD_C_b##N, FLD_O_b##N }, \
>> hw/audio/ac97.c:} BD;
>>
>> The last one is a name clash in unrelated files; should not bother
>> Coccinelle.
>>
>> The first three are due to QOM.  By convention, the name of the
>> function-like macro to convert to the QOM type is the QOM type in
>> ALL_CAPS.  Clash when the QOM type is ALL_CAPS already.
>
> To add to this list, another problem I'm having is with QOM interfaces.
>
> For example this line:
>
>   isa_bus_dma(bus, ISADMA(isa1), ISADMA(isa2));
>
> The definitions are:
>
> #define ISADMA(obj) INTERFACE_CHECK(IsaDma, (obj), TYPE_ISADMA)
> typedef struct IsaDma IsaDma;
>
> This is used as opaque pointer, so it compiles fine, but coccinelle is
> confused because the actual 'struct IsaDma' is never defined.

Can you give me an example where Coccinelle gets confused by it?

> The only 'documentation' I found about this is commit 00ed3da9b5 which
> describes this as 'common practice'.

We discussed this in the thread

    Subject: Issues around TYPE_INTERFACE
    To: qemu-devel@nongnu.org
    Date: Tue, 12 Mar 2019 11:50:54 +0100
    Message-ID: <87h8c82woh.fsf@dusky.pond.sub.org>

Perhaps start with Paolo's reply:

    Message-ID: <fdaa779c-ed79-647b-6038-3df2353fe502@redhat.com>
    https://lists.nongnu.org/archive/html/qemu-devel/2019-04/msg00749.html

I just suggested to have a future docs/devel/qom.rst cover the topic:

    Subject: Re: [PATCH v4 2/3] mac_via: fix incorrect creation of mos6522 device in mac_via
    Message-ID: <87369m3t18.fsf@dusky.pond.sub.org>