On Sun, 4 Nov 2012, Sasha Levin wrote:
On Sun, Nov 4, 2012 at 10:57 AM, Julia Lawall <julia.lawall@xxxxxxx> wrote:
On Sun, 4 Nov 2012, Sasha Levin wrote:
Hi Julia,
On Sat, Nov 3, 2012 at 4:30 PM, Julia Lawall <Julia.Lawall@xxxxxxx> wrote:
These patches convert a conditional with a simple test expression and a
then branch that only calls WARN_ON(1) to just a call to WARN_ON, which
will test the condition.
// <smpl>
@@
expression e;
@@
(
if(<+...e(...)...+>) WARN_ON(1);
|
- if (e) WARN_ON(1);
+ WARN_ON(e);
)// </smpl>
So this deals with WARN_ON(), are you considering doing the same for
the rest of it's friends?
I tried WARN_ON_ONCE, but the pattern never occurred. Are there others that
are worth trying?
Definitely!
Here's the semantic patch I've got:
@@
expression e;
@@
(
- if (e) WARN_ON(1);
+ WARN_ON(e);
|
- if (e) WARN_ON_ONCE(1);
+ WARN_ON_ONCE(e);
|
- if (e) WARN_ON_SMP(1);
+ WARN_ON_SMP(e);
|
- if (e) BUG();
+ BUG_ON(e);
)
This gave me a really huge patch output.
I can send it out if you think the patch above looks good.
I didn't change any cases where the if test contains a function call. The
current definitions of WARN_ON seem to always evaluate the condition
expression, but I was worried that that might not always be the case. And
calling a function (the ones I remember were some kinds of print
functions) seems like something one might not want buried in the argument
of a debugging macro.
WARN_ON_SMP is just WARN_ON if CONFIG_SMP is true, but it is just 0
otherwise. So in that case it seems important to check that one is not
throwing away something important.
I remember working on the BUG_ON case several years ago, and other people
worked on it too, but I guess some are still there... The current
definitions of BUG_ON seem to keep the condition, but there are quite a
few specialized definitions, so someone at some point might make a version
that does not have that property.
julia
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
