Patchwork monitor: fix the wrong order of releasing keys

login
register
mail settings
Submitter Amos Kong
Date April 16, 2013, 5:47 a.m.
Message ID <1366091252-16802-1-git-send-email-akong@redhat.com>
Download mbox | patch
Permalink /patch/236836/
State New
Headers show

Comments

Amos Kong - April 16, 2013, 5:47 a.m.
(qemu) sendkey ctrl_r-scroll_lock-scroll_lock

Executing this command could not let Windows guest panic, it caused by
the wrong order of releasing keys. This problem was introduced by
commit e4c8f004c55d9da3eae3e14df740238bf805b5d6.

The right release order should be starting from last item.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 ui/input.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
Markus Armbruster - April 16, 2013, 7:08 a.m.
Amos Kong <akong@redhat.com> writes:

> (qemu) sendkey ctrl_r-scroll_lock-scroll_lock
>
> Executing this command could not let Windows guest panic, it caused by

"could not"?

> the wrong order of releasing keys. This problem was introduced by
> commit e4c8f004c55d9da3eae3e14df740238bf805b5d6.
>
> The right release order should be starting from last item.
>
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  ui/input.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/ui/input.c b/ui/input.c
> index 9abef0c..ecfeb43 100644
> --- a/ui/input.c
> +++ b/ui/input.c
> @@ -234,13 +234,11 @@ static void free_keycodes(void)
>  
>  static void release_keys(void *opaque)
>  {
> -    int i;
> -
> -    for (i = 0; i < keycodes_size; i++) {
> -        if (keycodes[i] & 0x80) {
> +    while (keycodes_size > 0) {
> +        if (keycodes[--keycodes_size] & 0x80) {
>              kbd_put_keycode(0xe0);
>          }
> -        kbd_put_keycode(keycodes[i]| 0x80);
> +        kbd_put_keycode(keycodes[keycodes_size] | 0x80);
>      }
>  
>      free_keycodes();
Luiz Capitulino - April 17, 2013, 1:21 p.m.
On Tue, 16 Apr 2013 13:47:32 +0800
Amos Kong <akong@redhat.com> wrote:

> (qemu) sendkey ctrl_r-scroll_lock-scroll_lock
> 
> Executing this command could not let Windows guest panic, it caused by
> the wrong order of releasing keys. This problem was introduced by
> commit e4c8f004c55d9da3eae3e14df740238bf805b5d6.
> 
> The right release order should be starting from last item.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>

Applied to the qmp branch, thanks.

> ---
>  ui/input.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/ui/input.c b/ui/input.c
> index 9abef0c..ecfeb43 100644
> --- a/ui/input.c
> +++ b/ui/input.c
> @@ -234,13 +234,11 @@ static void free_keycodes(void)
>  
>  static void release_keys(void *opaque)
>  {
> -    int i;
> -
> -    for (i = 0; i < keycodes_size; i++) {
> -        if (keycodes[i] & 0x80) {
> +    while (keycodes_size > 0) {
> +        if (keycodes[--keycodes_size] & 0x80) {
>              kbd_put_keycode(0xe0);
>          }
> -        kbd_put_keycode(keycodes[i]| 0x80);
> +        kbd_put_keycode(keycodes[keycodes_size] | 0x80);
>      }
>  
>      free_keycodes();
Luiz Capitulino - April 17, 2013, 1:28 p.m.
On Tue, 16 Apr 2013 09:08:12 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Amos Kong <akong@redhat.com> writes:
> 
> > (qemu) sendkey ctrl_r-scroll_lock-scroll_lock
> >
> > Executing this command could not let Windows guest panic, it caused by
> 
> "could not"?

You can use Windows' editor to setup Windows to panic on a key sequence, and
that feature (or Windows itself) is sensible to the key release ordering.

This caught a problem with the sendkey command conversion to the qapi: we
changed the release key ordering from last to first to first to last.

> > the wrong order of releasing keys. This problem was introduced by
> > commit e4c8f004c55d9da3eae3e14df740238bf805b5d6.
> >
> > The right release order should be starting from last item.
> >
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  ui/input.c |    8 +++-----
> >  1 files changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/ui/input.c b/ui/input.c
> > index 9abef0c..ecfeb43 100644
> > --- a/ui/input.c
> > +++ b/ui/input.c
> > @@ -234,13 +234,11 @@ static void free_keycodes(void)
> >  
> >  static void release_keys(void *opaque)
> >  {
> > -    int i;
> > -
> > -    for (i = 0; i < keycodes_size; i++) {
> > -        if (keycodes[i] & 0x80) {
> > +    while (keycodes_size > 0) {
> > +        if (keycodes[--keycodes_size] & 0x80) {
> >              kbd_put_keycode(0xe0);
> >          }
> > -        kbd_put_keycode(keycodes[i]| 0x80);
> > +        kbd_put_keycode(keycodes[keycodes_size] | 0x80);
> >      }
> >  
> >      free_keycodes();
>

Patch

diff --git a/ui/input.c b/ui/input.c
index 9abef0c..ecfeb43 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -234,13 +234,11 @@  static void free_keycodes(void)
 
 static void release_keys(void *opaque)
 {
-    int i;
-
-    for (i = 0; i < keycodes_size; i++) {
-        if (keycodes[i] & 0x80) {
+    while (keycodes_size > 0) {
+        if (keycodes[--keycodes_size] & 0x80) {
             kbd_put_keycode(0xe0);
         }
-        kbd_put_keycode(keycodes[i]| 0x80);
+        kbd_put_keycode(keycodes[keycodes_size] | 0x80);
     }
 
     free_keycodes();