[Qemu-devel] Re: [PATCH 4/4] kbd keds: vnc

[Juan Quintela <quintela@redhat.com>]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> Use led status notification support in vnc.

> +static void kbd_leds(void *opaque, int ledstate)
> +{
> +    VncState *vs = opaque;
> +    int caps, num;
> +
> +    caps = ledstate & QEMU_CAPS_LOCK_LED ? 1 : 0;
> +    num  = ledstate & QEMU_NUM_LOCK_LED  ? 1 : 0;

I think it is clearer to use a bool.

   bool caps = ledstate & QEMU_CAPS_LOCK_LED;


> +    if (vs->modifiers_state[0x3a] != caps) {
> +        vs->modifiers_state[0x3a] = caps;

modifiers_state type needs to go from uint8_t to bool.  It simplifies
lots of !!foo around.  But the change is independent of this series.

> +    }
> +    if (vs->modifiers_state[0x45] != num) {
> +        vs->modifiers_state[0x45] = num;
> +    }
> +}
> +
>  static void do_key_event(VncState *vs, int down, int keycode, int sym)
>  {
>      /* QEMU console switch */
> @@ -1521,7 +1538,7 @@ static void do_key_event(VncState *vs, int down, int keycode, int sym)
>          break;
>      case 0x3a:                        /* CapsLock */
>      case 0x45:                        /* NumLock */
> -        if (!down)
> +        if (down)
>              vs->modifiers_state[keycode] ^= 1;
>          break;
>      }

This needs a comment on the changelog why this is needed IMHO.

Later, Juan.