Fwd: [PATCH 2/6] exa: increase/rework safety checks in Prepare/FinishAccess.

[Maarten Maathuis]

Forgot to send to list (again :-( ).

---------- Forwarded message ----------
From: Maarten Maathuis <madman2003 at gmail.com>
Date: Tue, Mar 3, 2009 at 2:10 PM
Subject: Re: [PATCH 2/6] exa: increase/rework safety checks in
Prepare/FinishAccess.
To: Michel Dänzer <michel at daenzer.net>


On Tue, Mar 3, 2009 at 9:10 AM, Michel Dänzer <michel at daenzer.net> wrote:
>
> On Mon, 2009-03-02 at 21:33 +0100, Maarten Maathuis wrote:
> >
> > +    /* Check if we're dealing SRC == DST or similar. */
> > +    if (pPixmap->devPrivate.ptr != NULL) {
> > +       int i;
> > +       for (i = 0; i < 6; i++)
> > +           if (pExaScr->prepare_access[i] == pPixmap)
> > +               break;
>
> Despite your comments, I don't understand how your changes are treating
> the src == dst case specially.

Imagine what happens when you have a CopyArea from one part of the
frontbuffer to another.
Prepare on dst (devPrivate.ptr is now non-NULL), prepare on src, boom.

>
>
> > +     /* Someone is messing with us, so we assert. */
> > +     if (i == 6 || i == index)
> > +         assert(pPixmap->devPrivate.ptr == NULL);
>
> If you need to abort the X server, please only use FatalError(), not
> assert(). The X server currently doesn't catch SIGABRT, so the only
> trace of an assertion failure would be in the X server stderr output.
>
> I'm still not sure it's a good idea to sprinkle FatalErrors all over the
> place though. These sanity checks may be nice for developer/debugging
> builds to catch problems early, but for end-user builds we should try to
> continue whenever reasonably possible.

How do i know it's a debugging build?

>
> In particular, AFAICT the above assertion would fail if some code path
> were to call PrepareAccess twice for the same pixmap and index. That
> would probably not be intended, but it's far from an unrecoverable
> problem.
>
>
> --
> Earthling Michel Dänzer           |                http://www.vmware.com
> Libre software enthusiast         |          Debian, X and DRI developer