[Linux-fbdev-devel] Re: FBCon invert_region is endian unsafe, same with generic invert_region

[Geert U. <ge...@li...>]

	Hi Daniel,

> I'm not quite sure who the maintainer is, fbcon.c lists 5 people, and
> no one is listed in the MAINTAINERS file (i searched on fbcon).
> 
> This one has been bothering me for a long time.
> It causes text selection to fail on fbcon devices for big endian
> platforms (Because, obviously, it gets big endian data on big endian
> machines, sinceit neglects to use scr_readw/scr_writew. Dumb. You end
> up inverting, pasting each byte reversed without fixing this, so where
> you would normally have pasted/see a background inverted 'u' (0x75),
> you get a 'W' (0x57)).

    [...]

> However, it's important to note that console.c has the same damn bug
> in it's generic inversion routine.
> 
> Changing the code to the right way makes text selection work perfectly
> again, which it hasn't since 2.2.x, at least for me.

So this patch (untested) fixes it?

Olaf: is this the missing piece for you as well?

--- console-2.4.5/drivers/char/console.c.orig	Mon Feb 19 10:36:47 2001
+++ console-2.4.5/drivers/char/console.c	Sat Jun  2 21:15:10 2001
@@ -390,20 +390,28 @@
 	else {
 		u16 *q = p;
 		int cnt = count;
+		u16 a;
 
 		if (!can_do_color) {
-			while (cnt--) *q++ ^= 0x0800;
+			while (cnt--) {
+			    a = scr_readw(q);
+			    a ^= 0x0800;
+			    scr_writew(a, q);
+			    q++;
+			}
 		} else if (hi_font_mask == 0x100) {
 			while (cnt--) {
-				u16 a = *q;
+				a = scr_readw(q);
 				a = ((a) & 0x11ff) | (((a) & 0xe000) >> 4) | (((a) & 0x0e00) << 4);
-				*q++ = a;
+				scr_writew(a, q);
+				q++;
 			}
 		} else {
 			while (cnt--) {
-				u16 a = *q;
+				a = scr_readw(q);
 				a = ((a) & 0x88ff) | (((a) & 0x7000) >> 4) | (((a) & 0x0700) << 4);
-				*q++ = a;
+				scr_writew(a, q);
+				q++
 			}
 		}
 	}
--- console-2.4.5/drivers/video/fbcon.c.orig	Mon Feb 19 10:37:00 2001
+++ console-2.4.5/drivers/video/fbcon.c	Sat Jun  2 21:07:38 2001
@@ -1934,17 +1934,15 @@
 static void fbcon_invert_region(struct vc_data *conp, u16 *p, int cnt)
 {
     while (cnt--) {
+	u16 a = scr_readw(p);
 	if (!conp->vc_can_do_color)
-	    *p++ ^= 0x0800;
-	else if (conp->vc_hi_font_mask == 0x100) {
-	    u16 a = *p;
+	    a ^= 0x0800;
+	else if (conp->vc_hi_font_mask == 0x100)
 	    a = ((a) & 0x11ff) | (((a) & 0xe000) >> 4) | (((a) & 0x0e00) << 4);
-	    *p++ = a;
-	} else {
-	    u16 a = *p;
+	else
 	    a = ((a) & 0x88ff) | (((a) & 0x7000) >> 4) | (((a) & 0x0700) << 4);
-	    *p++ = a;
-	}
+	scr_writew(a, p);
+	p++;
 	if (p == (u16 *)softback_end)
 	    p = (u16 *)softback_buf;
 	if (p == (u16 *)softback_in)

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@li...

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds