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

[Amos Kong <akong@redhat.com>]

On 31/08/12 01:46, Luiz Capitulino wrote:
> On Mon, 20 Aug 2012 12:39:28 +0800
> Amos Kong<akong@redhat.com>  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<akong@redhat.com>
>
> 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.