[PATCH] fb: Fix memcpy abuse

[Soeren Sandmann]

Adam Jackson <ajax at redhat.com> writes:

> -    if (alu == GXcopy && pm == FB_ALLONES && !reverse &&
> +    careful = !((srcLine < dstLine && srcLine + width * (bpp/8) > dstLine) ||
> +                (dstLine < srcLine && dstLine + width * (bpp/8) > srcLine)) ||
> +              (bpp % 8);

The srcLine and dstLine variables are pointers to FbBits which is an
alias for uint32_t, but "width * (bpp/8)" computes the number of *bytes*
in the line. For this to work, I think srcLine and dstLine pointers need
casts to (CARD8 *).

Also, in principle, the first "<" in each line should be "<=". Otherwise
the code would miss the case where the two lines overlap because they
are identical. This is only a bug in principle since the lines shouldn't
be identical, or if they are, memcpy() probably would be fine.

Finally, I think the overlapping check could be simplified by
considering how to check that the lines *don't* overlap. This is the
case precisely when the end of the src line is before or equal to the
beginning of the dst line, or the beginning of the src line is after or
equal to the end of the dst line:

    no_overlap = ((CARD8 *)srcLine + width * (bpp / 8) <= dstLine ||
                  (CARD8 *)dstLine + width * (bpp / 8) <= srcLine);

Negating and applying De Morgan then gives:

    overlap = (CARD8 *)srcLine + width * (bpp / 8) > dstLine &&
              (CARD8 *)dstLine + width * (bpp / 8) > srcLine;

So the careful check then looks something like this:

    careful = ((CARD8 *)srcLine + width * (bpp / 8) > dstLine &&
               (CARD8 *)dstLine + width * (bpp / 8) > srcLine) ||
              (bpp % 8);


Soren