[PATCH weston] desktop-shell: Fix a crash when right-clicking the panel

Derek Foreman derekf at osg.samsung.com
Mon Apr 6 12:33:25 PDT 2015


This bug was introduced in commits da6ecd0cc52 and 24185e2561

I guess nobody right clicked on the panel for over a month. :)

I've CC'd Jasper and Jonas in case they haven't noticed this yet...

On 06/04/15 01:52 AM, Dima Ryazanov wrote:
> It looks like the error-checking code assumes the popup's parent is
> a shell surface - but that's not always the case.
> 
> Signed-off-by: Dima Ryazanov <dima at gmail.com>
> ---
>  desktop-shell/shell.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
> index f7c928e..0159547 100644
> --- a/desktop-shell/shell.c
> +++ b/desktop-shell/shell.c
> @@ -3392,7 +3392,7 @@ add_popup_grab(struct shell_surface *shsurf,
>  
>  	parent = get_shell_surface(shsurf->parent);
>  	top_surface = get_top_popup(shseat);
> -	if (shell_surface_is_xdg_popup(shsurf) &&
> +	if (parent && shell_surface_is_xdg_popup(shsurf) &&

This part broke in da6ecd0cc52

Your patch definitely stops the crash for me, which is good.  :)

I think this part's ok, but I'm not 100% sure we're still going to send
the new not-top-level-popup error when we're supposed to now...

>  	    ((top_surface == NULL && !shell_surface_is_xdg_surface(parent)) ||
>  	     (top_surface != NULL && parent != top_surface))) {
>  		wl_resource_post_error(shsurf->owner->resource,
> @@ -4098,12 +4098,13 @@ create_xdg_popup(struct shell_client *owner, void *shell,
>  {
>  	struct shell_surface *shsurf, *parent_shsurf;
>  
> -	/* Verify that we are creating the top most popup when mapping,
> -	 * as its not until then we know whether it was mapped as most
> +	/* Verify that we are creating the topmost popup when mapping,
> +	 * as it's not until then we know whether it was mapped as most
>  	 * top level or not. */

The grammar fix-ups look good to me.

>  	parent_shsurf = get_shell_surface(parent);
> -	if (!shell_surface_is_xdg_popup(parent_shsurf) &&
> +	if (parent_shsurf &&

This part broke in 24185e2561...

I wonder if we should be doing a more comprehensive test here - it looks
like some invalid parent resources could get past this test?

> +	    !shell_surface_is_xdg_popup(parent_shsurf) &&
>  	    !shell_surface_is_xdg_surface(parent_shsurf)) {
>  		wl_resource_post_error(owner->resource,
>  				       XDG_POPUP_ERROR_INVALID_PARENT,
> 




More information about the wayland-devel mailing list