On 21/04/15 17:07, Hartley Sweeten wrote:
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?
Yes, but 1U is cleaner.
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Web: http://www.mev.co.uk/ )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
