[PATCH 02/20] mi: miPutImage with XYPixmap failed at depth 32 on 64-bit machines

[Aaron Plattner]

On 03/18/2014 10:09 PM, Keith Packard wrote:
> The X server still has 'unsigned long' in a few places to hold 32 bit
> values. One of those is in miPutImage where it's holding the temporary
> planemask for XYPixmap format images.
>
> It computed the highest plane in the source image with 1 << (depth -
> 1). On 64-bit machines, taking that value and storing it in an
> unsigned long promotes it to a signed 64-bit value
> (0xffffffff80000000).
>
> Then, it loops over that value, shifting one bit right each time,
> waiting for it to go to zero.. That takes 64 iterations, and ends up
> with some mystic planemask values *and* walking off the end of the
> source image data and out into space.
>
> A simple cast is all that is required to compute the correct initial
> plane mask (0x0000000080000000), at which point the loop operates
> correctly once again.
>
> Checking the fbPutImage code, I note that this same bug was fixed
> in 2006 by Aaron Plattner in commit
> f39fd4242902eaa862321d39337f429dd14ebacf
>
> Signed-off-by: Keith Packard <keithp at keithp.com>

Sorry for not noticing this when I fixed the other one.  Also, since 
2006 I learned that you can use 1UL instead of a cast to unsigned long, 
if that seems cleaner.

Reviewed-by: Aaron Plattner <aplattner at nvidia.com>

I still think we should make the server replace all XYPixmap images with 
a picture of something rude.

> ---
>   mi/mibitblt.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mi/mibitblt.c b/mi/mibitblt.c
> index b0d14ae..3ed4ed1 100644
> --- a/mi/mibitblt.c
> +++ b/mi/mibitblt.c
> @@ -730,7 +730,7 @@ miPutImage(DrawablePtr pDraw, GCPtr pGC, int depth,
>           ChangeGC(NullClient, pGC, GCForeground | GCBackground, gcv);
>           bytesPer = (long) h *BitmapBytePad(w + leftPad);
>
> -        for (i = 1 << (depth - 1); i != 0; i >>= 1, pImage += bytesPer) {
> +        for (i = (unsigned long) 1 << (depth - 1); i != 0; i >>= 1, pImage += bytesPer) {
>               if (i & oldPlanemask) {
>                   gcv[0].val = (XID) i;
>                   ChangeGC(NullClient, pGC, GCPlaneMask, gcv);
>

-- 
Aaron