[PATCH] shell: fix set_transient
ppaalanen at gmail.com
Thu Dec 8 23:56:31 PST 2011
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>
> > 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.
Yes, it's more difficult than it first appeared, I thought about it a
bit more on my way home yesterday. My idea was something like this:
If there is a callback parameter that has the type of wl_shell_surface
in the protocol, we could make the argument type in the C function
signature as (struct wl_shell_surface *). The client side already uses
these types as opaque pointers.
The trick is, we cannot pass the (struct wl_resource *) directly there,
we need to pass wl_resource->data, cast into (struct wl_shell_surface
This has the following implications:
- We always have to use the wl_surface::data member to point to
something that allows to access the underlying struct wl_resource.
Accessing the wl_resource becomes a little more verbose.
- callback boilerplate would be:
shell_set_foo(struct wl_client *client,
struct wl_shell_surface *shell_surface)
struct shell_surface *shsurf =
(struct shell_surface *)shell_surface;
which shows the benefit: we still have a cast, but the cast is
explicit. The problem I am trying to solve, is that wl_resource::data
is a void* which can be implicitly cast to whatever.
We could take the type safety one step further, and define private
helpers in e.g. shell.c in this case:
static inline struct shell_surface *
to_shell_surface(struct wl_shell_surface *p)
return (struct shell_surface *)p;
That would allow explicit casting from a single type to a single type,
the compiler giving out warnings about type mismatches on all other
- if the protocol changes and you forget to change your callback
function signature (because the function signatures in wl_*_interface
structs are autogenerated from the protocol and no longer match)
- you change the function signature, but forget to change the cast
(because the inline helper function types no longer match)
I have not tried to actually implement this yet, so there's probably
more stuff I have overlooked.
Do you think this idea would be good to try?
More information about the wayland-devel