[PATCH weston] libweston-desktop/xdg-shell: Properly properly handle ack_configure

[Quentin Glidic]

On 7/26/17 9:39 PM, Derek Foreman wrote:
> commit 749637a8a306588964885fe6b25fda6087a84ccd
> introduced this feature, but the break is outside of any conditional
> so only the first item in the list is ever tested.
> 
> If a client skips a few configures and then acks the most recent
> it's still operating within spec, so the break should only occur
> when a match is found.

That is indeed the intended behaviour, I overlooked the conditionals.
Do you have a client that triggered it?


> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>   libweston-desktop/xdg-shell-v5.c | 2 +-
>   libweston-desktop/xdg-shell-v6.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libweston-desktop/xdg-shell-v5.c b/libweston-desktop/xdg-shell-v5.c
> index 77d004e1..32ece586 100644
> --- a/libweston-desktop/xdg-shell-v5.c
> +++ b/libweston-desktop/xdg-shell-v5.c
> @@ -481,8 +481,8 @@ weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client,
>   		} else if (configure->serial == serial) {
>   			wl_list_remove(&configure->link);
>   			found = true;
> +			break;
>   		}
> -		break;

I wanted to optimize the “serial is not in the list” case (since the 
list is ordered) but I failed. The wanted code is
else if (==) { found = true; break; } else break;

Anyway, this is:
Reviewed-by: Quentin Glidic <sardemff7+git at sardemff7.net>
either with the optimized break or not.

Thanks for catching this,

>   	}
>   	if (!found) {
>   		struct weston_desktop_client *client =
> diff --git a/libweston-desktop/xdg-shell-v6.c b/libweston-desktop/xdg-shell-v6.c
> index 1344dda0..04233e7f 100644
> --- a/libweston-desktop/xdg-shell-v6.c
> +++ b/libweston-desktop/xdg-shell-v6.c
> @@ -1106,8 +1106,8 @@ weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client,
>   		} else if (configure->serial == serial) {
>   			wl_list_remove(&configure->link);
>   			found = true;
> +			break;
>   		}
> -		break;
>   	}
>   	if (!found) {
>   		struct weston_desktop_client *client =
> 


-- 

Quentin “Sardem FF7” Glidic