On Tue, 18 Feb 2020 at 13:33, Eric Blake <ebl...@redhat.com> 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é <phi...@redhat.com> > > > >> + > >> +@@ > >> +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
