[PATCH] server: Add get/set user data for wl_client
Pekka Paalanen
ppaalanen at gmail.com
Fri Feb 26 13:17:26 UTC 2016
On Wed, 17 Feb 2016 18:47:44 +0900
Sung-Jin Park <input.hacker at gmail.com> wrote:
> Hi pq,
> I'm sorry for replying late.
>
> The reasons why I tried to add are following.
> - If a wl_client is able to has a user data,
> a wayland server can use it for storing the client specific data such as
> security privilege, the result of privilege check, and other things.
> Those kinds of data are the client specific and need to be removed when
> the client is gone.
> In this case, it'll be better to use wl_client' user data as a data storage
> other than storing those information in the server's data.
> - There was a guy who wants to have tyis kind of APIs. (As I remember, he
> is nicesj.)
> - There was a reply for the above request and It seemed positive to me.
>
> If this can be merged into the stream, it'll be good for me and other guys
> who wants to find this out. :)
Hi,
yes, you want to store data for each wl_client. Is it really so much
better to add a single-use field in wl_client, when you can already add
any number of user data records by using a wl_client destroy listener
as the handle to the data?
Do you need an example of how that works?
Is your security thing an integral part of the compositor you are
writing, or is it an add-on? If it is an add-on or its own sub-system,
it probably should not use wl_client user data field, because the
compositor core might want to use that for its own purposes.
As I explained below, Sung-jae Park wants a notification when a new
client is created, which is a good idea. How to store user data for
wl_client is a separate issue.
I'm still waiting for comments from other Wayland developers or
compositor writers whether we really need two different ways to store
user data for a wl_client. The way we already have is more versatile
than the one you are proposing.
Thanks,
pq
> 2016. 2. 11. 오후 7:08에 "Pekka Paalanen" <ppaalanen at gmail.com>님이 작성:
>
> > Hi,
> >
> > I think we should think this a little more.
> >
> > There is no absolute requirement to add a user_data member to
> > wl_client, because you can use a destroy listener to look up your user
> > data struct. When you attach user data, you also want to always be
> > notified about wl_client destruction, which means you always have a
> > destroy listener.
> >
> > The downside of using a destroy listener to look up your user data
> > struct is that it is an O(number of destroy listeners) operation rather
> > than O(1), but usually that number should be 1 or 2 I think, your user
> > data destructor included.
> >
> > So the first question is, is this really worthwhile?
> >
> > For comparison to another related patch, adding the client created
> > signal is obviously worthwhile, since there is no other sure way to be
> > notified about new wl_clients.
> >
> > Then let's assume user_data for wl_client is worthwhile. The closest
> > example of a server-side structure with user data is wl_resource.
> > wl_resource also has an explicit wl_resource_destroy_func_t member, and
> > the rationale is clear: if you add user data, you want to be able to
> > tear down the user data at the right time, so you always need a way to
> > be notified about destruction.
> >
> > The second question is, should also wl_client have an explicit user
> > data destructor function? If yes, you have to add API for it.
> >
> > The third question is: do all wl_client objects have the same single
> > owner?
> >
> > The owner problem is particularly visible in the client side with
> > wl_proxy, which also has a single user data member of type void
> > pointer. This creates two problems:
> >
> > - If wl_proxy should have more than one user data attached, only one of
> > them can use the user data member and the others must use the
> > destructor trick anyway - oops, wl_proxy does not have a destroy
> > signal, but at least wl_client does.
> >
> > - It is not always obvious who owns the user data. If there are more
> > than one component that assume they own the wl_proxy and its user
> > data, then passing the wl_proxy to another component that also
> > assumes the same causes the other component to access not the data it
> > thinks it gets.
> >
> > The latter problem is especially dangerous when an application is
> > sharing the same connection with multiple toolkits. Both toolkits may
> > create wl_surface objects and naturally put their own pointer in the
> > wl_proxy user data field. They also listen for input events, and input
> > events carry a reference to a wl_surface. Therefore, a toolkit may
> > receive a wl_proxy with the other toolkit's user data, and crash or
> > misbehave due to assuming it is its own.
> >
> > In the wl_client case, I think the owner issue should be negligible,
> > there is always one obvious owner for the wl_client and its user data -
> > or that is what I assume, as I haven't read any other compositor code
> > than Weston's.
> >
> > In summary, while there is technically nothing wrong with this idea per
> > se, I do wonder if it is a net win.
> >
> > I believe that if there is a dedicated user data slot, there should
> > also be a dedicated destructor function slot for it.
> >
> > What do others think? I could go either way.
> >
> >
> > On Wed, 27 Jan 2016 19:34:56 +0900
> > Sung-Jin Park <sj76.park at samsung.com> wrote:
> >
> > > Signed-off-by: Sung-Jin Park <sj76.park at samsung.com>
> > > ---
> > > src/wayland-server-core.h | 6 ++++++
> > > src/wayland-server.c | 13 +++++++++++++
> > > 2 files changed, 19 insertions(+)
> > >
> > > diff --git a/src/wayland-server-core.h b/src/wayland-server-core.h
> > > index e8e1e9c..6990423 100644
> > > --- a/src/wayland-server-core.h
> > > +++ b/src/wayland-server-core.h
> > > @@ -186,6 +186,12 @@ int
> > > wl_client_get_fd(struct wl_client *client);
> > >
> > > void
> > > +wl_client_set_user_data(struct wl_client *client, void *data);
> > > +
> > > +void *
> > > +wl_client_get_user_data(struct wl_client *client);
> > > +
> > > +void
> > > wl_client_add_destroy_listener(struct wl_client *client,
> > > struct wl_listener *listener);
> > >
> > > diff --git a/src/wayland-server.c b/src/wayland-server.c
> > > index 6654cd7..e7a6eae 100644
> > > --- a/src/wayland-server.c
> > > +++ b/src/wayland-server.c
> > > @@ -81,6 +81,7 @@ struct wl_client {
> > > struct wl_signal destroy_signal;
> > > struct ucred ucred;
> > > int error;
> > > + void *data;
> >
> > Please name this user_data.
> >
> > > };
> > >
> > > struct wl_display {
> > > @@ -526,6 +527,18 @@ wl_client_get_fd(struct wl_client *client)
> > > return wl_connection_get_fd(client->connection);
> > > }
> > >
> > > +WL_EXPORT void
> > > +wl_client_set_user_data(struct wl_client *client, void *data)
> > > +{
> > > + client->data = data;
> > > +}
> > > +
> > > +WL_EXPORT void *
> > > +wl_client_get_user_data(struct wl_client *client)
> > > +{
> > > + return client->data;
> > > +}
> >
> > Documentation is missing.
> >
> > > +
> > > /** Look up an object in the client name space
> > > *
> > > * \param client The client object
> >
> > Thanks
> > pq
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >
> >
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160226/3d0fecbe/attachment-0001.sig>
More information about the wayland-devel
mailing list