[RFC PATCH xserver] xwayland: Fix non-argb cursor conversion

Emil Velikov emil.l.velikov at gmail.com
Thu Nov 16 13:55:13 UTC 2017


Hi Olivier,

On 27 September 2017 at 17:01, Olivier Fourdan <ofourdan at redhat.com> wrote:
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=103012
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  Note: I am not familiar with this so I have no idea whether or not the
>  fix is correct (thus the RFC), but it does fix the test case provided
>  in bug 103012.
>
>  hw/xwayland/xwayland-cursor.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
> index c95f4e830..cf8395f1d 100644
> --- a/hw/xwayland/xwayland-cursor.c
> +++ b/hw/xwayland/xwayland-cursor.c
> @@ -42,7 +42,7 @@ expand_source_and_mask(CursorPtr cursor, CARD32 *data)
>          (cursor->foreGreen & 0xff00) | (cursor->foreGreen >> 8);
>      bg = ((cursor->backRed & 0xff00) << 8) |
>          (cursor->backGreen & 0xff00) | (cursor->backGreen >> 8);
> -    stride = (bits->width / 8 + 3) & ~3;
> +    stride = BitmapBytePad(bits->width);

Bit of a disclaimer - I'm far from expert in the area.

The pattern seems like a common mistake, where one would align after
the division instead of the other way around.
As width is != 32 the code will provide larger than needed stride,
which seems to be exactly what the commit summary says.

One might mention exactly what's happening the commit message, since
it's not immediately obvious.

With the above disclaimer in mind, patch is
Reviewed-by: Emil Velikov <emil.velikov at collabora.com>

-Emil


More information about the xorg-devel mailing list