I agree it's a hack, but it's also one that invalidates the protocol specification. If the wire protocol requires four arguments then the specification needs to reflect that. Currently if another tool or protocol implementation (like the gowl example previously mentioned) attempts to generate code from the specification they end up code that is not compatible with libwayland applications. In my opinion that's a pretty bad outcome for a hack, documented or not.
Attached is a patch that resolves the problem (at least it limits the scope of the hack to the wayland code generator) while maintaining the existing api. Thank you in advance for reviewing and considering this patch. Paul On Wed, Sep 3, 2014 at 11:04 PM, Jasper St. Pierre <[email protected]> wrote: > The fact that we have an undocumented hack like that in our scanner > clearly isn't great. We need to document it. > On Sep 3, 2014 8:55 PM, "Boyan Ding" <[email protected]> wrote: > >> Hi, >> >> It is actually not a fault in wayland, instead it is designed to be so. >> new_id's without interface specified in the protocol (such as the one in >> wl_registry::bind) must come with 3 arguments (s: interface name, u: >> version, n: the actual new_id). That's why 'n' turns into 'sun'. If a >> language binding generates the wrong code, please contact the author of >> the language binding and let him correct it. If you are changing how >> wayland-scanner (the official C code generator) works, the whole world >> will break before you. >> >> Refer to "How protocol objects are created" section in [1] to see the >> details. >> >> Regards, >> Boyan Ding >> >> [1] >> >> http://ppaalanen.blogspot.fi/2014/07/wayland-protocol-design-object-lifespan.html >> >> On Wed, 2014-09-03 at 22:32 -0500, Paul Sbarra wrote: >> > I tried to start some discussion on this topic previously, but it >> > apparently didn't make it through the moderator, so I'm trying again >> > having now joined the list. >> > >> > >> > I've recently taken an interest in the gowl implementation of the >> > wayland protocol and noticed that the specification doesn't match the >> > interface that gets generated by the wayland scanner. >> > >> > >> > If I attempt to code-gen gowl using the wayland.xml file the >> > wl_registry bind interface is missing a couple arguments that weston >> > expects, resulting in the following runtime error: >> > libwayland: message too short, object (2), message bind(usun) >> > >> > >> > However, the spec indicates a bind request signature of "un". >> > >> > >> > I tracked this down into some curious logic in the scanner and have >> > been working on a patch to try and allow the corrected signature to be >> > specified in the protocol. Unfortunately this has become more >> > interesting then I'd anticipated. >> > >> > >> > Attached is my naive attempt at resolving this issue. I'm horrified >> > by the string->interface lookup and the wl_registry_bind api change >> > but I'm not sure what else one can do. Any feedback or additional >> > help would be appreciated. >> > >> > >> > Note that this patch applies to the 1.5.91 tagged commit and compiles >> > cleanly (for wayland). Weston didn't like having the lookup function >> > defined in multiple files, so maybe there would be a better place to >> > put such functionality. >> > >> > Thanks, >> > Paul >> > >> > _______________________________________________ >> > wayland-devel mailing list >> > [email protected] >> > http://lists.freedesktop.org/mailman/listinfo/wayland-devel >> >> >> >> _______________________________________________ >> wayland-devel mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/wayland-devel >> >
From cbec181428e6b4c296075e65f02631c4f518a7df Mon Sep 17 00:00:00 2001 From: Paul Sbarra <[email protected]> Date: Thu, 4 Sep 2014 21:21:26 -0500 Subject: [PATCH] update specification to match the wire protocol This change updates the code generator so it does not insert undocumented arguments when implementing the protocol. This discrepency prevents alternative implementations from being able to auto generate their protocol code from the wayland.xml specification. Note that some concessions are made to prevent an api change. Specifically, wl_register_bind requires a wl_interface* argument, but there is no mechanism to specify that type in the specification. The code generator would auto-insert this type. In order to continue to provide the same external interface some non-ideal checks had to be made to modify the proper arguments. This change limits the scope of a hack that broke the protocol specification to one that only affects the wayland code generator. Signed-off-by: Paul Sbarra <[email protected]> --- protocol/wayland.xml | 2 ++ src/scanner.c | 22 +++++----------------- 2 files changed, 7 insertions(+), 17 deletions(-) diff --git a/protocol/wayland.xml b/protocol/wayland.xml index bb457bc..caaba68 100644 --- a/protocol/wayland.xml +++ b/protocol/wayland.xml @@ -130,6 +130,8 @@ specified name as the identifier. </description> <arg name="name" type="uint" summary="unique name for the object"/> + <arg name="interface" type="string" summary="name of the object's interface"/> + <arg name="version" type="uint" summary="version of the object's interface"/> <arg name="id" type="new_id"/> </request> diff --git a/src/scanner.c b/src/scanner.c index 72fd3e8..e09e77c 100644 --- a/src/scanner.c +++ b/src/scanner.c @@ -356,7 +356,7 @@ start_element(void *data, const char *element_name, const char **atts) ctx->protocol->uppercase_name = uppercase_dup(name); ctx->protocol->description = NULL; } else if (strcmp(element_name, "copyright") == 0) { - + } else if (strcmp(element_name, "interface") == 0) { if (name == NULL) fail(&ctx->loc, "no interface name given"); @@ -704,9 +704,8 @@ emit_stubs(struct wl_list *message_list, struct interface *interface) interface->name, interface->name); wl_list_for_each(a, &m->arg_list, link) { - if (a->type == NEW_ID && a->interface_name == NULL) { - printf(", const struct wl_interface *interface" - ", uint32_t version"); + if (strncmp(m->name, "bind", 5) == 0 && a->type == STRING && a->interface_name == NULL) { + printf(", const struct wl_interface *interface"); continue; } else if (a->type == NEW_ID) continue; @@ -741,9 +740,9 @@ emit_stubs(struct wl_list *message_list, struct interface *interface) wl_list_for_each(a, &m->arg_list, link) { if (a->type == NEW_ID) { - if (a->interface_name == NULL) - printf(", interface->name, version"); printf(", NULL"); + } else if (strncmp(m->name, "bind", 5) == 0 && a->type == STRING) { + printf(", interface->name"); } else { printf(", %s", a->name); } @@ -884,13 +883,6 @@ emit_structs(struct wl_list *message_list, struct interface *interface, enum sid desc_dump(mdesc ? mdesc->summary : "(none)", "\t * %s - ", m->name); wl_list_for_each(a, &m->arg_list, link) { - - if (side == SERVER && a->type == NEW_ID && - a->interface_name == NULL) - printf("\t * @interface: name of the objects interface\n" - "\t * @version: version of the objects interface\n"); - - desc_dump(a->summary ? a->summary : "(none)", "\t * @%s: ", a->name); } @@ -920,8 +912,6 @@ emit_structs(struct wl_list *message_list, struct interface *interface, enum sid if (side == SERVER && a->type == OBJECT) printf("struct wl_resource *"); - else if (side == SERVER && a->type == NEW_ID && a->interface_name == NULL) - printf("const char *interface, uint32_t version, uint32_t "); else if (side == CLIENT && a->type == OBJECT && a->interface_name == NULL) printf("void *"); @@ -1144,8 +1134,6 @@ emit_messages(struct wl_list *message_list, printf("i"); break; case NEW_ID: - if (a->interface_name == NULL) - printf("su"); printf("n"); break; case UNSIGNED: -- 2.1.0
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
