[PATCH] console keyboard mapping broken by 04c71976

[PATCH] console keyboard mapping broken by 04c71976

[Jiri Bohac]

Hi,

the Czech (and I believe many other) console keyboard maps are
broken since 04c71976. 

To make old (non-unicode - the majority distributed with the kbd
package) keymaps work, the keyboard mapping in the kernel uses a
trick. It uses the translation tables loaded with the console
font to translate the 8-bit values to unicode values.

So with the currently available keymaps when using unicode in
the console you end up with:

- kbd->kbdmode set to VC_UNICODE, to send UTF-8 sequences to the
  terminal
- most letters, icluding non-latin1, defined as type=KT_LETTER in the
  keymap, with an 8-bit value, which only has its correct meaning
  when translated using conv_8bit_to_uni().

04c71976 changes k_self() to:
static void k_self(struct vc_data *vc, unsigned char value, char up_flag)
{
	unsigned int uni;
	if (kbd->kbdmode == VC_UNICODE)
		uni = value;
	else
		uni = conv_8bit_to_uni(value);
	k_unicode(vc, uni, up_flag);
}

It wrongly assumes, that when the keyboard is in the VC_UNICODE
mode, value is already the correct unicode value. This is only
true for latin1.

I think we need to always convert the value with
conv_8bit_to_uni(), as we did before 04c71976.

The following patch fixes the problem for me: 

diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index 7f7e798..16492b7 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -678,10 +678,7 @@ static void k_deadunicode(struct vc_data *vc, unsigned int value, char up_flag)
 static void k_self(struct vc_data *vc, unsigned char value, char up_flag)
 {
 	unsigned int uni;
-	if (kbd->kbdmode == VC_UNICODE)
-		uni = value;
-	else
-		uni = conv_8bit_to_uni(value);
+	uni = conv_8bit_to_uni(value);
 	k_unicode(vc, uni, up_flag);
 }

As far as I can see, it will affect latin1 users that now get
their keymaps working even without loading the "trivial" console
map (so the conversion will not do anything) but I think they
needed to do that before 04c71976, so they probably still do
without even knowing, right?

Samuel, any comments?

-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

Re: [PATCH] console keyboard mapping broken by 04c71976

[Samuel Thibault]

Err, at quick look it looks to me rather like a bug kbd, which doesn't
cope with the console being in unicode mode.  Which distribution are you
using?

In unicode mode, k_self is really meant to hold unicode values (there is
no other way to provide an arbitrary unicode value in keyboard maps),
and thus kbd should translate KT_LETTER's value from the map file's
encoding (set by the charset directive) to unicode.

Samuel

Re: [PATCH] console keyboard mapping broken by 04c71976

[Jiri Bohac]

On Wed, Jun 11, 2008 at 03:03:04PM +0100, Samuel Thibault wrote:
> Err, at quick look it looks to me rather like a bug kbd, which doesn't
> cope with the console being in unicode mode.  Which distribution are you
> using?
> 
> In unicode mode, k_self is really meant to hold unicode values (there is
> no other way to provide an arbitrary unicode value in keyboard maps),
> and thus kbd should translate KT_LETTER's value from the map file's
> encoding (set by the charset directive) to unicode.

I am actually hunting down a bug this causes in openSUSE. We use
kbd-1.12. kbd does not translate anything to unicode - there is
no space for that in the 8 bits the keycode uses for the value.
The kernel needs to take care for the conversion.

I'll demonstrate the problem on tracing kbd_keycode() in
drivers/char/keyboard.c when the key "2" (keycode 3) is pressed.
The expected result is the letter letter "e" with a caron [CARON]
accent, code 0xec in iso-8859-2 [88592].

The keysym for this key in the Czech keyboard is set to 0xbec
by a call to KDSKBENT, which stores it as 0xfbec in the map.

	...
	type = KTYP(keysym);		// 0xfb
	...
	type -= 0xf0;			// 0xb
        if (type == KT_LETTER) {	// true
                type = KT_LATIN;	// 0
 
	...
        (*k_handler[type])(vc, keysym & 0xff, !down); 
		// calls k_self, note there is really no space for a 
		// full unicode representation ;-)

So, we need to perform the conversion in k_self (or later).  Your
patch removed it from k_unicode, but only does it in k_self when
the keyboard mode is not VC_UNICODE.

[CARON]: http://en.wikipedia.org/wiki/Caron
[88592]: http://en.wikipedia.org/wiki/Iso-8859-2

Regards,

-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

Re: [PATCH] console keyboard mapping broken by 04c71976

[Samuel Thibault]

Jiri Bohac, le Wed 11 Jun 2008 17:10:28 +0200, a écrit :
> 	...
> 	type = KTYP(keysym);		// 0xfb
> 	...
> 	type -= 0xf0;			// 0xb
>         if (type == KT_LETTER) {	// true
>                 type = KT_LATIN;	// 0
>  
> 	...
>         (*k_handler[type])(vc, keysym & 0xff, !down); 
> 		// calls k_self, note there is really no space for a 
> 		// full unicode representation ;-)

Ah, right, my too-quick look mistook your case with the (type < 0xf0)
case, which does have room for unicode.

Well, in my tests it _was_ working with latin2.  I'll dig more this
evening.

Samuel

Re: [PATCH] console keyboard mapping broken by 04c71976

[Samuel Thibault]

Ok, a closer look agrees with the patch, but no need to keep the uni
variable then, here is an updated patch.  Thanks!

From: Jiri Bohac <jbohac@suse.cz>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index 7f7e798..16492b7 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -678,10 +678,5 @@
 static void k_self(struct vc_data *vc, unsigned char value, char up_flag)
 {
-	unsigned int uni;
-	if (kbd->kbdmode == VC_UNICODE)
-		uni = value;
-	else
-		uni = conv_8bit_to_uni(value);
-	k_unicode(vc, uni, up_flag);
+	k_unicode(vc, conv_8bit_to_uni(value), up_flag);
 }

Re: [PATCH] console keyboard mapping broken by 04c71976

[Andrew Morton]

On Thu, 12 Jun 2008 02:18:32 +0100 Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:

> 
> Ok, a closer look agrees with the patch, but no need to keep the uni
> variable then, here is an updated patch.  Thanks!
> 
> From: Jiri Bohac <jbohac@suse.cz>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
> index 7f7e798..16492b7 100644
> --- a/drivers/char/keyboard.c
> +++ b/drivers/char/keyboard.c
> @@ -678,10 +678,5 @@
>  static void k_self(struct vc_data *vc, unsigned char value, char up_flag)
>  {
> -	unsigned int uni;
> -	if (kbd->kbdmode == VC_UNICODE)
> -		uni = value;
> -	else
> -		uni = conv_8bit_to_uni(value);
> -	k_unicode(vc, uni, up_flag);
> +	k_unicode(vc, conv_8bit_to_uni(value), up_flag);
>  }

Is this ready to be applied?  If so, please send a changelog.

btw, the From: line shold be right at the top of the changelog.

Thanks.

Re: [PATCH] console keyboard mapping broken by 04c71976

[Samuel Thibault]

Andrew Morton, le Wed 11 Jun 2008 18:27:06 -0700, a écrit :
> Is this ready to be applied?  If so, please send a changelog.

Ah, right, there was no changelog.



From: Jiri Bohac <jbohac@suse.cz>

Several console keyboard maps are broken since 04c71976, because that
changeset made k_self consider the value as a latin1 character when in
Unicode mode, which is wrong; k_self should still take the console map
into account.

Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>

diff --git a/drivers/char/keyboard.c b/drivers/char/keyboard.c
index 7f7e798..16492b7 100644
--- a/drivers/char/keyboard.c
+++ b/drivers/char/keyboard.c
@@ -678,10 +678,5 @@
 static void k_self(struct vc_data *vc, unsigned char value, char up_flag)
 {
-	unsigned int uni;
-	if (kbd->kbdmode == VC_UNICODE)
-		uni = value;
-	else
-		uni = conv_8bit_to_uni(value);
-	k_unicode(vc, uni, up_flag);
+	k_unicode(vc, conv_8bit_to_uni(value), up_flag);
 }