Re: [PATCH] tmscsim: Fix big left-shifts of unsigned char

[Matthew Wilcox <matthew@wil.cx>]

On Thu, Jun 18, 2009 at 09:03:24AM -0400, James Bottomley wrote:
> On Thu, 2009-06-18 at 14:14 +0200, Roel Kluin wrote:
> > -           dc390_laststatus |= bval << 24;
> > +           dc390_laststatus |= (unsigned)bval << 24;
> 
> bval is already u8, thus unsigned, so the cast is a nop.

Err ... I believe you're pedantically uncorrect, though right in practice.

The (unsigned) cast is short for (unsigned int) -- it doesn't mean
'cast to an unsigned version of the type that it already has'.

Now, in practice what happens is:

6.5.7:

The integer promotions are performed on each of the operands. The type
of the result is that of the promoted left operand.

so bval would be promoted from an unsigned char to a signed int, and
then shifted left by 24 bits, resulting in a potentially negative number.
But dc390_laststatus is an u32, so the |= converts this negative number
into a positive one, leading to the same answer that would have been
carried out in unsigned arithmetic.

So you're right (the cast isn't needed) for the wrong reason ;-)

-- 
Matthew Wilcox				Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."