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

[Olaf H. <ol...@su...>]

On Sat, Jun 02, Geert Uytterhoeven wrote:

> 	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?

Yes, I didnt try the gpm selection. argh :) It works now on the ibook
with atyfb.
vga_console was disabled in the release so I didnt notice.
However, my asm-ppc/vga.h patch is still needed for real VGA text mode.

Btw, there is a syntax error, see below.


Gruss Olaf

> --- 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)


Gruss Olaf

-- 
 $ man clone

BUGS
       Main feature not yet implemented...