> Date: Mon, 11 Oct 2010 10:39:56 +0200 > From: Takashi Iwai <[email protected]> > > At Mon, 11 Oct 2010 10:24:44 +0200 (CEST), > Mark Kettenis wrote: > > > > > Date: Mon, 11 Oct 2010 09:47:56 +0200 > > > From: Takashi Iwai <[email protected]> > > > > > > At Fri, 08 Oct 2010 23:09:26 +0200, > > > Henrik Rydberg wrote: > > > > > > > > On 10/08/2010 10:51 PM, Mark Kettenis wrote: > > > > > > > > >> Date: Fri, 08 Oct 2010 21:36:02 +0200 > > > > >> From: Takashi Iwai <[email protected]> > > > > >> > > > > >> At Fri, 8 Oct 2010 20:48:10 +0200 (CEST), > > > > >> Mark Kettenis wrote: > > > > >>> > > > > >>>> From: Takashi Iwai <[email protected]> > > > > >>>> Date: Fri, 8 Oct 2010 19:22:29 +0200 > > > > >>> > > > > >>> Sorry, but it isn't obvious to me what issue this fixes. > > > > >> > > > > >> In C, "1" is an integer, not an unsigned long. > > > > >> Thus (1 << 33) doesn't give you the 33th bit shift, but it's > > > > >> undefined. > > > > > > > > > > But if you'd actually use 33 (or event 32) as an offset, things > > > > > wouldn't work on a 32-bit platform would they? > > > > > > > > > > > > I believe this is what the patch is supposed to fix. > > > > > > Yep :) > > > > > > > > > > > Anyway, > > > > > > > > > >> If any, this must be (1UL << 32). > > > > > > > > > > This is the idiom that is much more common. I probably wouldn't have > > > > > questioned things if you'd written it like that. I recommend you to > > > > > stick to this version. > > > > > > > > > > > > For a counter-example, see the definition of test_bit() in the Linux > > > > kernel. > > > > > > > > >> Also, it'd be better if such a test macro returns 1 instead of a > > > > >> random non-zero value. So does my patch. > > > > > > > > > > In C this doesn't matter. > > > > > > > > > > > > It is still useful to know if a logic expression evaluates to (0, 1) or > > > > (0, > > > > nonzero). > > > > > > It does matter if the returned type is bigger. > > > Try an example code below on 32bit and 64bit machines: > > > > > > ================================================================ > > > #include <stdio.h> > > > > > > #define LONG_BITS (sizeof(long) * 8) > > > #define OFF(x) ((x) % LONG_BITS) > > > #define LONG(x) ((x) / LONG_BITS) > > > > > > #define TEST_BIT1(bit, array) (array[LONG(bit)] & (1 << OFF(bit))) > > > #define TEST_BIT2(bit, array) (array[LONG(bit)] & (1UL << OFF(bit))) > > > #define TEST_BIT3(bit, array) ((array[LONG(bit)] >> OFF(bit)) & 1) > > > > > > static unsigned long test[4]; > > > > > > int main() > > > { > > > const int bit = 33; > > > int result; > > > > > > /* set the bit */ > > > test[LONG(bit)] |= 1UL << OFF(bit); > > > > > > result = TEST_BIT1(bit, test); > > > printf("TEST1 %d = %d\n", bit, result); > > > > > > result = TEST_BIT2(bit, test); > > > printf("TEST2 %d = %d\n", bit, result); > > > > > > result = TEST_BIT3(bit, test); > > > printf("TEST3 %d = %d\n", bit, result); > > > > > > return 0; > > > } > > > ================================================================ > > > > > > On a 32bit machine, it results in: > > > TEST1 33 = 2 > > > TEST2 33 = 2 > > > TEST3 33 = 1 > > > > > > while on a 64bit machine, it results in: > > > TEST1 33 = 0 > > > TEST2 33 = 0 > > > TEST3 33 = 1 > > > > > > See TEST2 still failed although it uses 1UL. > > > > This example is meaningless. On every reasonable 32-bit architecture, > > long is a 32-bit integer. So > > > > > /* set the bit */ > > > test[LONG(bit)] |= 1UL << OFF(bit); > > > > when bit is 33 (or even 32), is completely undefined on a 32-bit > > machine. Therefore, you should never call TEST_BIT() with a first > > argument of 32 or larger, even on a 64-bit architecture, since the > > code wouldn't work on a 32-bit machine. > > Hm? TEST_BIT() is used for an long *array*, with the unlimited size. > Otherwise the macro should have been more simplified :) > Ditto for the above code. It stores the value in an array member. > That's why LONG() and OFF() macros are used there.
Hmm, ok that code is seriously weird. Guess it dates from the time that people still cared about machines with 16-bit integers. Sorry for the noise. _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
