tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Using coccinelle for (quick?) syntax fixing



On Mon Aug 09 2010 at 11:20:29 +0200, Jean-Yves Migeon wrote:
> It is 'error-prone", in the sense that it can raise false positives. But
> when you get more familiar with it, you can either fix the cocci patch
> (easy for __arraycount, I missed one of the cases... less obvious for
> aprint stuff), and proof read the generated patch.

I really dislike untested wide-angle churn, especially if there is 0
measurable gain.  Converting code to __arraycount is a prime example.
The only benefit of __arraycount is avoiding typing and therefore typos.
Neither of those apply when doing a churn.
(there are subjective beauty values, but every C programmer knows
the sizeof/sizeof idiom, which is more than what can be said about
__arraycount)

Examples of measurable benefit are good.  Encouraging churn is less good,
even if spatch-churn is a million times better than sed-churn.

> I used these examples to get familiar with it; it starts getting useful
> when you try to find out buggy code, like double free() in the same
> function, mutex_exit() missing in a branch before returning, etc.

Static analysis is good.  However, it might take quite a bit of effort
to get the rules general enough so that they trigger in more than one
file and specific enough so that you don't get too many false positives.
Just to give an example, the ffs allocator routines don't release the
lock in an error branch.

I remember coccinelle had problems with cpp.  Any code which uses macros
to "skip" C syntax will fail silently.  procfs comes to mind here.  Also,
I remember it using so much memory when given our kernel source that I
could not finish a rototill and had to use it in combo with find/grep.

That said, if $someone can produce a set of rules which showably find
bugs in NetBSD code and do not produce a lot of false positives, I'm
very interested in seeing nightly runs.

... especially if there are no TAILQ false positives ;)


Home | Main Index | Thread Index | Old Index