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

Re: [libvirt] [PATCH 2/4 V4] util: add virtkey



On Thu, Jul 14, 2011 at 11:32:16AM +0800, Lai Jiangshan wrote:
> Add virtkey lib for usage-improvment and keycode translating.
> Add 4 internal API for the aim
> 
> const char *virKeycodeSetTypeToString(int codeset);
> int virKeycodeSetTypeFromString(const char *name);
> int virParseKeyName(virKeycodeSet codeset, const char *keyname);
> int virTranslateKeyCode(virKeycodeSet from_codeset,
>                         virKeycodeSet to_offset,
>                         int key_value);
> 
> Signed-off-by: Lai Jiangshan <laijs cn fujitsu com>
> ---
>  include/libvirt/libvirt.h.in |    8 +++
>  src/Makefile.am              |   11 ++++-
>  src/libvirt_private.syms     |    5 ++
>  src/util/virtkey.c           |  117 ++++++++++++++++++++++++++++++++++++++++++
>  src/util/virtkey.h           |   41 +++++++++++++++

Can we call these two  'virkeycode.h' and 'virkeycode.c'

>  src/util/virtkeymap-gen.py   |   47 +++++++++++++++++

And  'virkeycode-mapgen.py'


> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index d5a7105..acfe9d9 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -1778,6 +1778,14 @@ 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,

I have confirmed that "XT_KBD" is right for the QEMU keys, because
this matches what we use for the VNC protocol.

I would still like us to leave XWIN_XT and XFREE86_KBD_XT out of
the headers for now.


> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 3237d18..6611471 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1094,6 +1094,11 @@ virSetError;
>  virSetErrorLogPriorityFunc;
>  virStrerror;
>  
> +# virtkey.h
> +virKeycodeSetTypeToString;
> +virKeycodeSetTypeFromString;
> +virParseKeyName;
> +virTranslateKeyCode;

As a (newish) rule, all function name prefixes should match the
source filename. So can you change these to

   virKeycodeValueFromString
   virKeycodeValueTranslate


> +static unsigned int codeOffset[] = {
> +    [VIR_KEYCODE_SET_LINUX] =
> +        offsetof(struct keycode, linux_keycode),
> +    [VIR_KEYCODE_SET_XT] =
> +        offsetof(struct keycode, xt),
> +    [VIR_KEYCODE_SET_ATSET1] =
> +        offsetof(struct keycode, atset1),
> +    [VIR_KEYCODE_SET_ATSET2] =
> +        offsetof(struct keycode, atset2),
> +    [VIR_KEYCODE_SET_ATSET3] =
> +        offsetof(struct keycode, atset3),
> +    [VIR_KEYCODE_SET_OSX] =
> +        offsetof(struct keycode, os_x),
> +    [VIR_KEYCODE_SET_XT_KBD] =
> +        offsetof(struct keycode, xt_kbd),
> +    [VIR_KEYCODE_SET_USB] =
> +        offsetof(struct keycode, usb),
> +    [VIR_KEYCODE_SET_WIN32] =
> +        offsetof(struct keycode, win32),
> +    [VIR_KEYCODE_SET_XWIN_XT] =
> +        offsetof(struct keycode, xwin_xt),
> +    [VIR_KEYCODE_SET_XFREE86_KBD_XT] =
> +        offsetof(struct keycode, xfree86_kbd_xt),
> +};
> +
> +VIR_ENUM_IMPL(virKeycodeSet, VIR_KEYCODE_SET_LAST,
> +    "linux",
> +    "xt",
> +    "atset1",
> +    "atset2",
> +    "atset3",
> +    "os_x",
> +    "xt_kbd",
> +    "usb",
> +    "win32",
> +    "xwin_xt",
> +    "xfree86_kbd_xt");

Likewise leave out  xin_xt and xfree86_kbd_xt

> diff --git a/src/util/virtkey.h b/src/util/virtkey.h
> new file mode 100644
> index 0000000..45c5f08
> --- /dev/null
> +++ b/src/util/virtkey.h
> @@ -0,0 +1,41 @@
> +#ifndef __UTIL_VIRTKEY_H__
> +#define __UTIL_VIRTKEY_H__
> +
> +/*
> + * 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 "util.h"
> +#include "libvirt/libvirt.h"
> +
> +struct keycode {
> +    const char *linux_name;
> +    const char *os_x_name;
> +    const char *win32_name;
> +    unsigned short linux_keycode;
> +    unsigned short os_x;
> +    unsigned short atset1;
> +    unsigned short atset2;
> +    unsigned short atset3;
> +    unsigned short xt;
> +    unsigned short xt_kbd;
> +    unsigned short usb;
> +    unsigned short win32;
> +    unsigned short xwin_xt;
> +    unsigned short xfree86_kbd_xt;
> +};
> +extern struct keycode virtKeycodes[];
> +extern int virtKeycodesSize;

None of these three declarations need to be in the header file
since they are private impl details of the source fiel.

> +
> +VIR_ENUM_DECL(virKeycodeSet);
> +int virParseKeyName(virKeycodeSet codeset, const char *keyname);
> +int virTranslateKeyCode(virKeycodeSet from_codeset,
> +                        virKeycodeSet to_offset,
> +                        int key_value);
> +
> +#endif
> diff --git a/src/util/virtkeymap-gen.py b/src/util/virtkeymap-gen.py

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]