On Fri, 29 Oct 2010, Andi Kleen wrote: > On Fri, Oct 29, 2010 at 10:25:19PM +0200, Julia Lawall wrote: > > > - one problem is that larger drivers often consist out of multiple > > > files for different pattern matches (e.g. one file has the callback > > > definitio and the other the actual callback). I had to manually adjust > > > the command lines in this case and didn't find an obvious way to automate > > > this. > > > > If you give multiple file names at once, it will treat them all together, > > but you have to know which files to give. At one point, we tried to infer > > this automatically, but didn't really get that to a usable state. Perhaps > > something could be inferred from the make file. > > I originally just grepped for the entry point and used that list > as a starting point for a -j makefile that generated patches > for each file. Then I had to hand examine all the empty patches > and find out which files those really need. > > > > The advantage of this is that you parse the semantic patch only once per > > processor. > > I just used make, it parallelizes as well.
But you call spatch separately for each file, so you parse the semantic patch for each file. The semantic patch parsing proces is a little bit complicated, but I don't know how much overhead this adds in practice. > > If someone knows of a free indexing tool, then we could try to support > > that as well. Actually glimpse is not entirely satisfactory because it > > considers _ to be a word separator, leading to many false positives in > > some cases. > > GNU id-utils. I use that all the time. OK, thanks. I will look into it. > I guess if that works I could avoid the initial grep step and just > feed it all files. But then there won't be any parallelization, right, > or does Kees' script solve that somehow? The script uses the arguments -max $MAX -index $i -max is the total number of processes that will be created and -index is the index of the current call to spatch. spatch then parses the semantic patch and then runs the parsed semantic patch on 1/$MAX of the files. The index indicates which 1/$MAX to consider. If -use_glimpse is used, the set of files considered is the set returned by glimpse, not the whole set. The advantage is that ocaml and python are started up only once per process and the semantic patch is parsed only once per process. The disadvantage is that some of the 1/$MAX groups may be faster than others, and there is no way to move files between them. I don't know which approach is better. > > > - I ran into some problems with the version (0.2.2 from opensuse > > > buildservice) > > > looping forever on some files. I ended up patching these files manually > > > after killing the process after a few minutes. Maybe it needs a watchdog > > > at least? > > > > You can give a timeout expressed in a number of seconds. -timeout 120 is > > probably good enough. On the other hand, if you run into such problems, > > please let me know (C file + semantic patch). Often there is an > > inefficiency that can be corrected. > > I didn't report it because i was using a pretty old version (0.2.2) > If I see it next time I'll report it. OK, but since then I think that the only thing I have done is remove optimizations that turned out to be too aggressive, so even reports on the old version would be useful. > > > - White space management is a problem. I had to do quite some manual > > > changes to get rid of empty lines in the wrong places. Also sometimes > > > there are extra spaces when replacing identifiers (but I read there were > > > some fixes for this after 0.2.2) > > > > I understand about the empty line problem you mentioned earlier. But if > > you have other white space problems, please let me know. They seems to > > have to be fixed one by one... > > > The other problem was that I replaced a typedef with a struct foo > and then the type expressions sometimes ended up with an extra > space (like (* foo)(....)) OK, I ill try to take a look. > > > - One thing I missed was a way to express expressions without side > > > effects. > > > For example when pushing down a lock I usually want to add a unlock > > > before the return. But sometimes returns do tail calls to other functions, > > > in this case the code needs to be refactored to first assign to a > > > temporary. > > > I didn't find a straight forward way to detect this automatically and > > > ended up looking for it manually (had one case in a large change) > > > > You could do something like: > > I see. You suggest running an extra rule to detect the case. > I think it's ok for me if it just tells me about it so that > I can fix it up manually. You can use python to print out a warning message. Or you can transform a worrisome case as eg: - return E; + return PROBLEM(E); and then search for PROBLEM in the output. julia _______________________________________________ Cocci mailing list [email protected] http://lists.diku.dk/mailman/listinfo/cocci (Web access from inside DIKUs LAN only)
