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

Landed, with your optimization.  We discussed it at length on irc and 
while I don't feel it's a big performance gain, it's correct, utterly 
trivial and doesn't detract at all from the readability of the code.

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