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 <[email protected]>
---
  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 <[email protected]>
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 =




_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to