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 <psyc...@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
_______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel