[PATCH 1/3] xf86: return NULL for xf86CompatOutput

[vdb at picaros.org]

> Am 18.07.2013 13:37, schrieb vdb at picaros.org:
> > http://bugs.freedesktop.org/show_bug.cgi?id=65210
> > 
> > Commit 37d956e3ac9513b74078882dff489f9b0a7a5a28 presets 
> > config->compat_output = -1 to signal an unset compat_output.  
> > 
> > Since compat_output is used to index config->output[] during initial 
> > screen configuration a bad dereference occurs.  At this point the 
> > compatibility output can't be known so a NULL return from 
> > xf86CompatOutput(ScrnInfoPtr pScrn) is a logical solution. 

> > Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
> > ---
> >  hw/xfree86/modes/xf86Crtc.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
> > index 802303f..1ac8485 100644
> > --- a/hw/xfree86/modes/xf86Crtc.h
> > +++ b/hw/xfree86/modes/xf86Crtc.h
> > @@ -731,6 +731,8 @@ xf86CompatOutput(ScrnInfoPtr pScrn)
> >  {
> >      xf86CrtcConfigPtr config = XF86_CRTC_CONFIG_PTR(pScrn);
> >  
> 
> in patch 3/3 you use
> 
>            if (config->compat_output >= 0
> 	    && config->compat_output < config->num_output) {
> 
> maybe your should do the same here. just make it the same.
> 
> re,
>  wh
> 
> > +    if (config->compat_output < 0)
> > +        return NULL;
> >      return config->output[config->compat_output];
> >  }

Well, this is a chicken and egg problem.  The current xf86Crtc.c 
SetCompatOutput(config) implementation allows for 
config->compat_output >= config->num_output when num_output == 0.  

After patch 3/3 the invariant relation for compat_output becomes:

  compat_output == -1  <==>  there is no compat output because
    1. initial configuration is in process,
    2. all outputs were destroyed,

and

  compat_output >= 0   <==>  compat_output is valid
                        ==>  compat_output < num_output.

So there is no need to change xf86Crtc.h xf86CompatOutput(pScrn) into 

    if (config->compat_output < 0
        || config->compat_output >= config->num_output)
        return NULL;
    return config->output[config->compat_output];

since the second or-part will never test true.

Servaas.