<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Thu, Oct 16, 2014 at 11:16 PM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, 16 Oct 2014 10:40:20 +0300 Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> wrote: > On Thu, 16 Oct 2014 09:46:39 +0300 > Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com">ppaalanen@gmail.com</a>> wrote: > > > On Wed, 15 Oct 2014 22:44:25 +0400 > > Dmitry Cherkassov <<a href="mailto:dcherkassov@gmail.com">dcherkassov@gmail.com</a>> wrote: > > > > > Hi list! > > > > > > The definition of wl_display_interface symbol can come from > > > libwayland-server.so or libwayland-client.so. > > > > > > There is a code in closed source EGL implementation that does > > > interfaces comparison via pointers, yielding negative results when > > > addresses are different (one address is saved by QtWayland, the other > > > one is from &wl_display_interface in EGL source code). > > > > > > Is there a smarter way to compare wl_display_interface's? > > > > > > There is this function: > > > > > > wl_interface_equal(const struct wl_interface *a, const struct wl_interface *b) > > > { > > > return a == b || strcmp(a->name, b->name) == 0; > > > } > > > > > > But as you can see it's not safe to use it alone because in case of > > > bad pointer we will get undefined results. > > > > > > Currently struct wl_interface is the following: > > > struct wl_interface { > > > const char *name; > > > int version; > > > int method_count; > > > const struct wl_message *methods; > > > int event_count; > > > const struct wl_message *events; > > > }; > > > > > > Since there is no magic field at the beginning it is not possible to > > > see whether a pointer points to the valid wl_interface structure. > > > > > > So my question is: how can we compare wl_interfaces in more clever and > > > safer way? > > > > Hmm, why would you ever have a bad pointer? > > > > Bad pointers in general tend to cause crashes and stuff, so you > > shouldn't be having a bad one in the first place. > > > > What is the background for your question/problem? > > Ok, I suppose this was discussed in IRC last night: the actual problem > behind the question is how to detect whether the void pointer > (EGLNativeDisplayType) given to eglCreateDisplay is actually a > wl_display. > > That's tough. > > A couple of hacks come to mind. > > bool > _eglNativePlatformDetectWayland(void *nativeDisplay) > { > void *f; > > if (!dereferencable(nativeDisplay)) > return false; > > f = *(void **)nativeDisplay; > > The first is to dlopen both libwayland-client and libwayland-server in > turn with RTLD_NOLOAD | RTLD_LOCAL, and fetching the > wl_display_interface symbol from each, comparing those to 'f'. If > either matches, return true. While discussing in IRC, I realized half of this is nonsense. eglGetDisplay() can only get a wl_display created by libwayland-client, and never one created by libwayland-server. Assuming the dynamic linking is sane, the interface pointer in wl_display is *always* the one from libwayland-client.so. Now we just need to make sure, that we compare 'f' to the interface pointer from libwayland-client.so. That could probably be done with the dlopen/dlsym trick, or... > Another would be to check if 'f' is dereferencable (and not equal to > the default wl_display_interface), and check if (char *)f starts with > "wl_d". If yes, do a full string comparison with "wl_display". It's > not reliable, but maybe works enough. > > The proper solution is for apps to use EGL_KHR_platform_wayland to > explicitly say the pointer is a wl_display. That of course doesn't work > on old apps, so you might implement a workaround through environment > like Mesa does: EGL_PLATFORM=wayland bypasses all autodetection and > assumes it is a wl_display. > > In any case, EGL really should implement EGL_KHR_platform_wayland so we > have a chance to get rid of the inherently unreliable autodetect. And we > should start using that in our demo apps... > > If we consider autodetecting client wl_display pointers worth to solve > properly, we should probably add a function for that in > libwayland-client.so. However, implementing that portably might be > difficult. ...adding a function int/bool wl_is_wl_display_pointer(void *p) into libwayland-client.so. As that function would be built into libwayland-client.so, it naturally uses the client.so version of wl_display_interface symbol (unless dynamic linking miraculously manages to mess that up?). 'p' would be the tentative 'struct wl_display *', so that the implementation details of struct wl_display would be contained in libwayland-client.so, and not leaked elsewhere like e.g. in Mesa. All this means we should be able to get away with just one pointer comparison, after assuring that the first sizeof(void*) bytes pointed to by the given unknown pointer (EGLNativeDisplayType) are dereferencable without a segfault or other fatal error. So, a quick fix for EGL implementations right now would be to try the dlopen hack, until they can start to depend on a new libwayland-client.so providing wl_is_wl_display_pointer(). I am unsure about the exact definition of wl_is_wl_display_pointer(): can it do the dereferencability check itself, or should we require the caller to guarantee that dereferencing cannot crash. The latter would leave the portability burden on the callers rather than libwayland. How's that sound? </blockquote><div> </div><div>That sounds fine. I think we can count on the client passing in a valid pointer to something. Chances are that whatever the client passes in will be a pointer to some sort of structure and so we can probably read 8 bytes without a problem. If we really wanted to, we could do a quick alignment check to ensure that it's sizeof(void *)-aligned. </div><div>I don't know that I care whether we do your wl_is_wl_display_pointer or Bill's "dynamic cast" version. It doesn't much matter. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Is it possible to have several instances of libwayland-client in a process address space? Meaning we would have several different client-side wl_display_interface pointers? </blockquote><div> </div><div>I don't think so. At least not without some very bizarre hackery. Also, I think it's reasonable to assume that either a) libEGL is reasonably normally linked against the same chunks of code that link to libwayland-client or b) The client really knows what they're doing and is prepared for the consequences. Because really, if they're pulling those kinds of tricks they're asking for trouble. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> If yes, it's back to the string comparisons... </blockquote><div> </div><div>String comparisons are right out. They'll cause a segfault the moment it's not a wl_display pointer. </div></div>--Jason </div></div>