[PATCH] xwayland: fix access to invalid pointer

Daniel Stone daniel at fooishbar.org
Tue Aug 28 21:10:42 UTC 2018


Hi,

On Tue, 28 Aug 2018 at 21:30, Lionel Landwerlin
<lionel.g.landwerlin at intel.com> wrote:
> xwl_output->randr_crtc is used in the update_screen_size() function :
>
> [...]
>
> @@ -392,14 +392,15 @@ xwl_output_remove(struct xwl_output *xwl_output)
>      int width = 0, height = 0;
>      Bool need_rotate = (xwl_output->xdg_output == NULL);
>
> -    RRCrtcDestroy(xwl_output->randr_crtc);
> -    RROutputDestroy(xwl_output->randr_output);
>      xorg_list_del(&xwl_output->link);
>
>      xorg_list_for_each_entry(it, &xwl_screen->output_list, link)
>          output_get_new_size(it, need_rotate, &height, &width);
>      update_screen_size(xwl_output, width, height);
>
> +    RRCrtcDestroy(xwl_output->randr_crtc);
> +    RROutputDestroy(xwl_output->randr_output);
> +
>      xwl_output_destroy(xwl_output);

I counter-suggested a patch on IRC, which keeps the RandR object
destroy lines where they are originally, and just tries to pick any
other output in the loop for output_get_new_size().

That makes the code even less obvious though, and the only benefit is if:
  - you have two outputs connected with the same pixel dimensions but
different physical dimensions
  - you disconnect one of them
  - you want newly-connected legacy clients to get the correct DPI
value through the connection handshake

I'm finding that usecase hard to care enough about that to justify how
ugly it is compared to this, so this patch is:
Reviewed-by: Daniel Stone <daniels at collabora.com>

Cheers,
Daniel


More information about the xorg-devel mailing list