[PATCH] fix xserver crash when configuring TV Out on intel driver

rglowery at exemail.com.au rglowery at exemail.com.au
Wed Oct 24 14:53:35 PDT 2007


> On Wed, 2007-10-24 at 21:15 +1000, rglowery at exemail.com.au wrote:
>> I believe I have found the cause of the xserver crash when running
>> "xrandr
>> --output TV --set TV_FORMAT 576p --auto" while no other output devices
>> are
>> connected.
>>
>> The issue is that i830_tv_get_modes() is leaving the DisplayModeRec prev
>> pointer uninitialised which is confusing xf86DeleteMode()
>>
>> Patch below appears to fix the problem, however looking at the comments
>> in
>> xf86DeleteMode(), it is supposed to support a singly linked list, but
>> the
>> implementation obviously does not.
>>
>> diff --git a/src/i830_tv.c b/src/i830_tv.c
>> index 940250e..d96a501 100644
>> --- a/src/i830_tv.c
>> +++ b/src/i830_tv.c
>> @@ -1451,6 +1451,9 @@ i830_tv_get_modes(xf86OutputPtr output)
>>
>>         mode_ptr->type = M_T_DRIVER;
>>         mode_ptr->next = ret;
>> +       mode_ptr->prev = NULL;
>> +       if (ret)
>> +           ret->prev = mode_ptr;
>>         ret = mode_ptr;
>>      }
>
> Committed.  Thanks!
>
> Yeah, xf86DeleteMode works for all those types of lists, but only as
> long as all the fields are initialized :)

Thanks for the commit.  I'm getting closer to being happy with TVOut in
mythtv.  Just need to solve the washed out colour issue now though someone
with doco ... Hint Hint :)

In hw/xfree86/common/xf86Mode.c we have

1930 /*
1931  * xf86DeleteMode1932  *
1933  * This function removes a mode from a list of modes.
1934  *
1935  * There are different types of mode lists:1936  *
1937  *  - singly linked linear lists, ending in NULL
1938  *  - doubly linked linear lists, starting and ending in NULL
1939  *  - doubly linked circular lists
1940  *
1941  */


If all the prev field is initialized, the list is either a doubly linked
linear list, starting and ending in NULL or doubly linked circular list.

Should the comment "singly linked linear lists, ending in NULL" be
removed, I'm not sure what sort of list this is referring to. 
Initializing each nodes prev member to NULL would not work in the current
implementation.

-Rob




More information about the xorg mailing list