Re: Wayland specification doesn't match code generation

2014-09-04 Thread Pekka Paalanen
On Thu, 04 Sep 2014 12:17:12 +0800
Boyan Ding stu_...@126.com wrote:

 On Wed, 2014-09-03 at 21:04 -0700, Jasper St. Pierre wrote:
  The fact that we have an undocumented hack like that in our scanner
  clearly isn't great. We need to document it.
 
 I totally agree. This can really confuse every beginner.

And now we have a bug about it:
https://bugs.freedesktop.org/show_bug.cgi?id=83478


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Wayland specification doesn't match code generation

2014-09-04 Thread Pekka Paalanen
On Wed, 3 Sep 2014 22:32:02 -0500
Paul Sbarra sbarra.p...@gmail.com 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
 https://github.com/sebastianskejoe/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

No, you need to fix the generator instead, indeed.

For the record, see:
http://cgit.freedesktop.org/wayland/wayland/commit/?id=85a6a470873357089ffb968a176d5074fddd1756
That happened before Wayland 1.0.

Also somewhat relatedly, the very next commit:
http://cgit.freedesktop.org/wayland/wayland/commit/?id=8872956dfd43d36e4165d15cb50d8ef4f81fbe0d

And then the next commit introduces wl_registry which is what we use
today:
http://cgit.freedesktop.org/wayland/wayland/commit/?id=9fe75537ad207c1496e6d9be41a8f5af4b876506


Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Wayland specification doesn't match code generation

2014-09-03 Thread Paul Sbarra
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
https://github.com/sebastianskejoe/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
From 4a5dfecc051c920835032b64434096f9403bd4f7 Mon Sep 17 00:00:00 2001
From: Paul Sbarra sbarra.p...@gmail.com
Date: Wed, 3 Sep 2014 22:09:52 -0500
Subject: [PATCH] explicitly specify all registry bind request arguments

---
 protocol/wayland.xml|  5 -
 src/scanner.c   | 58 +
 tests/test-compositor.c |  2 +-
 3 files changed, 39 insertions(+), 26 deletions(-)

diff --git a/protocol/wayland.xml b/protocol/wayland.xml
index bb457bc..e8a14f0 100644
--- a/protocol/wayland.xml
+++ b/protocol/wayland.xml
@@ -127,9 +127,12 @@
 request name=bind
   description summary=bind an object to the display
 	Binds a new, client-created object to the server using the
-specified name as the identifier.
+   specified name as the identifier, and it implements the
+   given version of the given interface.
   /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..a11ad45 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,11 +704,7 @@ 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);
-continue;
-			} else if (a-type == NEW_ID)
+			if (a-type == NEW_ID)
 continue;
 			printf(, );
 			emit_type(a);
@@ -727,8 +723,15 @@ emit_stubs(struct wl_list *message_list, struct interface *interface)
 			   interface-uppercase_name,
 			   m-uppercase_name);
 
-			if (ret-interface_name == NULL)
-printf(interface);
+			if (ret-interface_name == NULL) {
+printf(string_to_interface();
+  		wl_list_for_each(a, m-arg_list, link) {
+  			if (a-type == STRING) {
+  printf(%s), a-name);
+  break;
+   }
+}
+ }
 			else
 printf(%s_interface, ret-interface_name);
 		} else {
@@ -741,8 +744,6 @@ 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 {
 printf(, %s, a-name);
@@ -885,12 +886,6 @@ emit_structs(struct wl_list *message_list, struct interface *interface, enum sid
 			  \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 +915,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 

Re: Wayland specification doesn't match code generation

2014-09-03 Thread Boyan Ding
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
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel



___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Wayland specification doesn't match code generation

2014-09-03 Thread Jasper St. Pierre
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 stu_...@126.com 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
  wayland-devel@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/wayland-devel



 ___
 wayland-devel mailing list
 wayland-devel@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/wayland-devel

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: Wayland specification doesn't match code generation

2014-09-03 Thread Boyan Ding
On Wed, 2014-09-03 at 21:04 -0700, Jasper St. Pierre wrote:
 The fact that we have an undocumented hack like that in our scanner
 clearly isn't great. We need to document it.

I totally agree. This can really confuse every beginner.



___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel