qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] coccinelle: new inplace-byteswaps.cocci to remo


From: Philippe Mathieu-Daudé
Subject: Re: [Qemu-devel] [PATCH] coccinelle: new inplace-byteswaps.cocci to remove inplace-byteswapping calls
Date: Wed, 10 Oct 2018 11:01:30 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0

On 09/10/2018 20:35, Peter Maydell wrote:
> On 9 October 2018 at 19:23, Eric Blake <address@hidden> wrote:
>> On 10/9/18 1:16 PM, Peter Maydell wrote:
>>>
>>> Add a new Coccinelle script which replaces uses of the inplace
>>> byteswapping functions *_to_cpus() and cpu_to_*s() with their
>>> not-in-place equivalents. This is useful for where the swapping
>>> is done on members of a packed struct -- taking the address
>>> of the member to pass it to an inplace function is undefined
>>> behaviour in C.
>>>
>>> Signed-off-by: Peter Maydell <address@hidden>
>>> ---
>>> Richard asked for a coccinelle script in the scripts/coccinelle
>>> directory, so here's a patch to add it.
>>>
>>>   scripts/coccinelle/inplace-byteswaps.cocci | 65 ++++++++++++++++++++++
>>>   1 file changed, 65 insertions(+)
>>>   create mode 100644 scripts/coccinelle/inplace-byteswaps.cocci
>>>
>>> diff --git a/scripts/coccinelle/inplace-byteswaps.cocci
>>> b/scripts/coccinelle/inplace-byteswaps.cocci
>>> new file mode 100644
>>> index 00000000000..a869a90cbfd
>>> --- /dev/null
>>> +++ b/scripts/coccinelle/inplace-byteswaps.cocci
>>> @@ -0,0 +1,65 @@
>>> +// Replace uses of in-place byteswapping functions with calls to the
>>> +// equivalent not-in-place functions.  This is necessary to avoid
>>> +// undefined behaviour if the expression being swapped is a field in a
>>> +// packed struct.
>>> +
>>> +@@
>>> +expression E;
>>> +@@
>>> +-be16_to_cpus(&E);
>>> ++E = be16_to_cpu(E);
>>> +@@
>>> +expression E;
>>> +@@
>>> +-be32_to_cpus(&E);
>>> ++E = be32_to_cpu(E);
>>
>>
>> It's probably possible to shorten this as:
> 
> This is kind of why I didn't post it as a patch to go in
> scripts/ initially :-)  I suspected it was possible to
> shorten it (maybe there's even coccinelle syntax to allow the
> function names to be substituted to be specified with
> a regex?), but the dumb-and-simple cut-and-paste for every
> function being replaced worked and it didn't seem worth
> spending even ten minutes messing around trying to improve
> what is fundamentally a throwaway script...

Julia Lawall once [*] said:

Even for 6 functions, I would suggest to write out the function names in
the pattern matching code rather than using regular expressions.  If the
names are explicit, then Coccinelle can do some filtering, either based
on an index made with idutils or glimpse (see the coccinelle scripts
directory for tools for making these indices) or based on grep, to drop
without parsing files that are not relevant.  It doesn't do this for
regular expressions.  So you will get a faster result if the names are
explicit in the pattern code.

[*] https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03004.html



reply via email to

[Prev in Thread] Current Thread [Next in Thread]