[PATCH libX11] XCB: Add more friendly error messages for common asserts
Peter Hutterer
peter.hutterer at who-t.net
Sun May 15 16:21:53 PDT 2011
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 <daniel at fooishbar.org>
> ---
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 <peter.hutterer at who-t.net>
> 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
More information about the xorg-devel
mailing list