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

[Daniel Stone]

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).

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.

Cheers,
Daniel