Re: [Qemu-devel] [PATCH 1/4] kbd leds: infrastructure

[Anthony Liguori <anthony@codemonkey.ws>]

On 02/25/2010 02:39 AM, Gerd Hoffmann wrote:
> Adds infrastructure for keyboard led status tracking to qemu.
>
> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>    

This is an obvious extension to the current API so I'm not necessarily 
opposed to it.

But I wonder if it really makes sense to treat all of these things 
differently since we end up duplicating a lot of code.  Would it make 
more sense to just introduce:

typedef struct QEMUInputHandler {
    void (*put_kbd_event)(QEMUInputHandler *obj, int keycode);
    void (*put_led_event)(QEMUInputHandler *obj, int ledstate);
    void (*put_mouse_event)(QEMUInputHandler *obj, int dx, int dy, int 
dz, int buttons_state);
    QLIST_ENTRY(QEMUInputHandler) node;
} QEMUInputHandler;

void qemu_add_input_handler(QEMUInputHandler *handler);
void qemu_remove_input_handler(QEMUInputHandler *handler);

Regards,

Anthony Liguori

> ---
>   console.h |   14 ++++++++++++++
>   input.c   |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 64 insertions(+), 0 deletions(-)
>
> diff --git a/console.h b/console.h
> index 916859d..0e969d1 100644
> --- a/console.h
> +++ b/console.h
> @@ -10,10 +10,15 @@
>   #define MOUSE_EVENT_RBUTTON 0x02
>   #define MOUSE_EVENT_MBUTTON 0x04
>
> +#define QEMU_SCROLL_LOCK_LED (1<<  0)
> +#define QEMU_NUM_LOCK_LED    (1<<  1)
> +#define QEMU_CAPS_LOCK_LED   (1<<  2)
> +
>   /* in ms */
>   #define GUI_REFRESH_INTERVAL 30
>
>   typedef void QEMUPutKBDEvent(void *opaque, int keycode);
> +typedef void QEMUPutLEDEvent(void *opaque, int ledstate);
>   typedef void QEMUPutMouseEvent(void *opaque, int dx, int dy, int dz, int buttons_state);
>
>   typedef struct QEMUPutMouseEntry {
> @@ -26,13 +31,22 @@ typedef struct QEMUPutMouseEntry {
>       struct QEMUPutMouseEntry *next;
>   } QEMUPutMouseEntry;
>
> +typedef struct QEMUPutLEDEntry {
> +    QEMUPutLEDEvent *put_led;
> +    void *opaque;
> +    struct QEMUPutLEDEntry *next;
> +} QEMUPutLEDEntry;
> +
>   void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque);
>   QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
>                                                   void *opaque, int absolute,
>                                                   const char *name);
>   void qemu_remove_mouse_event_handler(QEMUPutMouseEntry *entry);
> +QEMUPutLEDEntry *qemu_add_led_event_handler(QEMUPutLEDEvent *func, void *opaque);
> +void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry);
>
>   void kbd_put_keycode(int keycode);
> +void kbd_put_ledstate(int ledstate);
>   void kbd_mouse_event(int dx, int dy, int dz, int buttons_state);
>   int kbd_mouse_is_absolute(void);
>
> diff --git a/input.c b/input.c
> index 955b9ab..82bc85c 100644
> --- a/input.c
> +++ b/input.c
> @@ -31,6 +31,7 @@
>
>   static QEMUPutKBDEvent *qemu_put_kbd_event;
>   static void *qemu_put_kbd_event_opaque;
> +static QEMUPutLEDEntry *qemu_put_led_event_head;
>   static QEMUPutMouseEntry *qemu_put_mouse_event_head;
>   static QEMUPutMouseEntry *qemu_put_mouse_event_current;
>
> @@ -102,6 +103,44 @@ void qemu_remove_mouse_event_handler(QEMUPutMouseEntry *entry)
>       qemu_free(entry);
>   }
>
> +QEMUPutLEDEntry *qemu_add_led_event_handler(QEMUPutLEDEvent *func,
> +                                            void *opaque)
> +{
> +    QEMUPutLEDEntry *s;
> +
> +    s = qemu_mallocz(sizeof(QEMUPutLEDEntry));
> +
> +    s->put_led = func;
> +    s->opaque = opaque;
> +    s->next = qemu_put_led_event_head;
> +    qemu_put_led_event_head = s;
> +    return s;
> +}
> +
> +void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry)
> +{
> +    QEMUPutLEDEntry *prev = NULL, *cursor;
> +
> +    if (!qemu_put_led_event_head || entry == NULL)
> +        return;
> +
> +    cursor = qemu_put_led_event_head;
> +    while (cursor != NULL&&  cursor != entry) {
> +        prev = cursor;
> +        cursor = cursor->next;
> +    }
> +
> +    if (cursor == NULL) // does not exist or list empty
> +        return;
> +
> +    if (prev == NULL) { // entry is head
> +        qemu_put_led_event_head = cursor->next;
> +    } else {
> +        prev->next = entry->next;
> +    }
> +    qemu_free(entry);
> +}
> +
>   void kbd_put_keycode(int keycode)
>   {
>       if (qemu_put_kbd_event) {
> @@ -109,6 +148,17 @@ void kbd_put_keycode(int keycode)
>       }
>   }
>
> +void kbd_put_ledstate(int ledstate)
> +{
> +    QEMUPutLEDEntry *cursor;
> +
> +    cursor = qemu_put_led_event_head;
> +    while (cursor != NULL) {
> +        cursor->put_led(cursor->opaque, ledstate);
> +        cursor = cursor->next;
> +    }
> +}
> +
>   void kbd_mouse_event(int dx, int dy, int dz, int buttons_state)
>   {
>       QEMUPutMouseEvent *mouse_event;
>