Date:	Fri, 4 Apr 2008 10:19:49 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Harvey Harrison <harvey.harrison@...il.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	LKML <linux-kernel@...r.kernel.org>, josh@...edesktop.org
Subject: Re: [PATCH 7/7] asm-generic: suppress sparse warning in ioctl.h

On Thu, Apr 03, 2008 at 03:55:46AM +0100, Al Viro wrote:
> On Wed, Apr 02, 2008 at 07:51:08PM -0700, Linus Torvalds wrote:
> > 
> > 
> > On Thu, 3 Apr 2008, Al Viro wrote:
> > > 
> > > Um...  But that's _not_ a link-time constant - simply an int variable not
> > > defined in any object file...
> > 
> > Ahh, you're right, my bad. We could change it to take the address of it, 
> > though. I assume gcc is happy with that too?
> 
> gcc might be, but what are you going to do with it?  Expression is an
> integer one, after all - in cases that had triggered that thread it
> was used (after & 0xff) as array index.
> 
> Besides, logically it's _not_ a constant at all - not even a base + constant,
> after using it in such expression...

OK, I think I see how to do it.
	* new bit in ->flags - Weak_Int_Const
	* parser sets it by almost the same rules as Int_Const_Expr, except
that ?:, || and && simply inherit it from the first argument.
	* evaluate strips it in the same cases as Int_Const_Expr, except that
recalculation for ?:, || and && ignores everything except the first argument.
	* when expand sees VALUE && VALUE or VALUE || VALUE and the second
argument is not to be ignored, Weak_Int_Const on node is removed unless the
second argument also had it.
	* when expand sees VALUE ? : and overwrites it with appropriate
branch, have Weak_Int_Const on result iff both the original node and
overwriting argument used to have it.
	* bad_integer_constant_expression() checks either Int_Const_Expr or
Weak_Int_Const, depending on flag set by -W<something>

Should work, AFAICS...  If expression has no Weak_Int_Const out of parser,
it's definitely not an integer constant expression, even with lax rules.
If expression loses Weak_Int_Const on evaluate, we must have a cast to
something other than integer type in it => not an integer constant expression.
If expression loses Weak_Int_Const on expand, we must have found a
subexpression that _might_ be ignored, had not been an integer constant
expression by weak rules and actually was not ignored => not an integer
constant expression.

In no case we could gain Weak_Int_Const and if expand ended up with
EXPR_VALUE and no commas in evaluated subexpressions, having Weak_Int_Const
is equivalent to what we want.

Handling of expr->flags might get a bit clumsy, but other than that it seems
to be easy enough...  Comments?

One thing: behaviour of -Wall is getting more and more unpleasant.  On
kernel builds sparse gets hit with it due to CFLAGS and that kills all
defaults for sparse options.  That -Wall is there for gcc; having it make
sparse to lose all defaults is wrong, especially since gcc does *not*
turn each warning on on that.

For this flag it gets particulary nasty - we have two behaviours (strict
C99 and relaxed one) and the former warns on the things that are OK by
the latter.  So the natural thing to do would be to have -W<something>
to trigger the former, right?  Now think what we'll get on the kernel
build and recall what had started this thread...
--
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/

Powered by blists - more mailing lists