[PATCH 2/3] Modify wayland-scanner to generate inert objects

Marek Chalupa mchqwerty at gmail.com
Thu Mar 12 05:31:30 PDT 2015


I wonder if the same effect could be achieved by few smaller changes. What
I'm thinking about is something like:

wl_resource_set_intact()

So far I've got three ways how to achieve that:
1) change scanner so that it saves opcode of destructor events in
wl_interface, so when we invoke a closure, we can bail out if the resource
is intact and it is not a destructor
2) do the same but with wl_message

These first two options would break ABI, so I'm not fond of these.

3) add a sign to the siganture that the message is a destructor.

I went ahead and tried the last one locally. Generally, it work like this:

get_closure()
if (resource_is_intact && message_is_not_destructor)
    return
else
   invoke

(message_is_not_destructor <=> signature does not contain 'D')

This approach does not break ABI and is nicely compatible with old
protocols. It only adds one function wl_resource_set_intact() (thus changes
API) and the protocols must be re-generated and re-linked, but since it is
backward-compatible it is not crucial dependency (old protocols will work,
but message_is_not_destructor always return true)

What do you think?

Cheers,
Marek


On Wed, Feb 25, 2015 at 9:03 AM, David FORT <rdp.effort at gmail.com> wrote:

> As stated in the very good blog post[1] of Pekka Paalanen, server-side we
> can have
> sometime troubles with object that the server has deleted but that the
> client
> is still requesting. This patch complements the scanner to create some code
> that will return inert objects, ie objects that will do nothing and will
> never
> return anything else but an inert object. An example is a get_pointer() on
> a wl_seat, with an unplugged mouse (and so no pointer). Instead of keeping
> alive
> the old pointer, we could bind that inert object and wait that the client
> release
> it (which should happen very quickly as a wl_global_remove should be in
> the wire).
>
> [1]:
> http://ppaalanen.blogspot.fi/2014/07/wayland-protocol-design-object-lifespan.html
> ---
>  src/scanner.c      | 173
> +++++++++++++++++++++++++++++++++++++++++++++++++----
>  wayland-scanner.mk |   3 +
>  2 files changed, 166 insertions(+), 10 deletions(-)
>
> diff --git a/src/scanner.c b/src/scanner.c
> index 1f1e59a..dbdd1f6 100644
> --- a/src/scanner.c
> +++ b/src/scanner.c
> @@ -39,7 +39,7 @@ enum side {
>  static int
>  usage(int ret)
>  {
> -       fprintf(stderr, "usage: ./scanner
> [client-header|server-header|code]\n");
> +       fprintf(stderr, "usage: ./scanner
> [client-header|server-header|code|server-inert]\n");
>         fprintf(stderr, "\n");
>         fprintf(stderr, "Converts XML protocol descriptions supplied on "
>                         "stdin to client headers,\n"
> @@ -626,6 +626,18 @@ emit_type(struct arg *a)
>         }
>  }
>
> +static struct arg *
> +get_return_type(struct message *m)
> +{
> +       struct arg *a, *ret = NULL;
> +
> +       wl_list_for_each(a, &m->arg_list, link) {
> +               if (a->type == NEW_ID)
> +                       ret = a;
> +       }
> +       return ret;
> +}
> +
>  static void
>  emit_stubs(struct wl_list *message_list, struct interface *interface)
>  {
> @@ -688,11 +700,7 @@ emit_stubs(struct wl_list *message_list, struct
> interface *interface)
>                         continue;
>                 }
>
> -               ret = NULL;
> -               wl_list_for_each(a, &m->arg_list, link) {
> -                       if (a->type == NEW_ID)
> -                               ret = a;
> -               }
> +               ret = get_return_type(m);
>
>                 if (ret && ret->interface_name == NULL)
>                         printf("static inline void *\n");
> @@ -1103,8 +1111,7 @@ emit_types(struct protocol *protocol, struct wl_list
> *message_list)
>                         case NEW_ID:
>                         case OBJECT:
>                                 if (a->interface_name)
> -                                       printf("\t&%s_interface,\n",
> -                                              a->interface_name);
> +                                       printf("\t&%s_interface,\n",
> a->interface_name);
>                                 else
>                                         printf("\tNULL,\n");
>                                 break;
> @@ -1248,6 +1255,140 @@ emit_code(struct protocol *protocol)
>         }
>  }
>
> +static void
> +emit_inert_request(struct protocol *protocol, struct interface
> *interface, struct wl_list *message_list)
> +{
> +       struct message *m;
> +       struct arg *a, *ret;
> +       const char *newid_name;
> +
> +       wl_list_for_each(m, message_list, link) {
> +               ret = get_return_type(m);
> +
> +               /* forward declaration for the returned object */
> +               if (ret && ret->interface_name) {
> +                       printf("static void\n"
> +                                       "create_inert_%s(struct wl_client
> *client, uint32_t version, uint32_t id);\n"
> +                                       "\n",
> +                                       ret->interface_name);
> +               }
> +
> +               printf("static inline void inert_%s_%s(struct wl_client
> *client, struct wl_resource *resource",
> +                      interface->name, m->name
> +                      );
> +
> +               wl_list_for_each(a, &m->arg_list, link) {
> +                       if (a->type == NEW_ID) {
> +                               newid_name = a->name;
> +
> +                               if (a->interface_name == NULL) {
> +                                       printf(", const struct
> wl_interface *interface"
> +                                                  ", uint32_t version");
> +                                       continue;
> +                               }
> +                       }
> +
> +                       if (a->type == OBJECT) {
> +                               printf(", struct wl_resource *");
> +                       } else {
> +                               printf(", ");
> +                               emit_type(a);
> +                       }
> +                       printf("%s", a->name);
> +               }
> +
> +               printf(")\n"
> +                      "{\n");
> +
> +               if (ret && ret->interface_name) {
> +                       printf("\tcreate_inert_%s(client,
> wl_resource_get_version(resource), %s);\n",
> +                                       ret->interface_name, newid_name);
> +               } else if (m->destructor) {
> +                       printf("\twl_resource_destroy(resource);\n");
> +               }
> +               printf("}\n\n");
> +       }
> +}
> +
> +static void
> +emit_inert_interface(struct protocol *protocol, struct interface *i,
> struct wl_array *types) {
> +       struct message *m;
> +
> +
> +       if(wl_list_length(&i->request_list)) {
> +               emit_inert_request(protocol, i, &i->request_list);
> +
> +               printf ("static const struct %s_interface
> inert_%s_implementation = {\n",
> +                               i->name, i->name);
> +               wl_list_for_each(m, &i->request_list, link) {
> +                       printf("\tinert_%s_%s,\n", i->name, m->name);
> +               }
> +               printf ("};\n"
> +                               "\n");
> +       }
> +
> +       printf("static inline void\n"
> +                       "create_inert_%s(struct wl_client *client,
> uint32_t version, uint32_t id) {\n",
> +                       i->name
> +       );
> +
> +       if(wl_list_length(&i->request_list)) {
> +               /* emit the method body only when there is requests on the
> object */
> +               printf("\tstruct wl_resource *resource;\n"
> +                               "\tresource = wl_resource_create(client,
> &%s_interface, MIN(version, %d), id);\n"
> +
>  "\twl_resource_set_implementation(resource, &inert_%s_implementation,
> NULL, wl_resource_destroy);\n",
> +                               i->name, i->version, i->name);
> +       }
> +
> +       printf("}\n"
> +                       "\n");
> +
> +}
> +
> +
> +static
> +void emit_inert(struct protocol *protocol) {
> +       struct interface *i;
> +       struct wl_array types;
> +
> +       if (protocol->copyright)
> +               format_copyright(protocol->copyright);
> +
> +       printf("#ifndef __%s_SERVER_INERT__\n"
> +                       "#define __%s_SERVER_INERT__\n"
> +                       "\n"
> +                       "#include \"%s-server-protocol.h\"\n"
> +                       "\n"
> +                       "#ifdef  __cplusplus\n"
> +                       "extern \"C\" {\n"
> +                       "#endif\n"
> +                       "\n"
> +                       "#ifndef MIN\n"
> +                       "#define MIN(x,y) (((x) < (y)) ? (x) : (y))\n"
> +                       "#endif\n"
> +                       "\n",
> +                       protocol->uppercase_name,
> +                       protocol->uppercase_name,
> +                       //protocol->name,
> +                       protocol->name
> +                       );
> +
> +       wl_list_for_each(i, &protocol->interface_list, link) {
> +               if (!strcmp(i->name, "wl_registry") || !strcmp(i->name,
> "wl_display"))
> +                       continue;
> +
> +               emit_inert_interface(protocol, i, &types);
> +       }
> +
> +       printf("\n"
> +                       "#ifdef  __cplusplus\n"
> +                       "}\n"
> +                       "#endif\n"
> +                       "\n"
> +                       "#endif\n");
> +
> +}
> +
>  int main(int argc, char *argv[])
>  {
>         struct parse_context ctx;
> @@ -1257,17 +1398,26 @@ int main(int argc, char *argv[])
>         enum {
>                 CLIENT_HEADER,
>                 SERVER_HEADER,
> +               SERVER_INERT,
>                 CODE,
>         } mode;
>
> -       if (argc != 2)
> +       if (argc < 2) {
>                 usage(EXIT_FAILURE);
> -       else if (strcmp(argv[1], "help") == 0 || strcmp(argv[1], "--help")
> == 0)
> +       }
> +
> +       if (argc > 2) {
> +               stdin = fopen(argv[2], "r");
> +       }
> +
> +       if (strcmp(argv[1], "help") == 0 || strcmp(argv[1], "--help") == 0)
>                 usage(EXIT_SUCCESS);
>         else if (strcmp(argv[1], "client-header") == 0)
>                 mode = CLIENT_HEADER;
>         else if (strcmp(argv[1], "server-header") == 0)
>                 mode = SERVER_HEADER;
> +       else if (strcmp(argv[1], "server-inert") == 0)
> +               mode = SERVER_INERT;
>         else if (strcmp(argv[1], "code") == 0)
>                 mode = CODE;
>         else
> @@ -1317,6 +1467,9 @@ int main(int argc, char *argv[])
>                 case SERVER_HEADER:
>                         emit_header(&protocol, SERVER);
>                         break;
> +               case SERVER_INERT:
> +                       emit_inert(&protocol);
> +                       break;
>                 case CODE:
>                         emit_code(&protocol);
>                         break;
> diff --git a/wayland-scanner.mk b/wayland-scanner.mk
> index 0a72062..fd994e0 100644
> --- a/wayland-scanner.mk
> +++ b/wayland-scanner.mk
> @@ -6,3 +6,6 @@
>
>  %-client-protocol.h : $(wayland_protocoldir)/%.xml
>         $(AM_V_GEN)$(wayland_scanner) client-header < $< > $@
> +
> +%-server-inert.h: $(wayland_protocoldir)/%.xml
> +       $(AM_V_GEN)$(wayland_scanner) server-inert < $< > $@
> \ No newline at end of file
> --
> 1.9.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150312/8dc02362/attachment-0001.html>


More information about the wayland-devel mailing list