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