Re: [Qemu-devel] [PATCH v7 5/6] add the QKeyCode enum and the key_defs table

[Amos Kong]

On 31/08/12 01:46, Luiz Capitulino wrote:
> On Mon, 20 Aug 2012 12:39:28 +0800
> Amos Kong<address@hidden>  wrote:
>
> > key_defs[] in monitor.c is a mapping table of keys and keycodes,
> > this patch added a QKeyCode enum and a new key_defs table,
> > key's index in the enmu is same as keycode's index in new key_defs[].
> >
> > And added two help functions to convert key/code to index of mapping
> > table, those functions will return Q_KEY_CODE_MAX if the code/key is
> > invalid.
> >
> > 'key_defs' was dropped from the monitor, monitor functions were
> > changed to access key_defs directly.
> >
> > Signed-off-by: Amos Kong<address@hidden>
>
> This patch (and probably the next one too) doesn't apply on master, could
> you rebase please?

Will send V8 later.

> As you'll have to respin, please change versions to 1.3.0. I also have
> comments below (minor, but as you'll respin anyway).

Ok.

> > ---
> >   console.h        |    6 ++
> >   input.c          |  186 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   monitor.c        |  183 +++-------------------------------------------------
> >   qapi-schema.json |   26 ++++++++
> >   4 files changed, 229 insertions(+), 172 deletions(-)
> >
> > diff --git a/console.h b/console.h
> > index 4334db5..7934b11 100644
> > --- a/console.h
> > +++ b/console.h
> > @@ -6,6 +6,7 @@
> >   #include "notify.h"
> >   #include "monitor.h"
> >   #include "trace.h"
> > +#include "qapi-types.h"
> >
> >   /* keyboard/mouse support */
> >
> > @@ -397,4 +398,9 @@ static inline int vnc_display_pw_expire(DisplayState *ds, 
> > time_t expires)
> >   /* curses.c */
> >   void curses_display_init(DisplayState *ds, int full_screen);
> >
> > +/* input.c */
> > +extern const int key_defs[];
>
> Why are you exporting key_defs[]? It should be static.

key_defs[] is used in do_send_key() after applying this patch, so export 
key_defs[].
It will be changed to static in next patch.

> Also, it would be better to move the addition of QKeyCode out of this patch
> (ie. you first add it and then move the table).

Will do it.

--
                        Amos.