[PATCH] wayland-server: Add APIs to get some structure's value

Pekka Paalanen ppaalanen at gmail.com
Wed Jun 22 13:46:51 UTC 2016


On Wed, 22 Jun 2016 19:45:15 +0900
______ <jhyuni.kang at samsung.com> wrote:

> Dear, Mr. Pekka Paalanen.
> 
> Thank you for your reply and opinion.
> I have checked Mr. Giulio's patch:
> https://patchwork.freedesktop.org/series/4181/
> But unfortunately my patch was little different with Mr. Giulio's.
> 
> His patch is to watch interfaces binded such clients.
> So He added wl_resource_get_interface() function to refer interface from
> resource.
> But My patch's purpose is that print which interface is supported in this
> server and
> which requests and events are supported current interface.

Hi,

for that, you need none of what you posted.

Just make a normal client, create a wl_registry, and you will see what
is *actually supported* by the server. You cannot even get that from
the functions you added, because you are not taking into account the
interface versions advertised by the server.

A server does not need to support the highest interface version that
it has the descriptions for.

> 
> But if he has plan to expand "GammaRay"'s function,
> my patch can support that.
> Because I add this APIs to support debugging.
> 
> I explain to my purpose to this patch.
> So please give me more opinions.
> 
> I want to see all of interfaces and requests/events names supported current
> server.
> 
> Ex> command in console: $print_supported_layout  
>      [1] wl_compositor
>          -- Request List --
>             0) create_surface
>             1) create_region
>          -- Event List --
> 
>      [2] wl_subcompositor
>          -- Request List --
>             0) destroy
>             1) get_subsurface
>          -- Event List --
> 
> Each server has different configuration,
> so that they could have different about supported interface.

As I said, you get all the runtime information you need from a
wl_registry as a normal client, and the rest you get from the protocol
XML descriptions that are installed in the system, also extensions, not
just Wayland core.

Try 'weston-info', works on any Wayland server.

> Because this could show not only wayland's basic protocol
> but also extra protocol which is user defined.

Ok, so you want information from server supported extensions for which
you do not have the XML files for. This is getting strange.

Is this for reverse-engineering?

> And it provide more convenience to client developer.

This I disagree. A client developer will always need the corresponding
protocol XML description to be able to use it. You need to generate the
programming language bindings from XML.

> The client developer can watch all of interface and requests/events easily.
> (Yes, I know that to watch xml file is more easier to expert.)

You can also look at what wayland-scanner generates from the XML, and
you can push that through Doxygen. Then you will have all of nice
layout, documentation, and C API.

I would very strongly suggest writing a better document generator if
reading XML or what we generate from it is worse than reading a runtime
string dump that cannot include documentation.

> This patch is not related to make new functions, fix bugs.
> Just to make easier to debug.
> Watch server's global list more easier.

You can watch server's global list as a normal client just fine, just
create a wl_registry and listen.

> I don't know this is enough explain why I want to added these APIs.
> 
> Have you give me more opinions or tips about this.
> I'm ready to listen your opinions.
> If you feel my purpose is reasonable I'll add more specific doc to each
> APIs.

Right, so what I understood is that you want two different things: to
know what a server supports at runtime (weston-info gives you that),
and to get "better" list of the interfaces. I would not consider your
patch to be a solution to either. Your approach cannot even access the
documentation we have so carefully written. If you need an index, then
please generate an index from the XML descriptions.

I hope I didn't totally misunderstand.

Do you have a real example of your proposal being used?

You made a sketch with "$print_supported_layout", but to me it is just a
database generated from the XML files, filtered with what weston-info
reports. You do not need any changes to libwayland or compositors or
anything to implement that. And when you use the XML files as the
source, you can also include the actual documentation any way you want.

I am leaning on rejecting this patch.


Thanks,
pq

> > -----Original Message-----
> > From: wayland-devel [mailto:wayland-devel-bounces at lists.freedesktop.org]
> > On Behalf Of Pekka Paalanen
> > Sent: Tuesday, June 21, 2016 5:02 PM
> > To: JengHyun Kang
> > Cc: wayland-devel at lists.freedesktop.org
> > Subject: Re: [PATCH] wayland-server: Add APIs to get some structure's
> > value
> > 
> > On Tue, 21 Jun 2016 13:45:16 +0900
> > JengHyun Kang <jhyuni.kang at samsung.com> wrote:
> >   
> > > This patch's purpose is getting global interface information
> > > registerred in the server.
> > > If global is created (used wl_global_create()), information are saved
> > > in global_list.
> > > But almost structures used in wayland is defined statically.
> > > So it is hard to get structure's values in server side.
> > >
> > > Added following APIs.
> > >   - wl_display_get_global_list()
> > >   - wl_global_get_interface()
> > >   - wl_interface_get_name()
> > >   - wl_interface_get_method_count()
> > >   - wl_interface_get_methods()
> > >   - wl_interface_get_event_count()
> > >   - wl_interface_get_events()
> > >   - wl_message_get_name()
> > >   - wl_global_list_get_global()
> > >
> > > You can get interface information to combine added APIs.
> > > (Such as interface's name and events/requests name)
> > >
> > > 1st, you can get wl_list:global_list to use  
> wl_display_get_global_list().
> > > 2nd, you can get length of wl_list to use wl_list_length() and
> > >      wl_global to use wl_global_list_get_global().
> > >      wl_global is saved in list, so you need index to get wl_global.
> > > 3rd, you can get wl_interface to use wl_global_get_interface().
> > > 4th, in wl_interface structure, there are so many information about
> > >      interface name and events/requests information.
> > >      so you can get information to use wl_interface_get_name(),
> > >      wl_interface_get_method_count(), wl_interface_get_method(),
> > >      wl_interface_get_event_count(), wl_interface_get_event().  
> > 
> > Hi,
> > 
> > your commit message forgot to explain the one most important thing:
> > 
> > Why?
> > 
> > Also, please see https://patchwork.freedesktop.org/series/4181/ which is
> > already reviewed and seems to overlap with yours. Giulio has demonstrated
> > the use case for all the API he is adding with a single
> > screenshot:
> > https://i.imgur.com/ihW3d88.jpg
> > 
> > and also roughly explaining how "GammaRay" hooks up to a compositor.
> > 
> > It would be nice if you reviewed Giulio's series, checked how it matches
> > your needs, and then explain what you are still missing and why.
> > Personally I am in favour of Giulio's series and no-one has yet disagreed
> > with it, so I think it has good chances of getting merged once polished.
> > 
> > 
> > Thanks,
> > pq
> >   
> > > ---
> > >  src/wayland-server.c | 63
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  src/wayland-server.h | 27 ++++++++++++++++++++++
> > >  2 files changed, 90 insertions(+)
> > >
> > > diff --git a/src/wayland-server.c b/src/wayland-server.c index
> > > 19aa2e8..ac5c2e5 100644
> > > --- a/src/wayland-server.c
> > > +++ b/src/wayland-server.c
> > > @@ -997,6 +997,63 @@ wl_global_destroy(struct wl_global *global)
> > >  	free(global);
> > >  }
> > >
> > > +WL_EXPORT const struct wl_interface * wl_global_get_interface(struct
> > > +wl_global *global) {
> > > +	return global->interface;
> > > +}
> > > +
> > > +WL_EXPORT struct wl_global *
> > > +wl_global_list_get_global(struct wl_list *list, int idx) {
> > > +	int i = 0;
> > > +	struct wl_global *global;
> > > +
> > > +	wl_list_for_each(global, list, link)
> > > +	{
> > > +		if (idx == i) break;
> > > +		i++;
> > > +	}
> > > +
> > > +	return global;
> > > +}
> > > +
> > > +WL_EXPORT const char *
> > > +wl_interface_get_name(const struct wl_interface *interface) {
> > > +	return interface->name;
> > > +}
> > > +
> > > +WL_EXPORT int
> > > +wl_interface_get_method_count(const struct wl_interface *interface) {
> > > +	return interface->method_count;
> > > +}
> > > +
> > > +WL_EXPORT const struct wl_message *
> > > +wl_interface_get_methods(const struct wl_interface *interface) {
> > > +	return interface->methods;
> > > +}
> > > +
> > > +WL_EXPORT int
> > > +wl_interface_get_event_count(const struct wl_interface *interface) {
> > > +	return interface->event_count;
> > > +}
> > > +
> > > +WL_EXPORT const struct wl_message *
> > > +wl_interface_get_events(const struct wl_interface *interface) {
> > > +	return interface->events;
> > > +}
> > > +
> > > +WL_EXPORT const char *
> > > +wl_message_get_name(const struct wl_message *message) {
> > > +	return message->name;
> > > +}
> > > +
> > >  /** Get the current serial number
> > >   *
> > >   * \param display The display object
> > > @@ -1035,6 +1092,12 @@ wl_display_get_event_loop(struct wl_display  
> > *display)  
> > >  	return display->loop;
> > >  }
> > >
> > > +WL_EXPORT struct wl_list *
> > > +wl_display_get_global_list(struct wl_display *display) {
> > > +	return &display->global_list;
> > > +}
> > > +
> > >  WL_EXPORT void
> > >  wl_display_terminate(struct wl_display *display)  { diff --git
> > > a/src/wayland-server.h b/src/wayland-server.h index a6e7951..3494641
> > > 100644
> > > --- a/src/wayland-server.h
> > > +++ b/src/wayland-server.h
> > > @@ -92,6 +92,33 @@ wl_display_remove_global(struct wl_display
> > > *display,
> > >
> > >  #endif
> > >
> > > +struct wl_list *
> > > +wl_display_get_global_list(struct wl_display *display);
> > > +
> > > +const struct wl_interface *
> > > +wl_global_get_interface(struct wl_global *global);
> > > +
> > > +const char *
> > > +wl_interface_get_name(const struct wl_interface *interface);
> > > +
> > > +int
> > > +wl_interface_get_method_count(const struct wl_interface *interface);
> > > +
> > > +const struct wl_message *
> > > +wl_interface_get_methods(const struct wl_interface *interface);
> > > +
> > > +int
> > > +wl_interface_get_event_count(const struct wl_interface *interface);
> > > +
> > > +const struct wl_message *
> > > +wl_interface_get_events(const struct wl_interface *interface);
> > > +
> > > +const char *
> > > +wl_message_get_name(const struct wl_message *message);
> > > +
> > > +struct wl_global *
> > > +wl_global_list_get_global(struct wl_list *list, int idx);
> > > +
> > >  #ifdef  __cplusplus
> > >  }
> > >  #endif  
> 
> 
> _______________________________________________
> 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/20160622/558966b0/attachment-0001.sig>


More information about the wayland-devel mailing list