[Qemu-devel] Re: Another SIGFPE in display code, now in cirrus

[Stefano Stabellini]

On Wed, 12 May 2010, Avi Kivity wrote:
> > I suggest to start using the display pitch (with the proper sign)
> > instead of cirrus_blt_srcpitch in cirrus_do_copy at least when
> > cirrus_blt_srcpitch doesn't have a proper value.
> >    
> 
> Why switch from one bug to the other?
> 
> It's perfectly possible to take into account both values.  Of course 
> when abs(blt_pitch) != display pitch we can't use console_copy, but so what.
> 

I see: you want to support the case when abs(src_blt_pitch) != display_pitch,
while I though it is some sort of error.

Considering that src_blt_pitch can even be 0 (as in the bug report), we
would still need to calculate sx and sy using display_pitch instead, so
the only place in which it would make a difference is when src_blt_pitch
is passed as an argument to cirrus_rop.
I guess even a src blt pitch of 0 could be useful there, however in
practice I think the only rop function that was written with this case in
mind has:

dstpitch -= bltwidth;
srcpitch -= bltwidth;

if (dstpitch < 0 || srcpitch < 0) {
    /* is 0 valid? srcpitch == 0 could be useful */
    return;
}

at the beginning and the others probably just don't deal with the case
with possibly buggy consequences.
Also the documentation I have states:

3CEh index 26h W(R/W):  BLT Source Pitch                              (5426 +)
bit 0-11  (5426-28) Number of bytes in a scanline at the source.
    0-12  (5429 +)  do

if the source BLT is supposed to be the number of bytes in a scanline at
the source, then 0 is not a correct value for it.