Am 17.11.2013 11:58, schrieb Uli Schlachter:
On 16.11.2013 22:37, Jonas Petersen wrote:
+ /* Set bit 8 of 'request' when a 32-bit wrap has just happened
+ * so the sequence stays correct relatively to 'last_flushed'. */
"1 << 32" does not set bit 8 and this doesn't set anything in request, really.
You're right, sorry, "bit 8" is a typo, it's bit 32 of course. Other
than that it will indeed set something. Not exactly "in request", but it
will end up in unwrapped_request.
+ unwrapped_request = ((uint64_t)(dpy->request < dpy->xcb->last_flushed) <<
32) + dpy->request;
Also, I don't think that this code is intuitive this way.
I agree somewhat on that. I thought efficiency would have precedence
here. I was actually inspired by static void widen() in xcb_io.c where
it reads:
*wide = new + ((unsigned long) (new < *wide) << 16 << 16);
The comment says "Treating the comparison as a 1 and shifting it avoids
a conditional branch".
I would propose this instead:
unwrapped_request = dpy->request;
/* If there was a sequence number wrap since our last flush, make sure we
* use the correct 64 bit sequence number */
if (sizeof(uint64_t) > sizeof(unsigned long)
&& dpy->request < dpy->xcb_last_flushed)
unwrapped_request += UINT64_C(1) << 32;
(I am not sure/convinced if the sizeof comparision is necessary, but I saw
something like this in require_socket() and then thought that this might be
necessary on systems where unsigned long already is a 64 bit type.)
I admit I haven't tought it all through on other system configurations.
So thanks for giving your input.
I guess the sizeof comparison would not be necessary since the condition
should never meet with 64-bit longs. And if it does, something is wrong
anyway (from my understanding). After all this is the trigger of the bug.
Btw. as you might have guessed, on my system unsigned longs are 32-bit.
- Jonas
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
