[PATCH wayland] server: don't send an error to NULL display_resource

[Marek Chalupa]

Hi,

On 12/07/2015 06:54 PM, Bill Spitzak wrote:
>
>
> On Mon, Dec 7, 2015 at 12:42 AM, Marek Chalupa <mchqwerty at gmail.com
> <mailto:mchqwerty at gmail.com>> wrote:
>
>
>     diff --git a/src/wayland-server.c b/src/wayland-server.c
>     index 1364d5d..b372aa9 100644
>     --- a/src/wayland-server.c
>     +++ b/src/wayland-server.c
>     @@ -511,6 +511,14 @@ wl_client_get_object(struct wl_client *client,
>     uint32_t id)
>       WL_EXPORT void
>       wl_client_post_no_memory(struct wl_client *client)
>       {
>     +       /* display_resource is destroyed first upon client's destruction
>     +        * If some resource destructor calls wl_client_post_no_memory()
>     +        * (why it would do it? you never know...), we would pass NULL
>     +        * here as a resource to the wl_resource_post_error
>     +        * and we don't want that */
>     +       if (!client->display_resource)
>     +               return;
>     +
>              wl_resource_post_error(client->display_resource,
>                                     WL_DISPLAY_ERROR_NO_MEMORY, "no
>     memory");
>       }
>     @@ -779,7 +787,8 @@ bind_display(struct wl_client *client, struct
>     wl_display *display)
>              client->display_resource =
>                      wl_resource_create(client, &wl_display_interface,
>     1, 1);
>              if (client->display_resource == NULL) {
>     -               wl_client_post_no_memory(client);
>     +               /* Don't send error to client -
>     +                * what resource we would use anyway? */
>                      return -1;
>              }
>
>
> Seems like you put two fixes in this, one of them is redundant. My guess
> is that you want the second change and not the first, but I'm not really
> certain about it.

Yes, these are two fixes and either of them would fix the possible 
dereference. I agree that the later is the one I'd choose to fix this 
particular dereference coming from wl_resource_create() returning NULL 
in bind_display(). But there is another scenario possible: if a destroy 
signal handler or resource destructor would call 
wl_client_post_no_memory(), than there is the very same dereference of 
NULL, so I included the other check too.
You can view it as a fix of dereference (former part) and removal of 
incorrect code (later part).

Thanks,
Marek

>
> Also it is inconsistent about using !x v.s. x==NULL.
>
>
>