[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argumen
From: |
Peter Maydell |
Subject: |
Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument |
Date: |
Tue, 18 Feb 2020 13:41:59 +0000 |
On Tue, 18 Feb 2020 at 13:33, Eric Blake <address@hidden> wrote:
>
> On 2/18/20 6:56 AM, Philippe Mathieu-Daudé wrote:
>
> >> +++ b/scripts/coccinelle/as_rw_const.cocci
> >> @@ -0,0 +1,30 @@
> >> +// Avoid uses of address_space_rw() with a constant is_write argument.
> >> +// Usage:
> >> +// spatch --sp-file as-rw-const.spatch --dir . --in-place
> >
> > Nitpick, script is now scripts/coccinelle/as_rw_const.cocci.
> >
> > Reviewed-by: Philippe Mathieu-Daudé <address@hidden>
> >
> >> +
> >> +@@
> >> +expression E1, E2, E3, E4, E5;
> >> +symbol false;
> >> +@@
> >> +
> >> +- address_space_rw(E1, E2, E3, E4, E5, false)
> >> ++ address_space_read(E1, E2, E3, E4, E5)
> >> +@@
> >> +expression E1, E2, E3, E4, E5;
> >> +@@
> >> +
> >> +- address_space_rw(E1, E2, E3, E4, E5, 0)
> >> ++ address_space_read(E1, E2, E3, E4, E5)
>
> This feels a bit redundant. Doesn't coccinelle have enough smarts about
> isomorphisms (such as 0 == false, 1 == true) that it could get by with
> one @@ hunk instead of 2, if we come up with the right way to represent
> any isomorphism to a constant value? But admittedly, I don't know what
> that representation would actually be, and your verbose patch works even
> if it is not the most concise possible. So don't let my remarks hold
> this patch up.
My experience with Coccinelle has generally been that trying
to make semantic patches smaller and less redundant is futile
and a massive timesink. In this case as far as I can tell
Coccinelle has no idea at all about the existence of the 'bool'
type and that 'true' and 'false' are equivalent to 1 and 0.
Thus the 'symbol' declaration, otherwise it thinks that 'false'
is a random semantic identifier and doesn't look for a literal
match of it.
thanks
-- PMM
- [PATCH v2] Avoid address_space_rw() with a constant is_write argument, Peter Maydell, 2020/02/18
- Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument, Philippe Mathieu-Daudé, 2020/02/18
- Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument, Laurent Vivier, 2020/02/18
- Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument, Cédric Le Goater, 2020/02/18
- Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument, Christian Borntraeger, 2020/02/18
- Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument, Cornelia Huck, 2020/02/18
- Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument, Alistair Francis, 2020/02/18
- Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument, David Gibson, 2020/02/18
- Re: [PATCH v2] Avoid address_space_rw() with a constant is_write argument, Edgar E. Iglesias, 2020/02/18