Messages in this thread | | | Date | Thu, 18 Feb 2021 11:17:02 +0100 (CET) | From | Julia Lawall <> | Subject | Re: [PATCH] coccinelle: misc: add swap script |
| |
On Thu, 18 Feb 2021, Denis Efremov wrote:
> > > On 2/18/21 12:31 AM, Julia Lawall wrote: > >> +@depends on patch@ > >> +identifier tmp; > >> +expression a, b; > >> +type T; > >> +@@ > >> + > >> +( > >> +- T tmp; > >> +| > >> +- T tmp = 0; > >> +| > >> +- T *tmp = NULL; > >> +) > >> +... when != tmp > >> +- tmp = a; > >> +- a = b; > >> +- b = tmp; > >> ++ swap(a, b); > >> +... when != tmp > > > > In this rule and the next one, if you remove the final ; from the b = tmp > > line and from the swap line, and put it into context code afterwards, them > > the generated code looks better on cases like fs/xfs/xfs_inode.c in the > > function xfs_lock_two_inodes where two successive swap calls are > > generated. > > > > There are also some cases such as drivers/net/wireless/ath/ath5k/phy.c in > > the function ath5k_hw_get_median_noise_floor where the swap code makes up > > a whole if branch. > > > In this cases it would be good to remove the {}. > > How this can be handled? > > If I use this pattern: > @depends on patch@ > identifier tmp; > expression a, b; > @@ > > ( > if (...) > - { > - tmp = a; > - a = b; > - b = tmp > + swap(a, b) > ; > - } > | > - tmp = a; > - a = b; > - b = tmp > + swap(a, b) > ; > ) > > The tool fails with error: > > EXN: Failure("rule starting on line 58: already tagged token:\nC code > context\nFile \"drivers/net/wireless/ath/ath5k/phy.c\", line 1574, > column 4, charpos = 41650\n around = 'sort',\n whole content = > \t\t\t\tsort[j - 1] = tmp;") in drivers/net/wireless/ath/ath5k/phy.c
A disjunction basically says "at this node in the cfg, can I match the first patter, or can I match the second pattern, etc." Unfortunately in this case the two branches start matching at different nodes, so the short circuit aspect of a disjunction isn't used, and it matches both patterns.
The solution is to just make two rules. The first for the if case and the second for everything else.
julia
| |