[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/3 V3] lib: add virtkey



On Fri, Jun 24, 2011 at 02:33:29PM +0800, Lai Jiangshan wrote:
> Add virtkey lib for usage-improvment and keycode translating.
> Add 4 internal API for the aim
> 
> const char *virKeycodeSetName(virKeycodeSet codeset);
> virKeycodeSet virParseKeycodeSet(const char *name);

These should just be done using the standard VIR_ENUM_DECL/IMPL macros.

> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index 3f634e6..2f2efe7 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1815,6 +1815,12 @@ typedef enum {
>      VIR_KEYCODE_SET_ATSET1         = 2,
>      VIR_KEYCODE_SET_ATSET2         = 3,
>      VIR_KEYCODE_SET_ATSET3         = 4,
> +    VIR_KEYCODE_SET_OSX            = 5,
> +    VIR_KEYCODE_SET_XT_KBD         = 6,
> +    VIR_KEYCODE_SET_USB            = 7,
> +    VIR_KEYCODE_SET_WIN32          = 8,
> +    VIR_KEYCODE_SET_XWIN_XT        = 9,
> +    VIR_KEYCODE_SET_XFREE86_KBD_XT = 10,
>  } virKeycodeSet;

IMHO, we don't really need to include the XT_KBD, XWIN_XT or
XFREE86_KBD_XT codesets, since these are all special purpose
sets which are just derived from the based XT set. Lets just
stick to the core interesting sets. So add OSX, USB and WIN32
only.

> diff --git a/src/util/virtkey.c b/src/util/virtkey.c
> new file mode 100644
> index 0000000..48fbfcc
> --- /dev/null
> +++ b/src/util/virtkey.c
> @@ -0,0 +1,633 @@
> +
> +/*
> + * Copyright (c) 2011 Lai Jiangshan
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <config.h>
> +#include <string.h>
> +#include <stddef.h>
> +#include <libvirt/libvirt.h>
> +#include "virtkey.h"
> +
> +#define ARRAY_SIZE(array) (sizeof(array)/sizeof(array[0]))
> +#define getfield(object, field_type, field_offset) \
> +    (*(typeof(field_type) *)((char *)(object) + field_offset))
> +
> +struct keycode {
> +    const char *linux_name;
> +    const char *os_x_name;
> +    const char *win32_name;
> +    unsigned short linux_keycode;
> +    unsigned short xt;
> +    unsigned short atset1;
> +    unsigned short atset2;
> +    unsigned short atset3;
> +    unsigned short os_x;
> +    unsigned short xt_kbd;
> +    unsigned short usb;
> +    unsigned short win32;
> +    unsigned short xwin_xt;
> +    unsigned short xfree86_kbd_xt;
> +};
> +
> +/*
> + * generated from http://git.gnome.org/browse/gtk-vnc/plain/src/keymaps.csv
> + * script:
> + *
> + * #!/bin/python
> + * import sys
> + * import re
> + *
> + * for line in sys.stdin.xreadlines():
> + *     a = re.match("([^,]*)," * 13 + "([^,]*)$", line[0:-1]).groups()
> + *     b = ""
> + *     for i in (0,2,10,1,7,4,5,6,3,8,9,11,12,13):
> + *         if i in (0, 2, 10):
> + *             b = b + (a[i] and ('"' + a[i] + '"') or 'NULL') + ','
> + *         else:
> + *             b = b + (a[i] or '0') + ','
> + *     print "    { " + b + "},"
> + */

One of the goals of having the keymap data in the CSV file was that
it makes it trivially updatable across apps using it, without making
code changes. In fact my goal is to actually put 'keymaps.csv' and
'keymaps.pl' into a separate shared package at some point. So rather
than hardcoding this giant array in libvirt, just include the GTK-VNC
keymaps.csv and keymaps.pl file as-is, and run them to generate the
mapping tables for combinations we need.

NB, keymaps.pl will need to be updated to be able to output a
table for doing "string->keycode" mapping since it doesn't
do that yet. For the plain  keycode->keycode mappings though
just use its currently functionality.

> +const char *virKeycodeSetName(virKeycodeSet codeset)
> +{
> +    int i = (int)codeset;
> +
> +    if (i < 0 || i >= ARRAY_SIZE(codesetInfo))
> +        return "UNKNOWN";
> +
> +    return codesetInfo[i].name;
> +}
> +
> +virKeycodeSet virParseKeycodeSet(const char *name)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(codesetInfo); i++) {
> +        if (!strcmp(codesetInfo[i].name, name))
> +            return (virKeycodeSet)i;
> +    }
> +
> +    return (virKeycodeSet)-1;
> +}

These just get replaced by VIR_ENUM_IMPL

> +static int virParseKeyNameOffset(unsigned int name_offset,
> +                                 unsigned int code_offset,
> +                                 const char *keyname)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(keycodes); i++) {
> +        const char *name = getfield(keycodes + i, const char *, name_offset);
> +
> +        if (name && !strcmp(name, keyname))
> +            return getfield(keycodes + i, unsigned short, code_offset);
> +    }
> +
> +    return -1;
> +}

This will want to use a number table that keymaps.pl will
need to generate for name -> code mapping.

> +static int virTranslateKeyCodeOffset(unsigned int from_offset,
> +                                     unsigned int to_offset,
> +                                     int key_value)
> +{
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(keycodes); i++) {
> +        if (getfield(keycodes + i, unsigned short, from_offset) == key_value)
> +            return getfield(keycodes + i, unsigned short, to_offset);
> +    }
> +
> +    return -1;
> +}

This is not nice because it is O(n) lookups. If you just use
the keymaps.pl script to generate all the conversion tables
we need, we get O(1) lookups & simpler code.

> diff --git a/tools/virsh.c b/tools/virsh.c
> index fcd254d..a1e2f83 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -58,6 +58,7 @@
>  #include "threads.h"
>  #include "command.h"
>  #include "count-one-bits.h"
> +#include "virtkey.h"
>  
>  static char *progname;

This ought to be in the next patch, since this doesn't need it yet

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]