[PATCH libX11 2/2] xcb_io: Add comment explaining a mixed type double assignment

[Jonas Petersen]

Am 17.11.2013 14:06, schrieb Uli Schlachter:
> Hi,
>
> On 16.11.2013 22:37, Jonas Petersen wrote:
>> @@ -83,6 +83,10 @@ static void require_socket(Display *dpy)
>>   						 "did not own the socket",
>>   			                         xcb_xlib_seq_number_wrapped);
>>   		}
>> +		/* The following line will truncate the 64-bit 'sent'
>> +		 * to 32-bit when assigning it to 'dpy->request'. The
>> +		 * truncated value will then be assigned to the 64-bit
>> +		 * 'dpy->xcb->last_flushed' (which is intended). */
>>   		dpy->xcb->last_flushed = dpy->request = sent;
>>   		dpy->bufmax = dpy->xcb->real_bufmax;
>>   	}
> The involved types are:
>
>     uint64_t sent;
>     uint64_t last_flushed;
>     unsigned long request;
>
> So request isn't really a 32-bit type. In fact, on my system, all of these three
> are the same type. So the comment doesn't really fit.

I agree. The comment only applies to systems with 32-bit longs (like 
mine). Maybe the comment should start out with something like "On 
systems with 32-bit unsigned longs the following line...".

> Now let's assume that unsigned long is a 32-bit integer. *Why* is it intended to
> truncate last_flushed? This means that Xlib uses a 64 bit integer for to
> tracking the sequence number, but only actually tracks it with 32 bits. This
> seems wasteful. Why are you sure that this is intended?

Well, because when the 32-bit request wraps (which it may), then 
last_flushed must reflect the wrapped values because they get refered to 
each other (which fails in the moment when the wrap happens, which then 
results in the crash).

Apart from that I was writing "intended" because it wouldn't work 
otherwise. I was hitting this at first when debugging the issue. I 
thought: damn it's truncating the request, this must be the bug. But it 
wasn't.

Maybe it wasn't strictly speaking "intended" in first place and it 
simply works by accident. Which would be on more reason to put some 
comment clarifying the matter. Separating the assignments might be 
another measure.

I'm glad for any enlightenment in case I'm totally wrong with all of my 
theories.

- Jonas