[PATCH xlib] Add support for a display-specific error handler

[Uli Schlachter]

Am 16.11.2015 um 21:57 schrieb Jasper St. Pierre:
> Now do this:
> 
> static void Foo()
> {
>     Request1();
>     Request2();
> }
> 
> static void Bar()
> {
>     Request1();
>     Request3();
> }
> 
> static void Thing(int x)
> {
>     SetErrorHandler();
>     if (x == 0)
>         Foo();
>     else
>         Bar();
>     UnsetErrorHandlerAndCheck();
> }
> 
> We have a similar codepath in our driver and this is where I gave up
> and started on the Xlib solution.

I agree that this gets a little more complicated, but ok:

struct pending_checked_requests {
  struct pending_checked_requests *next;
 xcb_void_request_t request;
};

static void add_checked(struct pending_checked_requests **pend,
xcb_void_request_t request)
{
  struct pending_checked_requests *n = malloc(sizeof(*n));
  n->request = request;
  n->next = *pend;
  *pend = n;
}

static int check_errors(struct pending_checked_requests *pend)
{
  int result = 0;
  while (pend) {
    if (xcb_request_check(pend->request)) {
       free error;
       result = 1;
    }
    struct pending_checked_requests *last = pend;
    pend = pend->next;
    free(last);
  }
  return result;
}

static void Bar(struct pcq **pend)
{
  add_checked(pend, Request1());
  add_checked(pend, Request2());
}

static int Thing(int x)
{
  struct pending_checked_requests *pend = NULL;
  if (x == 0)
    Bar(&pend);
  return check_errors(pend);
}

(And if (which I guess) the frequent malloc()s are a problem, then add something
more sophisticated where you e.g. have space for 20 requests in each such
structure and use stack space for the 50 first requests.)

> On Mon, Nov 16, 2015 at 12:39 PM, Uli Schlachter <psychon at znc.in> wrote:
>> Am 16.11.2015 um 18:06 schrieb Jasper St. Pierre:
>> [...]
>>> We currently use a behavior like:
>>>
>>>     SetErrorHandler();
>>>     Request1();
>>>     Request2();
>>>     Request3();
>>>     if (UnsetErrorHandlerAndCheck())
>>>         goto error;
>>>
>>> UnsetErrorHandlerAndCheck will XSync, and look at any newly incoming
>>> errors, check their serials so that they match up, and if they are
>>> inside those bounds, we return the error to the user and return that
>>> no error happened. Fairly standard trick.
>>>
>>> With xcb, I can do:
>>>
>>>     if (check_request(request_1_checked(), &err) {
>>>         free(err);
>>>         goto error;
>>>     }
>>>     if (check_request(request_2_checked(), &err) {
>>>         free(err);
>>>         goto error;
>>>     }
>>>     if (check_request(request_3_checked(), &err) {
>>>         free(err);
>>>         goto error;
>>>     }
>>>
>>> So that's clearly unusable.
>>
>> Well, why?
>>
>> How about this version:
>>
>> xcb_void_cookie_t cookies[3];
>>
>> cookies[0] = request_1_checked();
>> cookies[1] = request_2_checked();
>> cookies[2] = request_3_checked();
>>
>> bool had_error = false;
>> for (int i = 0; i < 3; i++) {
>>   xcb_generic_error_t *err = xcb_request_check(c, cookies[i]);
>>   had_error |= err != NULL;
>>   free(NULL);
>> }
>>
>> This only syncs once during the first call to xcb_request_check() and afterwards
>> it's done. This even improves parallelism, because you do not have to
>> XLockDisplay() to lock out other threads (which also means that "just a range of
>> sequence numbers" isn't enough and an array is needed).
>>
>> You could even generalize this with some helper functions to make it nicer,
>> similar to your SetErrorHandler() and UnsetErrorHandlerAndCheck() above.
>>
>> Uli
>>
>> [...]
>>> The Display-specific error handler solves my needs here by giving me a
>>> callback to that above model that's locked to that local section that
>>> works well from a library -- this code runs in a different thread, and
>>> we had issues where we overwrite another thread's error handler (in my
>>> case, the two threads were using different Displays, as they should).
>>>
>>> On Sun, Nov 15, 2015 at 11:56 PM, Keith Packard <keithp at keithp.com> wrote:
>>>> "Jasper St. Pierre" <jstpierre at mecheye.net> writes:
>>>>
>>>>> Writing error-safe code that uses Xlib is too obnoxious, and using XCB
>>>>> is tedious and not performant, as we can't catch events on a giant
>>>>> stream -- we have to check every operation manually.
>>>>
>>>> You get errors returned in the event stream; is there something fancier
>>>> you'd like from xcb to make it faster?
>>>>
>>>> --
>>>> -keith
>>
>> --
>> "Every once in a while, declare peace. It confuses the hell out of your enemies"
>>  - 79th Rule of Acquisition
>> _______________________________________________
>> xorg-devel at lists.x.org: X.Org development
>> Archives: http://lists.x.org/archives/xorg-devel
>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 
> 
> 


-- 
Who needs a ~/.signature anyway?