[PATCH] RandR: Remove sensless checks for xf86RandR12Key

[Keith Packard]

Egbert Eich <eich at freedesktop.org> writes:

> When xf86RandR12Key is not set we will not get to the places where
> these tests are done as the functions in question are not called.
> In most cases we would have crashed before these checks anyway.

This is really hard to review, and that's almost entirely my fault...

xf86RandR12Key will be set whenever RandR12 is initialized by the
driver, but only when Xinerama is disabled. So, drivers may be using
RandR12 interfaces for all video mode setting and yet the RandR12
extension bits may not be enabled. That means we need to be careful at
any interface where the caller assumes that RandR12 is running because
xf86CrtcScreenInit has been called.

We can safely ignore static functions within xf86RandR12.c -- all of
the function pointers that would land on those are initialized after
xf86RandR12Key is set.

Some of the RandR12 interfaces are called only from xf86Crtc.c. Those
interfaces are:

	xf86RandR12CreateScreenResources
                Checks !noPanoramiXExtension
	xf86RandR12CloseScreen
                Checks xf86RandR12Key
	xf86RandR12Init
                Checks !noPanoramiXExtension

A few are called from drivers and xf86Crtc.c:

	xf86RandR12PreInit
	        (This is a stub and should be deleted)
        
	xf86RandR12SetRotations
                Checks xf86RandR12Key
	xf86RandR12SetTransformSupport
                Checks xf86RandR12Key
	xf86RandR12TellChanged
                Checks xf86RandR12Key

As none of the callers to these functions can tell whether RandR12 is
enabled, it's a good thing all of these functions do.

Ok, now as to the specifics of Egbert's patch.

>
> Signed-off-by: Egbert Eich <eich at freedesktop.org>
> ---
>  hw/xfree86/modes/xf86RandR12.c | 13 ++++---------
>  1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
> index b1c306a..5c1ba71 100644
> --- a/hw/xfree86/modes/xf86RandR12.c
> +++ b/hw/xfree86/modes/xf86RandR12.c
> @@ -683,11 +683,9 @@ xf86RandR12ScreenSetSize(ScreenPtr pScreen,
>      Bool ret = FALSE;
>      int c;
>  
> -    if (xf86RandR12Key) {
> -        if (randrp->virtualX == -1 || randrp->virtualY == -1) {
> -            randrp->virtualX = pScrn->virtualX;
> -            randrp->virtualY = pScrn->virtualY;
> -        }
> +    if (randrp->virtualX == -1 || randrp->virtualY == -1) {
> +        randrp->virtualX = pScrn->virtualX;
> +        randrp->virtualY = pScrn->virtualY;
>      }
>      if (pRoot && pScrn->vtSema)
>          (*pScrn->EnableDisableFBAccess) (pScrn, FALSE);
> @@ -730,7 +728,7 @@ xf86RandR12ScreenSetSize(ScreenPtr pScreen,
>      if (pRoot && pScrn->vtSema)
>          (*pScrn->EnableDisableFBAccess) (pScrn, TRUE);
>  #if RANDR_12_INTERFACE
> -    if (xf86RandR12Key && pScreen->root && ret)
> +    if (pScreen->root && ret)
>          RRScreenSizeNotify(pScreen);
>  #endif
>      return ret;

xf86RandR12ScreenSetSize is called from xf86RandR12CreateScreenResources
and set as rrScreenSetSize. Both have already checked that RandR12 is
enabled, so Egbert's change is correct


> @@ -826,9 +824,6 @@ xf86RandR12CreateScreenResources(ScreenPtr pScreen)
>          xf86RandR12ScreenSetSize(pScreen, width, height, mmWidth, mmHeight);
>      }
>  
> -    if (xf86RandR12Key == NULL)
> -        return TRUE;
> -
>      if (randrp->virtualX == -1 || randrp->virtualY == -1) {
>          randrp->virtualX = pScrn->virtualX;
>          randrp->virtualY = pScrn->virtualY;

This check comes after the check for !noPanoramiXExtension, which is
equivalent to checking for xf86RandR12Key != NULL, so this patch is also
correct.

xf86RandR12CreateScreenResources12 also has a spurious check for
xf86RandR12Key == NULL which should be removed. That's the only other
place I found which needs changing.

With that additional check removed, this patch is:

Reviewed-by: Keith Packard <keithp at keithp.com>

p.s. a few published functions don't appear to be called from anywhere:

	xf86RandR12GetOriginalVirtualSize
                (not used *anywhere*)

	xf86RandR12GetRotation
                (not used *anywhere*)

	xf86RandR12SetConfig
                (used for the rrSetConfig hook)

We should delete the first two and make the third static.

-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 810 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20150331/6c869f2b/attachment.sig>