On Fri, Jun 11, 2010 at 11:22 AM, Mikhail Gusarov <[email protected]> wrote: > The only reference to it in server and drivers is in XAA overlay code which > would segfault as no miInitOverlay is called ever. > > Signed-off-by: Mikhail Gusarov <[email protected]> > --- > hw/xfree86/loader/sdksyms.sh | 2 - > hw/xfree86/xaa/xaaOverlay.c | 29 +- > mi/Makefile.am | 4 +- > mi/mioverlay.c | 1946 > ------------------------------------------ > mi/mioverlay.h | 32 - > 5 files changed, 2 insertions(+), 2011 deletions(-)
Awesome. Nice diffstat! It wasn't obvious to me that the patch was correct, because you pretend that miOverlayCopyUnderlay returns false instead of crashing. But on further inspection I see that XAACopyWindow8_32 could only set doUnderlay to true if it's called from miOverlayMoveWindow or miOverlayResizeWindow, which can only be called if miInitOverlay has hooked those functions, and I can confirm that no driver or server code calls that. I'd appreciate a note in the commit message making that more clear. How did you discover this? Did you actually encounter one of these crashes, or just notice that the code was unused? You might comment on that in the commit message too. Then, is xaaOverlay used anywhere, or can it be removed too? If nobody has seen this crash, maybe nobody cares about this code. I think there is one error in the patch though: > diff --git a/hw/xfree86/loader/sdksyms.sh b/hw/xfree86/loader/sdksyms.sh > index 13c5ae5..604762d 100755 > --- a/hw/xfree86/loader/sdksyms.sh > +++ b/hw/xfree86/loader/sdksyms.sh > @@ -26,7 +26,6 @@ cat > sdksyms.c << EOF > /* > #include "fb.h" > #include "fbrop.h" > -#include "fboverlay.h" > #include "wfbrename.h" > #include "fbpict.h" > */ Isn't this wrong? fboverlay.h still exists and is still included in some places, including drivers. With that fixed and ideally a more complete commit message, Reviewed-by: Jamey Sharp <[email protected]> Jamey _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
