About wl_guard again

[Pekka Paalanen]

On Tue, 20 Mar 2012 16:07:26 -0400
Kristian Hoegsberg <hoegsberg at gmail.com> wrote:

> We really need that wl_guard thing that Pekka suggested now...  too
> many destroy listeners that just set a pointer to NULL.

Hi Kristian,

before you go ahead, I'd like to do a comparison here with my first
proposal:
http://lists.freedesktop.org/archives/wayland-devel/2012-March/002449.html

All the naming issues aside...

> We didn't find a really good way to make it both typesafe and
> convenient, but I think I prefer the approach where wl_guard manages a
> wl_resource pointer internally the best.  We put the wl_resource
> pointer and the wl_listener in a wl_guard struct:
> 
>   struct wl_resource_ref {
>     struct wl_resource *resource;
>     struct wl_listener listener;
>   };

The struct size is the same for both of us.

> and then we can just say wl_resource_ref_set(ref, resource).  If we
> want a typesafe pointer as well, we have to add it in the containing
> struct and set it from resource->data:
> 
>   wd->saved_kbd_focus = es;
>   wl_guard_set(&wd->saved_kbd_guard, &es->surface.resource);

You have easier API here by one function parameter less, otherwise the
same.

> The wl_resource_ref will not clear the guard->resource pointer, but the
> typesafe pointer, so access to the typesafe pointer has to be guarded
> by checking ref->resource != NULL:
> 
>   es = wd->saved_kbd_focus;
>   if (wd->saved_kbd_guard.resource) {
>     /* do stuff with es */
>   }

Here you have to inspect and add the extra check to every existing check
of the guarded typesafe pointer. This puts the maintenance burden on
users of the pointer, instead of the manager (the owner of the typesafe
pointer). This requires the knowledge of which guard is related to
which typesafe pointer in all users, as opposed to recording it once
when the manager sets or clears the pointer.

Your proposal changes the way of using the typesafe pointers, my
proposal is a little uglier, but lets the users go unmodified.

We could have helpers like
void wl_input_device_set_pointer_focus(struct wl_input_device *device,
                                       struct wl_surface *surface)
that would handle re-setting and setting to NULL properly, to confine
the "which guard to use" knowledge in truly a single point. (Ok, that
particular example is a bit poor because such function already exists
for other purposes.)

The call is yours, of course, just an idea.


Thanks,
pq