[PATCH] xfree86: belately init RandR12 if xinerama fails. (#24627)

[Aaron Plattner]

On Tue, Dec 08, 2009 at 05:55:14PM -0800, Peter Hutterer wrote:
> On Tue, Dec 08, 2009 at 05:24:06PM -0800, Aaron Plattner wrote:
> > On Tue, Dec 08, 2009 at 03:52:27PM -0800, Peter Hutterer wrote:
> > > Xorg +xinerama crashes immediately due to whacky dependency between Xinerama
> > > and RandR12. The latter doesn't initialize if Xinerama is enabled, but if
> > > only one screen is found, Xinerama is disabled again and RandR12 tries to
> > > access data it never initialized.
> > > 
> > > Dependency chain is:
> > > - ProcessCommandLine sets noPanoramiXExtension to FALSE
> > > - xf86RandR12Init() is a noop
> > > - PanoramiXExtensionInit sets noPanoramiXExtension to TRUE
> > > - xf86RandR12CreateScreenResources tries to use the devPrivates key it never
> > >   initialized.
> > > 
> > > This hack checks if the key was initialized if noPanoramiXExtension is TRUE.
> > > If not, re-initialize RandR12, this time for reals.
> > > 
> > > X.Org Bug 24627 <http://bugs.freedesktop.org/show_bug.cgi?id=24627>
> > > 
> > > Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> > > ---
> > > 
> > > Don't know if there are any side-effects of doing this.
> > > 
> > >  hw/xfree86/modes/xf86RandR12.c |    2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
> > > index 6ea9d26..dd80ee0 100644
> > > --- a/hw/xfree86/modes/xf86RandR12.c
> > > +++ b/hw/xfree86/modes/xf86RandR12.c
> > > @@ -767,6 +767,8 @@ xf86RandR12CreateScreenResources (ScreenPtr pScreen)
> > >      /* XXX disable RandR when using Xinerama */
> > >      if (!noPanoramiXExtension)
> > >  	return TRUE;
> > > +    else if (!xf86RandR12Key)
> > > +        xf86RandR12Init(pScreen);
> > 
> > This makes me nervous because it starts calling a big chain of
> > ScreenInit-style functions at CreateScreenResources time.  I can't find any
> > concrete examples of things that would fail, but it perturbs the init order
> > pretty drastically, but only in obscure Xinerama cases.  Would it be
> > acceptable to just bail out without doing anything if (!xf86RandR12Key)?
> 
> that'd be the following patch then which - afaict - works too. Though the
> same predicament stands, I don't know if there are any side-effects. Keith,
> any comments?
> 
> From ca7aa60a580e8c15016b05b75c62f2c45eee01b7 Mon Sep 17 00:00:00 2001
> From: Peter Hutterer <peter.hutterer at who-t.net>
> Date: Wed, 9 Dec 2009 09:45:39 +1000
> Subject: [PATCH] xfree86: don't set up RandR12 screen resources not initialized. (#24627)
> 
> Xorg +xinerama crashes immediately due to whacky dependency between Xinerama
> and RandR12. The latter doesn't initialize if Xinerama is enabled, but if
> only one screen is found, Xinerama is disabled again and RandR12 tries to
> access data it never initialized.
> 
> Dependency chain is:
> - ProcessCommandLine sets noPanoramiXExtension to FALSE
> - xf86RandR12Init() is a noop
> - PanoramiXExtensionInit sets noPanoramiXExtension to TRUE
> - xf86RandR12CreateScreenResources tries to use the devPrivates key it never
>   initialized.
> 
> If we didn't initialize RandR12, don't try to set anything up either.
> 
> X.Org Bug 24627 <http://bugs.freedesktop.org/show_bug.cgi?id=24627>
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  hw/xfree86/modes/xf86RandR12.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/xfree86/modes/xf86RandR12.c b/hw/xfree86/modes/xf86RandR12.c
> index 6ea9d26..1a4f2bd 100644
> --- a/hw/xfree86/modes/xf86RandR12.c
> +++ b/hw/xfree86/modes/xf86RandR12.c
> @@ -765,7 +765,7 @@ xf86RandR12CreateScreenResources (ScreenPtr pScreen)
>      int			mmWidth, mmHeight;
>  #ifdef PANORAMIX
>      /* XXX disable RandR when using Xinerama */
> -    if (!noPanoramiXExtension)
> +    if (!noPanoramiXExtension || !xf86RandR12Key)
>  	return TRUE;
>  #endif

I'd recommend moving the !xf86RandR12Key case outside the #ifdef to catch
any other cases where it failed to initialize, but otherwise,
Acked-by: Aaron Plattner <aplattner at nvidia.com>