const fb_{fillrect,copyarea,image}?

const fb_{fillrect,copyarea,image}?

[Geert Uytterhoeven]

Wouldn't it make sense to make the fb_{fillrect,copyarea,image} parameters of
the fb_ops.fb_{fillrect,copyarea,image}() operations const? This would protect
us against side effects when reusing the fb_{fillrect,copyarea,image} structs
without reinitializing their contents (as is currently done by the logo drawing
code in fbcon_show_logo() on SMP).

Of course this means that we have to modify the clipping code, which currently
just modifies the passed structure.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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



-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: const fb_{fillrect,copyarea,image}?

[James Simmons]

> Wouldn't it make sense to make the fb_{fillrect,copyarea,image} parameters of
> the fb_ops.fb_{fillrect,copyarea,image}() operations const? This would protect
> us against side effects when reusing the fb_{fillrect,copyarea,image} structs
> without reinitializing their contents (as is currently done by the logo drawing
> code in fbcon_show_logo() on SMP).

Where is this exactly?

> Of course this means that we have to modify the clipping code, which currently
> just modifies the passed structure.

:-( That is done to prevent someone from passing data that is larger than 
the framebuffer.



-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com

Re: const fb_{fillrect,copyarea,image}?

[Geert Uytterhoeven]

On Tue, 7 Jan 2003, James Simmons wrote:
> > Wouldn't it make sense to make the fb_{fillrect,copyarea,image} parameters of
> > the fb_ops.fb_{fillrect,copyarea,image}() operations const? This would protect
> > us against side effects when reusing the fb_{fillrect,copyarea,image} structs
> > without reinitializing their contents (as is currently done by the logo drawing
> > code in fbcon_show_logo() on SMP).
> 
> Where is this exactly?

drivers/video/fbmem.c:fb_show_logo() initializes only image.dx in each loop
iteration:

| for (x = 0; x < num_online_cpus() * (LOGO_W + 8) &&
|      x < info->var.xres - (LOGO_W + 8); x += (LOGO_W + 8)) {
|         image.dx = x;
|         info->fbops->fb_imageblit(info, &image);
|         done = 1;
| }

> > Of course this means that we have to modify the clipping code, which currently
> > just modifies the passed structure.
> 
> :-( That is done to prevent someone from passing data that is larger than 
> the framebuffer.

You can still do clipping without modifying the passed structure, right?

BTW, is it possible that someone passes data that is larger (except for bugs)?
We have control over what happens in the kernel. Data passed from userspace is
a different issue, of course.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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



-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com

Re: const fb_{fillrect,copyarea,image}?

[James Simmons]

> drivers/video/fbmem.c:fb_show_logo() initializes only image.dx in each loop
> iteration:
> 
> | for (x = 0; x < num_online_cpus() * (LOGO_W + 8) &&
> |      x < info->var.xres - (LOGO_W + 8); x += (LOGO_W + 8)) {
> |         image.dx = x;
> |         info->fbops->fb_imageblit(info, &image);
> |         done = 1;
> | }

Oh.
 
> > > Of course this means that we have to modify the clipping code, which currently
> > > just modifies the passed structure.
> > 
> > :-( That is done to prevent someone from passing data that is larger than 
> > the framebuffer.
> 
> You can still do clipping without modifying the passed structure, right?

Probable it can be arranged this way.
 
> BTW, is it possible that someone passes data that is larger (except for bugs)?
> We have control over what happens in the kernel. Data passed from userspace is
> a different issue, of course.

Yes. Soon the standard ioctl for a cursor will be truly in place. This 
means cfb_imageblit could be a issue from userland.



-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com

Re: const fb_{fillrect,copyarea,image}?

[Antonino Daplas]

On Fri, 2003-01-10 at 02:16, James Simmons wrote: 

>  
> > BTW, is it possible that someone passes data that is larger (except for bugs)?
> > We have control over what happens in the kernel. Data passed from userspace is
> > a different issue, of course.
> 
> Yes. Soon the standard ioctl for a cursor will be truly in place. This 
> means cfb_imageblit could be a issue from userland.
> 
If you are going to export the cursor API to userland, then soft_cursor
is useless, because it implies that we always have a copy of the screen
under the cursor (so we can restore it when we move the cursor) and it
also implies that fb_imageblit can support transparency (so the mask
bits will work).  For soft cursors, the userland has to take care of it.

For those with hardware cursors, this is exportable to user space. 

The structure fbcursor has one field that is console specific
fbcursor.dest which we basically use to invert the cursor image.  So the
structure that is passed via the cursor ioctl will be slightly
different, it should not have the 'dest' field.  So when it gets passed
to fb_cursor(), dest == NULL.  Then this is how drivers can respond: 

1. soft_cursor - if cursor->dest == NULL, always exit with an error. 

2. hardware cursors which cannot do invert - if cursor->dest == NULL and
rop == ROP_XOR, exit with an error, otherwise (ROP_COPY), proceed. 

3. hardware cursors with invert - cursor->dest will be ignored so it has
full support. 

The rivafb_cursor() and i810fb_cursor() already behaves like #2 already.

Anyway, inverting the cursor image is not commonly used in GUI apps.
Typically, they just need an opaque cursor + a transparency mask to
"shape" the cursor. 

Tony  





-------------------------------------------------------
This SF.NET email is sponsored by:
SourceForge Enterprise Edition + IBM + LinuxWorld = Something 2 See!
http://www.vasoftware.com