Forgot to send to list (again :-( ). ---------- Forwarded message ---------- From: Maarten Maathuis <[email protected]> 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 <[email protected]>
On Tue, Mar 3, 2009 at 9:10 AM, Michel Dänzer <[email protected]> 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 _______________________________________________ xorg-devel mailing list [email protected] http://lists.x.org/mailman/listinfo/xorg-devel
