[1/2] xf86: actually set the compat output in the failure case

Submitted by Dave Airlie on Jan. 9, 2013, 3:14 a.m.

Details

Message ID 1357701295-19913-1-git-send-email-airlied@gmail.com
State Accepted
Commit 3ec35c45ca17f5ed6fd02c50fc49ae7b8d128dcb
Headers show

Not browsing as part of any series.

Commit Message

Dave Airlie Jan. 9, 2013, 3:14 a.m. 
From: Dave Airlie <airlied@redhat.com>

The previous fix for the previous fix, didn't fully work,

If we don't set compat_output we end up doing derferences
of arrays with -1, leading to valgrind warnings.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 hw/xfree86/modes/xf86Crtc.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox 

diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
index 77a0218..45764fc 100644
--- a/hw/xfree86/modes/xf86Crtc.c
+++ b/hw/xfree86/modes/xf86Crtc.c
@@ -1848,8 +1848,10 @@  SetCompatOutput(xf86CrtcConfigPtr config)
     }
 
     /* All outputs are disconnected, select one to fake */
-    if (!output && config->num_output)
-        output = config->output[0];
+    if (!output && config->num_output) {
+        config->compat_output = 0;
+        output = config->output[config->compat_output];
+    }
 
     return output;
 }

Comments

Peter Hutterer Jan. 10, 2013, 11 p.m. 
On Wed, Jan 09, 2013 at 01:14:54PM +1000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> The previous fix for the previous fix, didn't fully work,
> 
> If we don't set compat_output we end up doing derferences
> of arrays with -1, leading to valgrind warnings.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

Cheers,
   Peter

> ---
>  hw/xfree86/modes/xf86Crtc.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/xfree86/modes/xf86Crtc.c b/hw/xfree86/modes/xf86Crtc.c
> index 77a0218..45764fc 100644
> --- a/hw/xfree86/modes/xf86Crtc.c
> +++ b/hw/xfree86/modes/xf86Crtc.c
> @@ -1848,8 +1848,10 @@ SetCompatOutput(xf86CrtcConfigPtr config)
>      }
>  
>      /* All outputs are disconnected, select one to fake */
> -    if (!output && config->num_output)
> -        output = config->output[0];
> +    if (!output && config->num_output) {
> +        config->compat_output = 0;
> +        output = config->output[config->compat_output];
> +    }
>  
>      return output;
>  }
> -- 
> 1.8.1
> 
> _______________________________________________
> xorg-devel@lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
Chris Wilson Jan. 10, 2013, 11:21 p.m. 
On Wed,  9 Jan 2013 13:14:54 +1000, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> The previous fix for the previous fix, didn't fully work,
> 
> If we don't set compat_output we end up doing derferences
> of arrays with -1, leading to valgrind warnings.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Ok, that makes sense. output can only be NULL at this point iff there
are no outputs and the existing compat_output is either unset or
invalid. Then the value of config->compat_output always matches the
xf86OutputPtr returned. And indeed explains the -1 that caused me pain
recently, thanks.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris