On Fri, May 13, 2011 at 12:03:54PM +0200, Daniel Stone wrote: > This patch adds more friendly error messages for three common classes of > assertion: > - missed sequence numbers due to being griefed by another thread > - unknown requests in queue due to being griefed by another thread > - extensions dequeuing too much or too little reply data > > It adds error messages offering advice (e.g. call XInitThreads() first) > on stderr, but still generates actual assertions. Hopefully this means > it's a little more Googleable and a little less frightening. > > Signed-off-by: Daniel Stone <[email protected]> > ---
xcb_io.c seems to use tabs for indenting, your code uses spaces. might want to check that up first. maybe it's better to use a macro for the "broken X extension library" error as well instead of copy/paste. that said, still Reviewed-by: Peter Hutterer <[email protected]> > src/xcb_io.c | 105 > +++++++++++++++++++++++++++++++++++++++++++++++++--------- > 1 files changed, 89 insertions(+), 16 deletions(-) > > diff --git a/src/xcb_io.c b/src/xcb_io.c > index 8930736..4dadc88 100644 > --- a/src/xcb_io.c > +++ b/src/xcb_io.c > @@ -15,6 +15,7 @@ > #ifdef HAVE_INTTYPES_H > #include <inttypes.h> > #endif > +#include <stdio.h> > #include <stdint.h> > #include <stdlib.h> > #include <string.h> > @@ -22,6 +23,16 @@ > #include <sys/select.h> > #endif > > +#define throw_thread_fail_assert(_message, _var) { \ > + unsigned int _var = 1; \ > + fprintf(stderr, "[xcb] " _message "\n"); \ > + fprintf(stderr, \ > + "Most likely this is a multi-threaded client and XInitThreads " \ > + "has not been called\n"); \ "This error occurs when multi-threaded clients did not call XInitThreads"? > + fprintf(stderr, "Aborting, sorry about that.\n"); \ but honestly, are we _really_ sorry about it? :) Cheers, Peter > + assert(!_var); \ > +} > + > static void return_socket(void *closure) > { > Display *dpy = closure; > @@ -51,9 +62,14 @@ static void require_socket(Display *dpy) > * happens while Xlib does not own the socket. A > * complete fix would be to make XCB's public API use > * 64-bit sequence numbers. */ > - assert(!(sizeof(unsigned long) > sizeof(unsigned int) > - && dpy->xcb->event_owner == XlibOwnsEventQueue > - && (sent - dpy->last_request_read >= (UINT64_C(1) << > 32)))); > + if ((sizeof(unsigned long) > sizeof(unsigned int) && > + dpy->xcb->event_owner == XlibOwnsEventQueue && > + (sent - dpy->last_request_read >= (UINT64_C(1) << > 32)))) { > + throw_thread_fail_assert("Sequence number wrapped beyond > " > + "32 bits while Xlib did not own > " > + "the socket", > + xcb_xlib_seq_number_wrapped); > + } > dpy->xcb->last_flushed = dpy->request = sent; > dpy->bufmax = dpy->xcb->real_bufmax; > } > @@ -125,9 +141,19 @@ static PendingRequest *append_pending_request(Display > *dpy, unsigned long sequen > node->reply_waiter = 0; > if(dpy->xcb->pending_requests_tail) > { > - > assert(XLIB_SEQUENCE_COMPARE(dpy->xcb->pending_requests_tail->sequence, <, > node->sequence)); > - assert(dpy->xcb->pending_requests_tail->next == NULL); > - dpy->xcb->pending_requests_tail->next = node; > + if > (XLIB_SEQUENCE_COMPARE(dpy->xcb->pending_requests_tail->sequence, > + >=, node->sequence)) > + { > + throw_thread_fail_assert("Unknown sequence number while " > + "appending request", > + xcb_xlib_seq_number_wrapped); > + } > + if (dpy->xcb->pending_requests_tail->next != NULL) { > + throw_thread_fail_assert("Unknown request in queue while " > + "appending request", > + xcb_xlib_seq_number_wrapped); > + } > + dpy->xcb->pending_requests_tail->next = node; > } > else > dpy->xcb->pending_requests = node; > @@ -137,15 +163,30 @@ static PendingRequest *append_pending_request(Display > *dpy, unsigned long sequen > > static void dequeue_pending_request(Display *dpy, PendingRequest *req) > { > - assert(req == dpy->xcb->pending_requests); > + if (req != dpy->xcb->pending_requests) > + { > + throw_thread_fail_assert("Unknown request in queue while " > + "dequeuing", > + xcb_xlib_seq_number_wrapped); > + } > dpy->xcb->pending_requests = req->next; > if(!dpy->xcb->pending_requests) > { > - assert(req == dpy->xcb->pending_requests_tail); > + if (req != dpy->xcb->pending_requests_tail) > + { > + throw_thread_fail_assert("Unknown request in queue while > " > + "dequeuing", > + xcb_xlib_seq_number_wrapped); > + } > dpy->xcb->pending_requests_tail = NULL; > } > - else > - assert(XLIB_SEQUENCE_COMPARE(req->sequence, <, > dpy->xcb->pending_requests->sequence)); > + else if (XLIB_SEQUENCE_COMPARE(req->sequence, >=, > + dpy->xcb->pending_requests->sequence)) > + { > + throw_thread_fail_assert("Unknown sequence number while " > + "dequeuing request", > + xcb_xlib_threads_sequence_lost); > + } > free(req); > } > > @@ -218,7 +259,13 @@ static xcb_generic_reply_t *poll_for_event(Display *dpy) > if(!req || XLIB_SEQUENCE_COMPARE(event_sequence, <, > req->sequence) > || (event->response_type != X_Error && event_sequence > == req->sequence)) > { > - assert(XLIB_SEQUENCE_COMPARE(event_sequence, <=, > dpy->request)); > + if (XLIB_SEQUENCE_COMPARE(event_sequence, >, > + dpy->request)) > + { > + throw_thread_fail_assert("Unknown sequence > number " > + "while processing > queue", > + > xcb_xlib_threads_sequence_lost); > + } > dpy->last_request_read = event_sequence; > dpy->xcb->next_event = NULL; > return (xcb_generic_reply_t *) event; > @@ -237,7 +284,12 @@ static xcb_generic_reply_t *poll_for_response(Display > *dpy) > !req->reply_waiter && > xcb_poll_for_reply(dpy->xcb->connection, req->sequence, > &response, &error)) > { > - assert(XLIB_SEQUENCE_COMPARE(req->sequence, <=, dpy->request)); > + if(XLIB_SEQUENCE_COMPARE(req->sequence, >, dpy->request)) > + { > + throw_thread_fail_assert("Unknown sequence number while " > + "processing queue", > + xcb_xlib_threads_sequence_lost); > + } > dpy->last_request_read = req->sequence; > if(response) > break; > @@ -512,7 +564,15 @@ Status _XReply(Display *dpy, xReply *rep, int extra, > Bool discard) > char *reply; > PendingRequest *current; > > - assert(!dpy->xcb->reply_data); > + if (dpy->xcb->reply_data) > + { > + fprintf(stderr, "[xcb] Extra reply data still left in queue\n"); > + fprintf(stderr, > + "Most likely this is caused by a broken X extension " > + "library\n"); > + fprintf(stderr, "Aborting, sorry about that.\n"); > + assert(!dpy->xcb->reply_data); > + } > > if(dpy->flags & XlibDisplayIOError) > return 0; > @@ -568,7 +628,11 @@ Status _XReply(Display *dpy, xReply *rep, int extra, > Bool discard) > > req->reply_waiter = 0; > ConditionBroadcast(dpy, dpy->xcb->reply_notify); > - assert(XLIB_SEQUENCE_COMPARE(req->sequence, <=, dpy->request)); > + if(XLIB_SEQUENCE_COMPARE(req->sequence, >, dpy->request)) { > + throw_thread_fail_assert("Unknown sequence number while " > + "processing queue", > + xcb_xlib_threads_sequence_lost); > + } > dpy->last_request_read = req->sequence; > if(!response) > dequeue_pending_request(dpy, req); > @@ -665,8 +729,17 @@ int _XRead(Display *dpy, char *data, long size) > assert(size >= 0); > if(size == 0) > return 0; > - assert(dpy->xcb->reply_data != NULL); > - assert(dpy->xcb->reply_consumed + size <= dpy->xcb->reply_length); > + if(dpy->xcb->reply_data == NULL || > + dpy->xcb->reply_consumed + size > dpy->xcb->reply_length) { > + unsigned int xcb_xlib_too_much_data_requested = 1; > + fprintf(stderr, > + "[xcb] Too much data requested for reading from > _XRead\n"); > + fprintf(stderr, > + "Most likely this is caused by a broken X extension " > + "library\n"); > + fprintf(stderr, "Aborting, sorry about that.\n"); > + assert(!xcb_xlib_too_much_data_requested); > + } > memcpy(data, dpy->xcb->reply_data + dpy->xcb->reply_consumed, size); > dpy->xcb->reply_consumed += size; > _XFreeReplyData(dpy, False); > -- > 1.7.4.4 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
