[PATCH] EXA: ExaDoPrepareAccess return value fixes.

[Maarten Maathuis]

Acked-by: Maarten Maathuis <madman2003 at gmail.com>

2009/12/8 Michel Dänzer <michel at daenzer.net>:
> From: Michel Dänzer <daenzer at vmware.com>
>
> Only return TRUE if the GPU copy is being accessed, and preserve the return
> value on repeated / nested calls for the same pixmap.
>
> exaPrepareAccessReg_mixed could get inconsistent return values e.g. when the
> same pixmap is both the destination and source of an operation, potentially
> resulting in a crash.
>
> Signed-off-by: Michel Dänzer <daenzer at vmware.com>
> ---
>  exa/exa.c      |   30 +++++++++++++++++++-----------
>  exa/exa_priv.h |    1 +
>  2 files changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/exa/exa.c b/exa/exa.c
> index 23a1388..c699210 100644
> --- a/exa/exa.c
> +++ b/exa/exa.c
> @@ -283,7 +283,7 @@ exaGetOffscreenPixmap (DrawablePtr pDrawable, int *xp, int *yp)
>  }
>
>  /**
> - * Returns TRUE if pixmap can be accessed offscreen.
> + * Returns TRUE if the pixmap GPU copy is being accessed.
>  */
>  Bool
>  ExaDoPrepareAccess(PixmapPtr pPixmap, int index)
> @@ -291,7 +291,7 @@ ExaDoPrepareAccess(PixmapPtr pPixmap, int index)
>     ScreenPtr pScreen = pPixmap->drawable.pScreen;
>     ExaScreenPriv (pScreen);
>     ExaPixmapPriv(pPixmap);
> -    Bool has_gpu_copy;
> +    Bool has_gpu_copy, ret;
>     int i;
>
>     if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS))
> @@ -304,7 +304,7 @@ ExaDoPrepareAccess(PixmapPtr pPixmap, int index)
>     for (i = 0; i < EXA_NUM_PREPARE_INDICES; i++) {
>        if (pExaScr->access[i].pixmap == pPixmap) {
>            pExaScr->access[i].count++;
> -           return TRUE;
> +           return pExaScr->access[i].retval;
>        }
>     }
>
> @@ -323,29 +323,33 @@ ExaDoPrepareAccess(PixmapPtr pPixmap, int index)
>
>     has_gpu_copy = exaPixmapHasGpuCopy(pPixmap);
>
> -    if (has_gpu_copy && pExaPixmap->fb_ptr)
> +    if (has_gpu_copy && pExaPixmap->fb_ptr) {
>        pPixmap->devPrivate.ptr = pExaPixmap->fb_ptr;
> -    else
> +       ret = TRUE;
> +    } else {
>        pPixmap->devPrivate.ptr = pExaPixmap->sys_ptr;
> +       ret = FALSE;
> +    }
>
>     /* Store so we can handle repeated / nested calls. */
>     pExaScr->access[index].pixmap = pPixmap;
>     pExaScr->access[index].count = 1;
>
>     if (!has_gpu_copy)
> -       return FALSE;
> +       goto out;
>
>     exaWaitSync (pScreen);
>
>     if (pExaScr->info->PrepareAccess == NULL)
> -       return TRUE;
> +       goto out;
>
>     if (index >= EXA_PREPARE_AUX_DEST &&
>        !(pExaScr->info->flags & EXA_SUPPORTS_PREPARE_AUX)) {
>        if (pExaPixmap->score == EXA_PIXMAP_SCORE_PINNED)
>            FatalError("Unsupported AUX indices used on a pinned pixmap.\n");
>        exaMoveOutPixmap (pPixmap);
> -       return FALSE;
> +       ret = FALSE;
> +       goto out;
>     }
>
>     if (!(*pExaScr->info->PrepareAccess) (pPixmap, index)) {
> @@ -353,11 +357,15 @@ ExaDoPrepareAccess(PixmapPtr pPixmap, int index)
>            !(pExaScr->info->flags & EXA_MIXED_PIXMAPS))
>            FatalError("Driver failed PrepareAccess on a pinned pixmap.\n");
>        exaMoveOutPixmap (pPixmap);
> -
> -       return FALSE;
> +       ret = FALSE;
> +       goto out;
>     }
>
> -    return TRUE;
> +    ret = TRUE;
> +
> +out:
> +    pExaScr->access[index].retval = ret;
> +    return ret;
>  }
>
>  /**
> diff --git a/exa/exa_priv.h b/exa/exa_priv.h
> index f9d9d87..5919977 100644
> --- a/exa/exa_priv.h
> +++ b/exa/exa_priv.h
> @@ -193,6 +193,7 @@ typedef struct {
>     struct {
>        PixmapPtr pixmap;
>        int count;
> +       Bool retval;
>     } access[EXA_NUM_PREPARE_INDICES];
>
>     /* Holds information on fallbacks that cannot be relayed otherwise. */
> --
> 1.6.4.3
>
> _______________________________________________
> xorg-devel mailing list
> xorg-devel at lists.x.org
> http://lists.x.org/mailman/listinfo/xorg-devel
>