[PATCH 1/5] mi: Remove unused overlay support
Jamey Sharp
jamey at minilop.net
Fri Jun 11 11:29:26 PDT 2010
On Fri, Jun 11, 2010 at 11:22 AM, Mikhail Gusarov
<dottedmag at dottedmag.net> 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 <dottedmag at dottedmag.net>
> ---
> 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 <jamey at minilop.net>
Jamey
More information about the xorg-devel
mailing list