Marc-André Lureau <marcandre.lur...@gmail.com> writes: > Hi > > On Tue, Sep 5, 2017 at 2:56 PM Markus Armbruster <arm...@redhat.com> wrote: > >> Marc-André Lureau <marcandre.lur...@gmail.com> writes: >> >> > Hi, >> > >> > I have a series of changes generated with clang-tidy qemu [1] pending >> > for review [2]. >> > >> > It translates calloc/*malloc*/*realloc() calls to >> > g_new/g_newa/g_new0/g_renew() where the argument is a sizeof(T) [* N]. >> >> Only for *type* T, or for arbitrary T? >> >> > If sizeof() argument is a type: > https://github.com/elmarco/clang-tools-extra/blob/master/clang-tidy/qemu/UseGnewCheck.cpp#L65 > > For type T, we've done the transformation repeatedly with Coccinelle, >> first in commit b45c03f (commit message appended). Repeatedly, because >> they creep back in. >> >> How does your technique compare to this prior one? >> > > Well, for one, I have a lot of trouble running coccinelle, mostly because > it is slow and/or eat all my memory.
It's a bit of a pig, but it doesn't get anywhere close to prohibitive even on my rather modest development box. Example run with a minor variation of the semantic patch from commit b45c03f: $ time spatch --in-place --sp-file g_new.cocci --macro-file scripts/cocci-macro-file.h --use-idutils --dir . [...] real 0m42.379s user 0m38.988s sys 0m2.179s $ git-diff --shortstat 123 files changed, 211 insertions(+), 216 deletions(-) Note that "--use-idutils --dir ." directs spatch to work only on files that need work. There's also --use-glimpse. You can easily do the directing by hand with a judicious `git-grep ...`. Your experience can be far worse if you feed it source files indiscriminately. > Afaik, coccinelle also has trouble > parsing the code in various cases. Yes, but anything else that doesn't is almost certainly not tackling the full problem. Preprocessing C is easy enough. Parsing preprocessed C is easy enough. Parsing unpreprocessed C into something that's suitable for transforming is darn hard. Doesn't mean that clang-tidy couldn't be doing a better job for our code. > I have also trouble writing coccinelle > semantic patch... Point taken. > And, I gave up. I've found it frustrating at times, but overall too useful to pass up. I'd love to buy a well-written book on the thing. > clang-tidy in comparison, is quite fast (it takes a minute to apply on qemu Same order of magnitude as my spatch run above. > code base). I find it easier to write a tidy check, and more friendly > command line / output etc: > > /tmp/test.c:3:17: warning: use g_new() instead [qemu-use-gnew] > int *f = (int*)malloc(sizeof(int) * 2); > ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~ > g_new(int, 2) > > clang-tidy should also respect clang-format rules when modifying the code > (see also > https://lists.nongnu.org/archive/html/qemu-devel/2017-08/msg05762.html). > > I also imagine it should be possible to integrate to clang warnings in the > future (similar to libreoffice clang plugins for ex). All in all, > coccinelle or clang-tidy are probably both capable of doing this job, but > clang-tidy is snappier and has more potentials for toolchain integration. Suggest you show us cool things you can do with clang-tidy that haven't been done with Coccinelle :) [...]
