console UTF-8 fixes

Hi Andrew,

I've been told to send this patch to you for inclusion in your patchset, and hopefully sooner or later in the mainline kernel too. It has been discussed on lkml and finally found to be OK by HPA and Jan.

The UTF-8 part of the vt driver suffers from the following issues which are addressed in my patch:

1) If there's no glyph found for a particular valid UTF-8 character, we try to display U+FFFD. However if this one is not found either, here's what the current kernel does:

- First, if the Unicode value is less than the number of glyphs, use the glyph directly from that position of the glyph table. While it may be a good idea in the 8-bit world, it has absolutely no sense with Unicode in mind. For example, if a Latin-2 font is loaded and an application prints U+00FB ("u with circumflex", not present in Latin-2) then as a fallback solution the glyph from the 0xFB position of the Latin-2 fontset (which is an "u with double accent" - a different character) is displayed.

- Second, if this fallback fails too, a simple ASCII question mark is printed, which is visually undistinguishable from a real question mark.

I changed the code to skip the first step (except if in non-UTF-8 mode), and changed the second step to print the question mark with inverse color attributes, so it is visually clear that it's not a real question mark, and resembles more to the common glyph of U+FFFD.

2) The UTF-8 decoder is buggy in many ways:

- Lone continuation bytes (section 3.1 of Markus Kuhn's UTF-8 stress test) are not caught, they are displayed as some "random" (taken directly form the font table, see above) glyphs instead the replacement character.

- Incomplete sequences (sections 3.2 and 3.3 of the stress test) emit no replacement character, but rather cause the subsequent valid character to be displayed more times(!).

- The decoder is not safe: overlong sequences are not caught currently, they are displayed as if these were valid representations. This may even have security impacts.

- The decoder does not handle D800..DFFF and FFFE..FFFF specially, it just emits these code points and lets it be looked up in the glyph table. Since these are invalid code points, I replace them by U+FFFD and hence give no chance for them to be looked up in the glyph table. (Assuming no font ships glyphs for these code points, this change is not visible to the users since the glyph shown will be the same.)

With my fixes to the decoder it now behaves exactly as Markus Kuhn's stress test recommends.

3) It has no concept of double-width (CJK) characters. It's way beyond the scope of my patch to try to display them, but at least I think it's important for the cursor to jump two positions when printing such characters, since this is what applications (such as text editors) expect. Currently the cursor only jumps one position, and hence applications suffer from displaying and refreshing problems, and editing some English letters that are preceded by some CJK characters in the same line is a nightmare. With my patch an additional space is inserted after the CJK character has been printed (which usually means a replacement symbol of course). (If U+FFFD isn't availble and hence an inverse question mark is displayed in the first cell, I keep the inverted state for the space in the 2nd column so it's quite easy to see that they are tied together.)

4) There is a small built-in table of zero-width spaces that are not to be printed but silently skipped. U+200A is included there, but it's not a zero-width character, so I remove it from there.

The patch is exactly the same as I've sent to the list the last time (labelled as "version 4").

diff -Naur linux-2.6.20.orig/drivers/char/consolemap.c linux-2.6.20/drivers/char/consolemap.c
@@ -626,10 +626,10 @@
   
 	/* Only 16-bit codes supported at this time */
 	if (ucs > 0xffff)
-		ucs = 0xfffd;		/* U+FFFD: REPLACEMENT CHARACTER */
-	else if (ucs < 0x20 || ucs >= 0xfffe)
+		return -4;		/* Not found */
+	else if (ucs < 0x20)
 		return -1;		/* Not a printable character */
-	else if (ucs == 0xfeff || (ucs >= 0x200a && ucs <= 0x200f))
+	else if (ucs == 0xfeff || (ucs >= 0x200b && ucs <= 0x200f))
 		return -2;			/* Zero-width space */
 	/*
 	 * UNI_DIRECT_BASE indicates the start of the region in the User Zone

Hi folks,

Recently my console UTF-8 patch went mainline. Here I send a very small additinal patch that fixes two nasty issues and improves a third one, namely:

1. My patch changed the behavior if a glyph is not found in the Unicode mapping table. Previously for Unicode values less than 256 or 512 the kernel tried to display the glyph from that position of the glyph table, which could lead to a different accented letter being displayed. I removed this fallback possibility and changed it to display the replacement symbol.

As Behdad pointed out, some fonts (e.g. sun12x22 from the kbd package) lack Unicode mapping information, hence all you get is lots of question marks. Though theoretically it's actually a user-space bug (the font should be fixed), Behdad and I both believe that it'd be good to work around in the kernel by re-introducing the fallback solution for ASCII characters only. This sounds a quite reasonable decision, since all fonts ship the ASCII characters in the first 128 positions. This way users won't be surprised by lots of question marks just because s/he issued a not-so-perfectly parameterized setfont command. As this fallback is only re-introduced for code points below 128, you still won't see an accented letter replaced by another, but at least you'll always get the English letters right.

2. My patch introduced "question mark with inverted color attributes" as a last resort fallback glyph. Though it perfectly works on VGA console, on framebuffer you may end up with question marks that are highlighed but shouldn't be, and normal characters that are accidentally highlighed. This is caused by missing FLUSHes when changing the color attribute.

3. I've updated the table of double-width character based on Markus's updated version. Only ten new code poings (one interval) is added.

Please review and commit this patch... I'm afraid it might be too late, but it would be nice to see it in 2.6.22 -- as this will be the first stable release that introduces my work, and the first two issues addressed now are regressions since 2.6.21.

Thanks,

diff -Naur linux-2.6.22-rc5.orig/drivers/char/vt.c linux-2.6.22-rc5/drivers/char/vt.c
@@ -1956,7 +1956,7 @@
 DEFINE_MUTEX(con_buf_mtx);
 
 /* is_double_width() is based on the wcwidth() implementation by
- * Markus Kuhn -- 2003-05-20 (Unicode 4.0)
+ * Markus Kuhn -- 2007-05-26 (Unicode 5.0)
  * Latest version: http://www.cl.cam.ac.uk/~mgk25/ucs/wcwidth.c
  */
 struct interval {

@@ -1988,8 +1988,8 @@
 	static const struct interval double_width[] = {
 		{ 0x1100, 0x115F }, { 0x2329, 0x232A }, { 0x2E80, 0x303E },
 		{ 0x3040, 0xA4CF }, { 0xAC00, 0xD7A3 }, { 0xF900, 0xFAFF },
-		{ 0xFE30, 0xFE6F }, { 0xFF00, 0xFF60 }, { 0xFFE0, 0xFFE6 },
-		{ 0x20000, 0x2FFFD }, { 0x30000, 0x3FFFD }
+		{ 0xFE10, 0xFE19 }, { 0xFE30, 0xFE6F }, { 0xFF00, 0xFF60 },
+		{ 0xFFE0, 0xFFE6 }, { 0x20000, 0x2FFFD }, { 0x30000, 0x3FFFD }
 	};
 	return bisearch(ucs, double_width,
 		sizeof(double_width) / sizeof(*double_width) - 1);

@@ -2187,9 +2187,12 @@
 				    continue; /* nothing to display */
 				}
 				/* Glyph not found */
-				if (!(vc->vc_utf && !vc->vc_disp_ctrl) && !(c & ~charmask)) {
+				if ((!(vc->vc_utf && !vc->vc_disp_ctrl) || c < 128) && !(c & ~charmask)) {
 				    /* In legacy mode use the glyph we get by a 1:1 mapping.
-				       This would make absolutely no sense with Unicode in mind. */
+				       This would make absolutely no sense with Unicode in mind,
+				       but do this for ASCII characters since a font may lack
+				       Unicode mapping info and we don't want to end up with
+				       having question marks only. */
 				    tc = c;
 				} else {
 				    /* Display U+FFFD. If it's not found, display an inverse question mark. */

@@ -2213,6 +2216,7 @@
 				} else {
 					vc_attr = ((vc->vc_attr) & 0x88) | (((vc->vc_attr) & 0x70) >> 4) | (((vc->vc_attr) & 0x07) << 4);
 				}
+				FLUSH
 			}
 
 			while (1) {

@@ -2246,6 +2250,10 @@
 				if (tc < 0) tc = ' ';
 			}
 
+			if (inverse) {
+				FLUSH
+			}
+
 			if (rescan) {
 				rescan = 0;
 				inverse = 0;