linux-kernel - Re: [patch] trace: cleanup: make some types unsigned

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20111007202004.GI18470@longonot.mountain>
Date:	Fri, 7 Oct 2011 23:20:04 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [patch] trace: cleanup: make some types unsigned

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/