[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freedesktop-xorg-devel
Subject:    Re: [PATCH] xwayland: fix access to invalid pointer
From:       Daniel Stone <daniel () fooishbar ! org>
Date:       2018-08-28 21:10:42
Message-ID: CAPj87rMDoUAjsv0uc4USOGO4+-FTx2JJu6YO9Me-iMgV=dzpfQ () mail ! gmail ! com
[Download RAW message or body]

Hi,

On Tue, 28 Aug 2018 at 21:30, Lionel Landwerlin
<lionel.g.landwerlin@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@collabora.com>

Cheers,
Daniel
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[prev in list] [next in list] [prev in thread] [next in thread]