qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: Markus Armbruster <armbru@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	QEMU Developers <qemu-devel@nongnu.org>
Subject: Re: Function-like macro with the same name as a typedef confuses Coccinelle
Date: Thu, 2 Apr 2020 13:21:29 +0100	[thread overview]
Message-ID: <CAFEAcA8aBjWPVH7VsicTrKce1K-sOh0Sv+Ok-75zbtsJV=OBaA@mail.gmail.com> (raw)
In-Reply-To: <87k12y5by1.fsf@dusky.pond.sub.org>

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


  reply	other threads:[~2020-04-02 12:22 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-02 12:06 Function-like macro with the same name as a typedef confuses Coccinelle Markus Armbruster
2020-04-02 12:21 ` Peter Maydell [this message]
2020-04-02 13:30   ` Markus Armbruster
2020-04-07 11:58     ` Markus Armbruster
2020-04-07 13:47       ` Eric Blake
2020-04-02 12:29 ` Philippe Mathieu-Daudé
2020-04-02 13:47   ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFEAcA8aBjWPVH7VsicTrKce1K-sOh0Sv+Ok-75zbtsJV=OBaA@mail.gmail.com' \
    --to=peter.maydell@linaro.org \
    --cc=armbru@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).