[PATCH] Fix a couple more possible errors with input-only windows

[Jamey Sharp]

On Thu, Jun 10, 2010 at 7:31 AM, Keith Packard <keithp at keithp.com> wrote:
> Using type == DRAWABLE_WINDOW to differentiate between pixmaps and
> windows isn't sufficient as input-only windows will end up in the
> pixmap case. This patch changes a few more code paths to use
> WindowDrawable instead.
>
> Signed-off-by: Keith Packard <keithp at keithp.com>
> ---
>
> The xinerama patch replaces a hand-written test of the drawable type
> with a call to the WindowDrawable macro, the other cases are probably
> just defensive patches as I don't think an InputOnly window can get into
> those code paths, but I think having the explicit checks requires the
> reader to have less global knowledge of the server which makes reading
> the code easier.

I was going to suggest keeping your earlier patch that did this for
DRI because of the code clarity argument, but I couldn't figure out
how to express it. Seems like it's worth having both the clarity of
your WindowDrawable patch and the efficiency of the patch that makes
DRI reject input-only windows.

Anyway, I like this patch for the same reason, so:
Reviewed-by: Jamey Sharp <jamey at minilop.net>

>  Xext/panoramiXprocs.c |    2 +-
>  hw/xnest/Drawable.h   |    2 +-
>  miext/damage/damage.c |    4 ++--
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Xext/panoramiXprocs.c b/Xext/panoramiXprocs.c
> index b744e4d..67b4030 100644
> --- a/Xext/panoramiXprocs.c
> +++ b/Xext/panoramiXprocs.c
> @@ -557,7 +557,7 @@ int PanoramiXGetGeometry(ClientPtr client)
>        rep.width = root->pixWidth;
>        rep.height = root->pixHeight;
>     } else
> -    if ((pDraw->type == UNDRAWABLE_WINDOW) || (pDraw->type == DRAWABLE_WINDOW))
> +    if (WindowDrawable(pDraw->type))
>     {
>         WindowPtr pWin = (WindowPtr)pDraw;
>        rep.x = pWin->origin.x - wBorderWidth (pWin);
> diff --git a/hw/xnest/Drawable.h b/hw/xnest/Drawable.h
> index d94916e..4268b7b 100644
> --- a/hw/xnest/Drawable.h
> +++ b/hw/xnest/Drawable.h
> @@ -19,7 +19,7 @@ is" without express or implied warranty.
>  #include "XNPixmap.h"
>
>  #define xnestDrawable(pDrawable) \
> -  ((pDrawable)->type == DRAWABLE_WINDOW ? \
> +  (WindowDrawable((pDrawable)->type) ? \
>    xnestWindow((WindowPtr)pDrawable) : \
>    xnestPixmap((PixmapPtr)pDrawable))
>
> diff --git a/miext/damage/damage.c b/miext/damage/damage.c
> index 7c2f8a0..1cf0513 100644
> --- a/miext/damage/damage.c
> +++ b/miext/damage/damage.c
> @@ -84,7 +84,7 @@ getDrawableDamageRef (DrawablePtr pDrawable)
>  {
>     PixmapPtr   pPixmap;
>
> -    if (pDrawable->type == DRAWABLE_WINDOW)
> +    if (WindowDrawable(pDrawable->type))
>     {
>        ScreenPtr   pScreen = pDrawable->pScreen;
>
> @@ -300,7 +300,7 @@ damageRegionAppend (DrawablePtr pDrawable, RegionPtr pRegion, Bool clip,
>         * Need to move everyone to screen coordinates
>         * XXX what about off-screen pixmaps with non-zero x/y?
>         */
> -       if (pDamage->pDrawable->type != DRAWABLE_WINDOW)
> +       if (!WindowDrawable(pDamage->pDrawable->type))
>        {
>            draw_x += ((PixmapPtr) pDamage->pDrawable)->screen_x;
>            draw_y += ((PixmapPtr) pDamage->pDrawable)->screen_y;
> --
> 1.7.1
>
>
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>