Hi Jonas,

> The API additions to wl_global should be their own patches IMHO.

Agreed, I have splitted the patch in 3:

 - Add the global filter with its own user data
 - Add the API to retrieve the wl_global's interface and data
 - Add the test

> What I meant was that one would pass a user_data with set_filter() but
> using the user_data that is already set on the global is fine too I
> guess.  If so, we could probably just not pass a user_data, and add a
> wl_global_get_user_data() instead.

I reckon having both a dedicated user data for the filter and still
being able to access the wl_global's associated data is helpful, so I
added both.

> nit: Could be "global_filter". We are already in wl_display here.

Done.

> nit: if you put () around the a || b expression you can rely on your
> editor for indentation.

Done, Not sure my editor does that, but adding a parenthesis puts us on
a tab boundary, so it's nice anyway :)

> "Set a filter for"

Done.

> I'd suggest:
> 
>       The set filter will be used during wl_global advertisment to
>       determine whether a global object should be advertised to a
>       given client, and during wl_global binding to determine whether
>       a given client should be allowed to bind to a global.
> 
>       Clients that try to bind to a global that was filtered out will
>       have an error raised.
> 
>       Setting the filter NULL will result in all globals being
>       advertised to all clients. The default is no filter.

Done

> Jonas

Thanks again for your review,

Cheers,
Olivier

Olivier Fourdan (3):
  wayland-server: Add API to control globals visibility
  wayland-server: Add functions to wl_global
  tests: Add a test for global filter

 src/wayland-server-core.h | 16 ++++++++++
 src/wayland-server.c      | 74 +++++++++++++++++++++++++++++++++++++++++++----
 tests/display-test.c      | 58 +++++++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+), 5 deletions(-)

-- 
2.7.4

_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to