[weston] input: Use slightly less obscure logic in evdev_notify_keyboard_focus()

Submitted by Derek Foreman on Nov. 17, 2014, 2:47 p.m.

Details

Message ID 1416235664-30604-1-git-send-email-derekf@osg.samsung.com
State Superseded
Headers show

Not browsing as part of any series.

Commit Message

Derek Foreman Nov. 17, 2014, 2:47 p.m.
While the test is actually correct (for non-negative numbers), it's not
at all clear and seems to be an accidental order of operations mistake.

Also, add an assert() to make sure this number is never negative.

Closes bug 86346 - https://bugs.freedeskto.org/show_bug.cgi?id=86346

Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
---
 src/input.c           | 1 +
 src/libinput-device.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/input.c b/src/input.c
index 5f19875..6784ead 100644
--- a/src/input.c
+++ b/src/input.c
@@ -2209,6 +2209,7 @@  weston_seat_release_keyboard(struct weston_seat *seat)
 		weston_keyboard_reset_state(seat->keyboard);
 		seat_send_updated_caps(seat);
 	}
+	assert(seat->keyboard_device_count >= 0);
 }
 
 WL_EXPORT void
diff --git a/src/libinput-device.c b/src/libinput-device.c
index 0e3f46d..8a48905 100644
--- a/src/libinput-device.c
+++ b/src/libinput-device.c
@@ -470,7 +470,7 @@  evdev_notify_keyboard_focus(struct weston_seat *seat,
 {
 	struct wl_array keys;
 
-	if (!seat->keyboard_device_count > 0)
+	if (seat->keyboard_device_count == 0)
 		return;
 
 	wl_array_init(&keys);

Comments

On 17 November 2014 15:47, Derek Foreman <derekf@osg.samsung.com> wrote:

> While the test is actually correct (for non-negative numbers), it's not
> at all clear and seems to be an accidental order of operations mistake.
>
> Also, add an assert() to make sure this number is never negative.
>
> Closes bug 86346 - https://bugs.freedeskto.org/show_bug.cgi?id=86346
>
> Signed-off-by: Derek Foreman <derekf@osg.samsung.com>
> ---
>  src/input.c           | 1 +
>  src/libinput-device.c | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/input.c b/src/input.c
> index 5f19875..6784ead 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -2209,6 +2209,7 @@ weston_seat_release_keyboard(struct weston_seat
> *seat)
>                 weston_keyboard_reset_state(seat->keyboard);
>                 seat_send_updated_caps(seat);
>         }
> +       assert(seat->keyboard_device_count >= 0);
>

Maybe this assert could be right after decrementing of the
keyboard_device_count? But I don't insist on it. Better here than nowhere :)


>  }
>
>  WL_EXPORT void
> diff --git a/src/libinput-device.c b/src/libinput-device.c
> index 0e3f46d..8a48905 100644
> --- a/src/libinput-device.c
> +++ b/src/libinput-device.c
> @@ -470,7 +470,7 @@ evdev_notify_keyboard_focus(struct weston_seat *seat,
>  {
>         struct wl_array keys;
>
> -       if (!seat->keyboard_device_count > 0)
> +       if (seat->keyboard_device_count == 0)
>

This condition is not equal to the former one (if the former one would be
written correctly), but is correct IMO. Together with the
assert this should work perfectly.

                return;
>
>         wl_array_init(&keys);
> --
> 2.1.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>

Reviewed-by: Marek Chalupa <mchqwerty@gmail.com>