Antonino Daplas wrote:
But current users of cyblafb will be affected if your patch
does have a problem.
They definitely will be affected when they lock their system
while trying rotation options ...
+ // That should never happen, but it would be fatal
It won't :-)
The graphics engine would not react kindly, and it does not really hurt.
+ if (image->width == 0 || image->height == 0) {
+ output("imageblit: width/height 0 detected\n");
+ return;
+ }
+
+ if (bpp < 8 || bpp > 32 || bpp % 8 != 0 ||
+ info->pixmap.scan_align > 4 ) {
Why this paranoid check? The check_var() function already
guaranteed that these conditions will not happen.
Yes, I am a bit paranoid. That paranoia led to the discovery of some bugs
nobody knew or cared about. But you are right, this check might be a bit too
paranoid.
Do you really have to support scan_align 1 and 2? Why not just stick
with scan_align of 4, the code is so much easier to understand? I can't
find anything useful with this, even for debugging.
Well, you are shure that there is really not a single bug left in the
bitmap construction
code? And that the code will never be touched again because it already
is optimal? I
think support for all alignment possibilities will be handy in the near
future, and
although it could be hidden by an #ifdef or stay a private patch, I
prefer to include it.
Currently bitmap construction takes longer than blitting the image to
the screen with
cyblafb, and I think I will have a very close look at that code soon.
BTW, something fundamental: Isn´t the pixelmap alignment really a
property of the
image bitmap like the depth of the image data?
+ // try to be smart about (x|y)res(_virtual) problems.
+ //
+ if (var->xres % 8 != 0)
return -EINVAL;
Isn't this too much? Why not var->xres = (var->xres + 7) & ~7?
Do you really think that this is a good idea? I would like to ease the
use of
e.g. fbset in scripts by returning -EINVAL when something as fundamental as
the selected xres is not acceptable. Ok, it´s always possible to parse
the output
of fbset -s in those cases.
+ if (var->xres_virtual % 8 != 0)
+ var->xres_virtual &= ~7;
Or just var->xres_virtual &= ~7 without the if (...)
Yes. That saves a few bytes.
Wrong boolean check? Should be if (vesafb & 4). Or might as
well get rid of this check, it's redundant.
Shouldn't this be if (vesafb & 4)?
and this...
and this...?
No, no, no, no.
cu,
Knut
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
