On Tuesday, April 21, 2015 6:35 AM, Dan Carpenter wrote:
> On Tue, Apr 21, 2015 at 01:52:09PM +0100, Ian Abbott wrote:
>>>Is that really an improvement? The original code was actually defined.
>>>
>>>1U << 32 is actually defined. It is zero. Which works for us.
>>
>> According to the C standards it is undefined (for 32-bit unsigned
>> int). See the late draft of ISO/IEC 9899:2011 (dubbed C11) at
>> <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1570.pdf>, ยง6.5.7,
>> Semantics:
>>
>> 3. The integer promotions are performed on each of the operands. The
>> type of the result is that of the promoted left operand.If the value
>> of the right operand is negative or is greater than or equal to the
>> width of the promoted left operand, the behavior is undefined.
>>
>
> Yeah. You're right.
>
>>
>> It would be better to use (1U << b_chans) instead of (1 << b_chans)
>> in the code above, or possibly the slightly more obtuse:
>>
>> b_mask = ((b_chans < 32) ? 1 << b_chans : 0) - 1;
>
> I like just switching Hartley's 1 to 1U.
A quick test:
#include <stdio.h>
int main(int argc, char *argv[])
{
int shift;
shift = 31;
printf("(1 << %d) - 1 = %x\n", shift, (1 << shift) - 1);
printf("(1U << %d) - 1 = %x\n", shift, (1U << shift) - 1);
shift = 32;
printf("(1 << %d) - 1 = %x\n", shift, (1 << shift) - 1);
printf("(1U << %d) - 1 = %x\n", shift, (1U << shift) - 1);
return 0;
}
Produces this result:
(1 << 31) - 1 = 7fffffff
(1U << 31) - 1 = 7fffffff
(1 << 32) - 1 = 0
(1U << 32) - 1 = 0
Looks like the 1 vs 1U doesn't make any difference. And a shift of 32
results in the wrong value.
Are you ok with the original patch?
Regards,
Hartley
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
