<div dir="ltr"><div><div>I will send a v3 patch with some last-minute fixes found right after v2. Hopefully all your advices are contempled now.<br><br></div>Please tell me if you have any other concerns.<br><br></div>Att.<br></div><div class="gmail_extra"><br><div class="gmail_quote">2015-11-06 15:18 GMT-02:00 Uli Schlachter <span dir="ltr"><<a href="mailto:psychon@znc.in" target="_blank">psychon@znc.in</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
Am 06.11.2015 um 14:10 schrieb Laércio de Sousa:<br>
[...]<br>
<div><div class="h5">>>> +static void<br>
>>> +_NestedClientSetWMClass(NestedClientPrivatePtr pPriv,<br>
>>> +                        const char* wm_class)<br>
>>> +{<br>
>>> +    size_t class_len = strlen(wm_class) + 1;<br>
>>> +    char *class_hint = malloc(class_len);<br>
>>> +<br>
>>> +    if (class_hint)<br>
>>> +    {<br>
>>> +        strcpy(class_hint, wm_class);<br>
>>> +        xcb_change_property(pPriv->conn,<br>
>>> +                            XCB_PROP_MODE_REPLACE,<br>
>>> +                            pPriv->window,<br>
>>> +                            XCB_ATOM_WM_CLASS,<br>
>>> +                            XCB_ATOM_STRING,<br>
>>> +                            8,<br>
>>> +                            class_len,<br>
>>> +                            class_hint);<br>
>>> +        free(class_hint);<br>
>>> +    }<br>
>><br>
>> Why is this strcpy needed?<br>
>><br>
> I've copied over this piece of code from Xephyr. In that context,<br>
> class_hint stores a concatenation bewteen two strings, so that strcpy is<br>
> needed, but here the WM_CLASS string is much more simple, so that strcpy is<br>
> not needed. I'll remove it. Thanks!<br>
<br>
</div></div>Uhm, right. This wants to set WM_CLASS and WM_CLASS should contain "two" strings<br>
(the class and the instance), separated by a null byte.<br>
<br>
Sorry for not noticing this earlier, but isn't this code wrong then?<br>
<br>
<a href="https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.2.5" rel="noreferrer" target="_blank">https://tronche.com/gui/x/icccm/sec-4.html#s-4.1.2.5</a><br>
<br>
[...]<br>
<div><div class="h5">>>> +        if (!ev)<br>
>>> +        {<br>
>>> +            if (_NestedClientConnectionHasError(pPriv->scrnIndex,<br>
>>> +                                                pPriv->conn))<br>
>>> +            {<br>
>>> +                /* XXX: Is there a better way to do this? */<br>
>>> +                xf86DrvMsg(pPriv->scrnIndex,<br>
>>> +                           X_ERROR,<br>
>>> +                           "Connection with host X server lost.\n");<br>
>>> +                free(ev);<br>
>>> +                NestedClientCloseScreen(pPriv);<br>
>>> +                exit(1);<br>
>>> +            }<br>
>>> +<br>
>>> +            break;<br>
>>> +        }<br>
>>> +<br>
>>> +        switch (ev->response_type & ~0x80)<br>
>>> +        {<br>
>>> +        case XCB_EXPOSE:<br>
>>> +            _NestedClientProcessExpose(pPriv, ev);<br>
>>> +            break;<br>
>>> +        case XCB_CLIENT_MESSAGE:<br>
>>> +            _NestedClientProcessClientMessage(pPriv, ev);<br>
>>> +            break;<br>
>>> +        case XCB_MOTION_NOTIFY:<br>
>>> +            _NestedClientProcessMotionNotify(pPriv, ev);<br>
>>> +            break;<br>
>>> +        case XCB_KEY_PRESS:<br>
>>> +            _NestedClientProcessKeyPress(pPriv, ev);<br>
>>> +            break;<br>
>>> +        case XCB_KEY_RELEASE:<br>
>>> +            _NestedClientProcessKeyRelease(pPriv, ev);<br>
>>> +            break;<br>
>>> +        case XCB_BUTTON_PRESS:<br>
>>> +            _NestedClientProcessButtonPress(pPriv, ev);<br>
>>> +            break;<br>
>>> +        case XCB_BUTTON_RELEASE:<br>
>>> +            _NestedClientProcessButtonRelease(pPriv, ev);<br>
>>> +            break;<br>
>>> +        }<br>
>>> +<br>
>>> +        free(ev);<br>
>>> +        xcb_flush(pPriv->conn);<br>
>><br>
>> Why is this flushing inside of the event loop? Wouldn't a flush after the<br>
>> loop<br>
>> be enough?<br>
>><br>
> Well... Comparing it with other XCB snippets I've found in the web, it<br>
> seems this xcb_flush() call is not needed at all. Calling it right atfer<br>
> window creation or redrawing (XCB_EXPOSE event only) should be enough,<br>
> right?<br>
</div></div>[...]<br>
<br>
Well, it depends. Something should flush in the end in case there are any<br>
requests still in XCB's output buffer (and nothing implicitly flushes them by<br>
waiting for a reply). So I think that having a call to xcb_flush() before<br>
returning to this function should be added. If it does something, it just<br>
prevented a bug and if it doesn't do anything, it's not expensive. ;-)<br>
<br>
Everything else seems fine, thanks.<br>
<span class="HOEnZb"><font color="#888888"><br>
Uli<br>
--<br>
"For saving the Earth.. and eating cheesecake!"<br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div class="gmail_signature"><div dir="ltr"><div><b>Laércio de Sousa</b><br></div><div style="text-align:center"><div style="text-align:left"><i>Orientador de Informática</i></div><div style="text-align:left"><u>Escola Municipal "Professor Eulálio Gruppi"</u></div><div style="text-align:left"><i>Rua Ismael da Silva Mello, 559, Mogi Moderno</i></div><i><div style="text-align:left"><i>Mogi das Cruzes - SP</i></div><div style="text-align:left"><i>CEP 08717-390</i></div></i></div><div><span style="font-family:arial;font-size:small">Telefone: (11) 4726-8313</span></div></div></div>
</div>