[Mesa-dev] [PATCH 7/9] wayland-drm: static inline wayland_drm_buffer_get

[Pekka Paalanen]

On Mon, 23 Oct 2017 16:41:14 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi Emil,
> 
> On 28 September 2017 at 13:36, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > On 20 September 2017 at 15:06, Daniel Stone <daniel at fooishbar.org> wrote:  
> >> On 19 September 2017 at 11:25, Emil Velikov <emil.l.velikov at gmail.com> wrote:  
> >>> It looks a bit icky and my Wayland knowledge is limited to actually review it.
> >>>
> >>> I still think that we're trying to do different things - me simplify
> >>> things, while you're focusing on fixing a bug.  
> >>
> >> I like the idea of the simplification, but it's just that a) it can't
> >> actually be simplified that far, and b) whilst we still rely on
> >> consistent resolution of wl_buffer_interface, your proposed change may
> >> actually _introduce_ a bug.  
> >
> > Right, I did not see that one. Can you share your train of thought?  
> 
> Happily!
> 
> When a compositor creates a client buffer, the request is dispatched
> by libwayland-server.so into libEGL.so (libEGL explicitly links
> libwayland-server). This lands in
> src/egl/wayland/wayland-drm/wayland-drm.c's create_buffer(), which
> calls wl_resource_create() with 'wl_buffer_interface'. Since libEGL.so
> is explicitly linked against both libwayland-client and
> libwayland-server, and both of those DSOs provide a resolution of
> wl_buffer_interface, resource->interface could be
> libwayland-server.so::wl_buffer_interface, or
> libwayland-client.so::wl_buffer_interface.
> 
> When a compositor wants to import a buffer into GBM, it calls
> gbm_bo_import(), which will call wl_drm_buffer_get().
> wl_drm_buffer_get will test (resource->interface ==
> &wl_buffer_interface). Previously, both create_buffer() and
> wl_drm_buffer_get() at least came from the same compilation unit, but
> now they are built and linked separately. This seems to make an
> existing problem (ambiguity of 'wl_buffer_interface') worse (symbol
> resolution now performed in separate compilation units).

Hi,

this is not the whole truth. At least, if wl_drm_buffer_get() uses
wl_resource_instance_of() rather than open-coding a broken version of
the same.

> I don't really see a solution to this, apart from no longer relying on
> having a single canonical resolution of wl_buffer_interface. The above
> patch implements that, by removing the interface-address-equal test
> and replacing it with the destroy-listener test. The patch I provided
> is equally valid both with and without your series.

There is no relying on having a single canonical resolution of
wl_buffer_interface, and there has never been as long as any part of
libwayland has been stable.

See:
https://cgit.freedesktop.org/wayland/wayland/tree/src/connection.c?id=0.99.0#n840

I suppose one might argue that libwayland-server and libwayland-client
might be different versions in some wacky distribution, but that
argument is moot because wl_buffer_interface can never change due to
the multiple factory interfaces issue.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20171024/cd1f8721/attachment-0001.sig>