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

[Laércio de Sousa]

I will send a v3 patch with some last-minute fixes found right after v2.
Hopefully all your advices are contempled now.

Please tell me if you have any other concerns.

Att.

2015-11-06 15:18 GMT-02:00 Uli Schlachter <psychon at znc.in>:

> Hi,
>
> Am 06.11.2015 um 14:10 schrieb Laércio de Sousa:
> [...]
> >>> +static void
> >>> +_NestedClientSetWMClass(NestedClientPrivatePtr pPriv,
> >>> +                        const char* wm_class)
> >>> +{
> >>> +    size_t class_len = strlen(wm_class) + 1;
> >>> +    char *class_hint = malloc(class_len);
> >>> +
> >>> +    if (class_hint)
> >>> +    {
> >>> +        strcpy(class_hint, wm_class);
> >>> +        xcb_change_property(pPriv->conn,
> >>> +                            XCB_PROP_MODE_REPLACE,
> >>> +                            pPriv->window,
> >>> +                            XCB_ATOM_WM_CLASS,
> >>> +                            XCB_ATOM_STRING,
> >>> +                            8,
> >>> +                            class_len,
> >>> +                            class_hint);
> >>> +        free(class_hint);
> >>> +    }
> >>
> >> Why is this strcpy needed?
> >>
> > I've copied over this piece of code from Xephyr. In that context,
> > class_hint stores a concatenation bewteen two strings, so that strcpy is
> > needed, but here the WM_CLASS string is much more simple, so that strcpy
> is
> > not needed. I'll remove it. Thanks!
>
> Uhm, right. This wants to set WM_CLASS and WM_CLASS should contain "two"
> strings
> (the class and the instance), separated by a null byte.
>
> Sorry for not noticing this earlier, but isn't this code wrong then?
>
> https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.2.5
>
> [...]
> >>> +        if (!ev)
> >>> +        {
> >>> +            if (_NestedClientConnectionHasError(pPriv->scrnIndex,
> >>> +                                                pPriv->conn))
> >>> +            {
> >>> +                /* XXX: Is there a better way to do this? */
> >>> +                xf86DrvMsg(pPriv->scrnIndex,
> >>> +                           X_ERROR,
> >>> +                           "Connection with host X server lost.\n");
> >>> +                free(ev);
> >>> +                NestedClientCloseScreen(pPriv);
> >>> +                exit(1);
> >>> +            }
> >>> +
> >>> +            break;
> >>> +        }
> >>> +
> >>> +        switch (ev->response_type & ~0x80)
> >>> +        {
> >>> +        case XCB_EXPOSE:
> >>> +            _NestedClientProcessExpose(pPriv, ev);
> >>> +            break;
> >>> +        case XCB_CLIENT_MESSAGE:
> >>> +            _NestedClientProcessClientMessage(pPriv, ev);
> >>> +            break;
> >>> +        case XCB_MOTION_NOTIFY:
> >>> +            _NestedClientProcessMotionNotify(pPriv, ev);
> >>> +            break;
> >>> +        case XCB_KEY_PRESS:
> >>> +            _NestedClientProcessKeyPress(pPriv, ev);
> >>> +            break;
> >>> +        case XCB_KEY_RELEASE:
> >>> +            _NestedClientProcessKeyRelease(pPriv, ev);
> >>> +            break;
> >>> +        case XCB_BUTTON_PRESS:
> >>> +            _NestedClientProcessButtonPress(pPriv, ev);
> >>> +            break;
> >>> +        case XCB_BUTTON_RELEASE:
> >>> +            _NestedClientProcessButtonRelease(pPriv, ev);
> >>> +            break;
> >>> +        }
> >>> +
> >>> +        free(ev);
> >>> +        xcb_flush(pPriv->conn);
> >>
> >> Why is this flushing inside of the event loop? Wouldn't a flush after
> the
> >> loop
> >> be enough?
> >>
> > Well... Comparing it with other XCB snippets I've found in the web, it
> > seems this xcb_flush() call is not needed at all. Calling it right atfer
> > window creation or redrawing (XCB_EXPOSE event only) should be enough,
> > right?
> [...]
>
> Well, it depends. Something should flush in the end in case there are any
> requests still in XCB's output buffer (and nothing implicitly flushes them
> by
> waiting for a reply). So I think that having a call to xcb_flush() before
> returning to this function should be added. If it does something, it just
> prevented a bug and if it doesn't do anything, it's not expensive. ;-)
>
> Everything else seems fine, thanks.
>
> Uli
> --
> "For saving the Earth.. and eating cheesecake!"
>



-- 
*Laércio de Sousa*
*Orientador de Informática*
*Escola Municipal "Professor Eulálio Gruppi"*
*Rua Ismael da Silva Mello, 559, Mogi Moderno*
*Mogi das Cruzes - SPCEP 08717-390*
Telefone: (11) 4726-8313
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg-devel/attachments/20151109/c06677e4/attachment-0001.html>