XWin 4.3.0-50 crashes with -multiwindow (ping Earle)
Earle F. Philhower III
Wed Mar 24 03:28:00 GMT 2004
Howdy Fabrizio, Harold.
Thanks for the good debug Fabrizio! The 24bpp icon handling was
something I never could test: I couldn't find any apps that had
24bpp icons, all I found were 1- 15-, 16-, or 32-bit ones.
I was assuming the X server always used a packed format, but
PixmapBytePad() looks to be the proper way of handling this.
(Can I ask how you knew? I did a Google on the macro and didn't
come up with anything of interest, I only found stuff in the
header files themselves...)
Harold, the xStride calc looks great, but by removing the conversion
of 15-bpp modes into effective 16-bpp modes will break 15bpp icon
handling. 15-bpp modes are handled exactly the same as 16bpp modes,
except they are not bit-packed (there's 1 extra unused bit every
pixel) so you can't do (bpp/8).
To fix it we can reinstate the if()...
> if (pixmap->drawable.bitsPerPixel == 15)
> effXBPP = 16;
> effXBPP = pixmap->drawable.bitsPerPixel;
> if (pixmap->drawable.depth == 15)
> effXDepth = 16;
> effXDepth = pixmap->drawable.depth;
Or get rid of the effX* variables completely, but modify (~line 218)
< if (effxdepth==16) into
> if (xdepth==16 || xdepth==15)
and modify all of the X image ptr walking
< ptr += posX * (effXBPP / 8);
> ptr += (xbpp==15)?(posX * (16/8):(posX * (xbpp/ 8));
If you'd like I can do it, just let me know which one would be
more appealing! Personally I find the effX variables make the routine
a bit more readable, but that's kind of expected since I put them
there in the first place...
At 07:49 PM 3/23/2004 -0500, you wrote:
>It looks like your conclusions are correct.
>I have included your suggested change in XFree86-xserv-4.3.0-60. Please
>test this on a 24 bit depth system. It seems to work okay on 32 bit depth
>I checked the change into CVS as well. I think that Earle should probably
>take a look at it to verify that it is likely correct since there are
>several calculations to correct the depth and bpp values, which may be
>made redundant by just using PixmapBytePad instead; but the function
>confuses me too much to understand it quickly.
>>Harold and all,
>>I built XWin from source and debugged with gdb, and in that way I was able
>>to track down the bug. It is due to my visual being 24bpp. It does not occur
>>if it is changed to 16bpp. 32bpp is not available, but I am confident
>>would work in that case.
>>Here is what happens:
>>- winScaleXBitmapToWindows is called. The pixmap passed has height 42, width
>>48 and bitsPerPixel 24
>>- effXBPP is 24, xStride is 144 (48*(24/8))
>>- iconData is allocated as an array of 144*42 bytes
>>- then, miGetImage is called. Here the line
>>linelength = PixmapBytePad(w, depth);
>>is executed with w=48 and depth=24. As a result, linelength is 192 (48*4),
>>not 144 (48*3).
>>- in the following for cycle, pDst (initialized as iconData) is incremented
>>by linelength(=192) each time. Soon the pointer overflows the allocated
>>bounds, causing the crash.
>>It seems that handling of 24-bit display is broken. Maybe
>>should use PixmapBytePad to calculate xStride, but I'm only guessing as
>>I'm not an expert.
>-Earle F. Philhower, III
> cdrlabel - ZipLabel - FlpLabel
More information about the Cygwin-xfree