[PATCH xf86-video-nested 5/5] Introduce a new XCB client backend, and make it the default one.

Uli Schlachter psychon at znc.in
Wed Nov 5 14:16:37 PST 2014


Hi,

Am 31.10.2014 um 14:12 schrieb LaƩrcio de Sousa:
> +    case XCB_CONN_CLOSED_REQ_LEN_EXCEED:
> +        xf86DrvMsg(scrnIndex,
> +                   X_ERROR,
> +                   "Connection to host X server closed: too many requests.\n");
> +        return TRUE;


This isn't a "too many requests" error, this is "something tried to send a
single request larger than the supported maximum request length" (Think:
"Something tried to send a request of size 1 GiB"). Or, to quote xcb.h:

 /** Connection closed, exceeding request length that server accepts. */
 #define XCB_CONN_CLOSED_REQ_LEN_EXCEED 4

[...]
> +static void
> +_NestedClientSetWindowTitle(NestedClientPrivatePtr pPriv,
> +                            const char *extra_text)
> +{
[...]
> +    xcb_flush(pPriv->conn);
> +}
[...]
> +static Bool
> +_NestedClientHostXInit(NestedClientPrivatePtr pPriv)
> +{
[...]
> +    xcb_flush(pPriv->conn);
> +}
[...]

What's your strategy for placing calls to xcb_flush()? I'd like to see each
xcb_flush() having a comment explaining why it is needed.

E.g. _NestedClientSetWindowTitle() calls xcb_flush(), but its only caller,
_NestedClientCreateWindow() does not flush. I can't see why a flush would be
required between setting a window title and mapping the window.

My preferred solution would be to remove all these calls to xcb_flush().
Instead, "something in the event loop" should flush. For example, you could
place a call to xcb_flush() at the end of NestedClientCheckEvents() and remove
all other calls (no idea if this actually works, I don't know the code well enough).

> +void
> +NestedClientCheckEvents(NestedClientPrivatePtr pPriv)
> +{
> +    xcb_generic_event_t *ev;
> +
> +    while (TRUE)
> +    {
> +        ev = xcb_poll_for_event(pPriv->conn);
> +
> +        if (!ev)
> +        {
> +            if (xcb_connection_has_error(pPriv->conn))
> +                exit(1);
[...]

Really? Doesn't this mean "when something goes wrong, we will silently exit
without any error message"?

Cheers,
Uli
-- 
"Why make things difficult, when it is possible to make them cryptic
and totally illogical, with just a little bit more effort?" -- A. P. J.


More information about the xorg-devel mailing list