On 06/18/2009 04:16 PM, Matthew Wilcox wrote: > 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. > Note the left operand, not the lvalue. > 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. > It could get dangerous in 64bit integer machines if the negative number gets truncated to 32bit while converted. So if the bval was > 127 and we get a 64bit signed negative number what will happen then? > So you're right (the cast isn't needed) for the wrong reason ;-) > Are you sure. It looks we need the cast Boaz -- 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
