[PATCH] shell: fix set_transient

[Pekka Paalanen]

> On Thu, 8 Dec 2011 13:05:42 -0500
> Kristian Høgsberg <krh at bitplanet.net> wrote:
> 
> > On Thu, Dec 8, 2011 at 10:08 AM, Pekka Paalanen
> > <ppaalanen at gmail.com> wrote:
> > > On Thu, 08 Dec 2011 17:04:47 +0200
> > > Tiago Vignatti <tiago.vignatti at linux.intel.com> wrote:
> ...
> > >> but I thought you'd implement the explicit cast scheme.. meh.
> > >
> > > Sorry, not today :-D
> > >
> > > I'll keep it in mind, though, for when I have time for it.
> > >
> > > For reference to others: it is for improving type safety in the
> > > compositor side, by putting some real pointer types into struct
> > > wl_*_interface's function arguments.
> > 
> > The question there is, what types?  We could make a struct
> > wl_compositor_resource that wraps the generic wl_resource, and then
> > move the void *data out of wl_resource and into
> > wl_compsitor_resource as a typesafe pointer.  To what though?  We
> > could make it struct wl_compositor * and just require that the
> > compositor use those types. I dunno, it just seems messy and I'm
> > not sure it's worth it.

I just had another idea, but I'm not sure if it is better or worse.

It is about playing with typedefs, here is an ad hoc example with
wl_shell_surface.

On the core lib side, we use a type placeholder instead of any real
type.


--- wayland-server-protocol.h	2011-12-09 14:09:03.000000000 +0200
+++ wayland-server-protocol.h.mod	2011-12-09 14:00:55.000000000 +0200
@@ -249,7 +249,7 @@ struct wl_shell_surface_interface {
 			      int32_t y,
 			      uint32_t flags);
 	void (*set_fullscreen)(struct wl_client *client,
-			       struct wl_resource *resource);
+			       struct_wl_shell_surface *resource);
 };
 
 #define WL_SHELL_SURFACE_CONFIGURE	0


Ok, so it's not 'resource' anymore, and we need to pass
wl_resource::data instead when we actually call it.

For that to compile, we need a binary compatible dummy definition in
the core library, private.


diff --git a/src/wayland-private.h b/src/wayland-private.h
index fada571..d65db2f 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -27,6 +27,9 @@
 #include <stdarg.h>
 #include "wayland-util.h"
 
+/* Dummy type definitions for just compiling the core lib */
+typedef void struct_wl_shell_surface;
+
 #define WL_ZOMBIE_OBJECT ((void *) 2)
 
 #define WL_MAP_SERVER_SIDE 0


Then, in the real compositor, we define it to be a proper type.


diff --git a/compositor/compositor.h b/compositor/compositor.h
index 3c88807..15e1a3b 100644
--- a/compositor/compositor.h
+++ b/compositor/compositor.h
@@ -23,6 +23,9 @@
 #ifndef _WAYLAND_SYSTEM_COMPOSITOR_H_
 #define _WAYLAND_SYSTEM_COMPOSITOR_H_
 
+/* The real type definitions */
+typedef struct shell_surface struct_wl_shell_surface;
+
 #include <libudev.h>
 #include <pixman.h>
 #include <wayland-server.h>


Then, in our callback definition, we can directly use the correct type.


diff --git a/compositor/shell.c b/compositor/shell.c
index b745693..7a72534 100644
--- a/compositor/shell.c
+++ b/compositor/shell.c
@@ -402,10 +401,9 @@ get_default_output(struct wlsc_compositor *compositor)
 
 static void
 shell_surface_set_fullscreen(struct wl_client *client,
-                            struct wl_resource *resource)
+                            struct shell_surface *shsurf)
 
 {
-       struct shell_surface *shsurf = resource->data;
        struct wlsc_surface *es = shsurf->surface;
        struct wlsc_output *output;
 

And this actually compiles without warnings, I tried it.

Now, the ugly part is, wayland-server-protocol.h is not compileable
without either a dummy or a proper typedef. Also, can we trust on the
binary compatibility, that if library is compiled with (void *), and
same datum is compiled as (struct foo *) in another compilation unit,
that the implicit-hidden pointer cast works?

With this idea, we avoid the intermediate types in casts, the explicit
casts and inline cast wrapper functions, and yet do not have to teach
wayland-scanner about our implementation-spefic types.

Another downside will come with the shell plugins, they would have to
agree on types, have them all in a common header for the core protocol.

Yet further upside with typedefs is, that we could start generating
server side type safe inline wrappers for all wl_resource_post_event()
calls instead of the current varargs dance with absolutely no type
safety.

What do you think?


- pq