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

[Derek Foreman]

On 2017-07-26 03:32 PM, Quentin Glidic wrote:
> 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?

terminology, the efl terminal emulator.  Just attempting a corner drag 
to resize.

Others haven't hit this, so it may be the silly gamer mouse (1000hz 
update rate) I have plugged in causing the configures to come in too 
fast to keep up with.

> 
>> 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.

Well, the not present case is just going to result in the same list walk 
later to free the list?  I'll probably be lazy and land it as-is 
sometime tomorrow if nobody objects in the meantime. :)

Thanks for the speedy review,
Derek

> 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 =
>>
> 
>