Re: [Qemu-devel] [v2, 3/4] qemu-char: add support for U-prefixed symbols

[Jan Krupa]

On 11/01/2013 12:28 PM, Michael Tokarev wrote:
> 01.11.2013 13:59, Michael Tokarev пишет:
>> 16.10.2013 16:40, Jan Krupa wrote:
>>> This patch adds support for Unicode symbols in keymap files. This
>>> feature was already used in some keyboard layouts in QEMU generated
>>> from XKB (e.g. Arabic) but it wasn't implemented in QEMU source code.
>>>
>>> There is no need for check of validity of the hex string after U
>>> character
>>> because strtol returns 0 in case the conversion was unsuccessful.
>>>
>>> Signed-off-by: Jan Krupa <address@hidden>
>>>
>>> ---
>>> ui/keymaps.c |    3 +++
>>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/ui/keymaps.c b/ui/keymaps.c
>>> index f373cc5..426a893 100644
>>> --- a/ui/keymaps.c
>>> +++ b/ui/keymaps.c
>>> @@ -33,6 +33,9 @@ static int get_keysym(const name2keysym_t *table,
>>>           if (!strcmp(p->name, name))
>>>               return p->keysym;
>>>       }
>>> +    if (strlen(name) == 5 && name[0] == 'U') {
>>> +        return (int)strtol(name + 1, NULL, 16);
>>> +    }
>>>       return 0;
>>>   }
>>>
>>
>> I still dislike this.  People already complained that the keysyms
>> should be case-insensitive.  And there might be many words starting
>> with "u" and of 5 chars long.
>>
>> How about this:
>>
>> Author: Jan Krupa <address@hidden>
>> Date:   Wed Oct 16 14:40:05 2013 +0200
>>
>>      qemu-char: add support for U-prefixed symbols
>>
>>      This patch adds support for Unicode symbols in keymap files. This
>>      feature was already used in some keyboard layouts in QEMU generated
>>      from XKB (e.g. Arabic) but it wasn't implemented in QEMU source
>> code.
>>
>>      There is no need for check of validity of the hex string after U
>> character
>>      because strtol returns 0 in case the conversion was unsuccessful.
>>
>>      Signed-off-by: Jan Krupa <address@hidden>
>>      Signed-off-by: Michael Tokarev <address@hidden>
>>
>> diff --git a/ui/keymaps.c b/ui/keymaps.c
>> index f373cc5..80d658d 100644
>> --- a/ui/keymaps.c
>> +++ b/ui/keymaps.c
>> @@ -33,6 +33,12 @@ static int get_keysym(const name2keysym_t *table,
>>           if (!strcmp(p->name, name))
>>               return p->keysym;
>>       }
>> +    if (name[0] == 'U' && strlen(name) == 5) { /* try unicode Uxxxx */
>> +        char *end;
>> +        int ret = (int)strtoul(name + 1, &end, 16);
> 
> Maybe strtol() here as in original.. On my system, both work
> the same anyway :)

Hi Michael,

Thanks for the review! Your change looks good. Agree with adding it this
way.

Jan