Re: [patch] trace: cleanup: make some types unsigned

[Dan Carpenter <dan.carpenter@oracle.com>]

On Fri, Oct 07, 2011 at 09:38:51AM -0400, Steven Rostedt wrote:
> On Fri, 2011-10-07 at 16:27 +0300, Dan Carpenter wrote:
> > The problem here is that I'm trying to silence a static checker
> > warning.  In replace_preds() we cap n_preds at MAX_FILTER_PRED but
> > we don't check for negative values.  It can't actually be negative
> > values, but the static checkers get confused.
> 
> I really hate to uglify code for the sake of static checkers.
> 
> This code may change in the near future, and the possibility that
> n_preds may become a possibility. Perhaps we should add a:
> 
> WARN_ON(n_preds < 0);
> 
> If in the future the count_preds() changes and incorrectly produces a
> negative number, or perhaps even overflows int, we wont catch it with
> unsigned.

I've sent a couple type changes to silence static checker warnings,
but I haven't been pushing it, because I'm interested to see what
people think about them first.  I didn't think unsigned int was
particularly ugly, but now that you point it out I guess it is
needlessly pedantic and longer to type.  So it's fine if you ignore
the patch.

Please don't add the WARN_ON().  WARN_ON()s are uglier than unsigned
ints.  WARN_ON() don't solve any problems, they just make debugging
the crash easier.  Are we going to crash here, and if so, do we
expect that debugging it will be difficult?  Probably not.

In theory, static checkers should be able to look at this code and
know that n_preds can't overflow.  So yeah.  Let's call this a
static checker bug and move on.

regards,
dan carpenter