[PATCH] wayland-client: Initialize newly created wl_proxys to zero.

[Pekka Paalanen]

On Fri, 08 Aug 2014 18:31:25 +0200
"Nils Chr. Brause" <nilschrbrause at gmail.com> wrote:

> Up until now, newly created wl_proxys (with proxy_create or
> wl_proxy_create_for_id) are not initialized properly after memory
> allocation. The wl_display object in contrast is. To prevent giving
> uninitialized data to the user (e.g. user_data) an appropriate memset
> has been added.
> 
> Signed-off-by: Nils Chr. Brause <nilschrbrause at googlemail.com>
> ---
>  src/wayland-client.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 3e401d3..8006581 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -274,6 +274,8 @@ proxy_create(struct wl_proxy *factory, const struct
> wl_interface *interface)
>      if (proxy == NULL)
>          return NULL;
>  
> +    memset(proxy, 0, sizeof *proxy);
> +
>      proxy->object.interface = interface;
>      proxy->object.implementation = NULL;
>      proxy->dispatcher = NULL;
> @@ -331,6 +333,8 @@ wl_proxy_create_for_id(struct wl_proxy *factory,
>      if (proxy == NULL)
>          return NULL;
>  
> +    memset(proxy, 0, sizeof *proxy);
> +
>      proxy->object.interface = interface;
>      proxy->object.implementation = NULL;
>      proxy->object.id = id;
> -- 
> 2.0.4

I see nothing wrong here, memset to zero is a usual pattern, and
wl_proxy is a complex enough type, that checking every member is
set is tedious. Therefore:

Reviewed-by: Pekka Paalanen <ppaalanen at gmail.com>

But I do wonder, if it ever makes sense to check user data for
NULL, and set if it is NULL. If user_data is NULL, how can you
ever be sure, that the wl_proxy was created by your code, and not
by some library you use?

Even when user_data is not NULL, you should have some explicit way
of checking whether the pointer is yours or not.

An example: you use a library and you passed the wl_display to it.
This library goes and creates a wl_surface of its own. This
wl_surface gets targeted by input, so your client receives e.g.
wl_pointer.enter. In the wl_pointer.enter handler, you now suddenly
have a wl_proxy for a wl_surface that you did not create, and
therefore the user_data is nothing you would expect. If you are
lucky, your program crashes here.

The problems emerges in all event handlers, where the event carries
an object as an argument.


Thanks,
pq