| Message ID | 1384637846-5343-3-git-send-email-jnsptrsn1@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
diff --git a/src/xcb_io.c b/src/xcb_io.c index f2978d0..acb1e3b 100644 --- a/src/xcb_io.c +++ b/src/xcb_io.c @@ -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; }
Hi, On 16.11.2013 22:37, Jonas Petersen wrote: > The assignment might be confusing at first. So I added a note. > > Signed-off-by: Jonas Petersen <jnsptrsn1@gmail.com> > --- > src/xcb_io.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/src/xcb_io.c b/src/xcb_io.c > index f2978d0..acb1e3b 100644 > --- a/src/xcb_io.c > +++ b/src/xcb_io.c > @@ -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. 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? Disclaimer: I don't know too much about Xlib internals and were not involved in writing xcb_io.c Cheers, Uli
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
The assignment might be confusing at first. So I added a note. Signed-off-by: Jonas Petersen <jnsptrsn1@gmail.com> --- src/xcb_io.c | 4 ++++ 1 file changed, 4 insertions(+)