[PATCH 1/2] DRI2: Avoid call to NULL pointer

[Mario Kleiner]

On Oct 7, 2010, at 10:06 AM, Pauli Nieminen wrote:

> On 07/10/10 00:42 +0200, ext Mario Kleiner wrote:
>> On Oct 6, 2010, at 1:05 PM, Pauli Nieminen wrote:
>>
>>> DDX driver may implement schedule swap without GetMSC. In that  
>>> case we
>>> can't call GetMSC in DRI2SwapBuffers.
>>>
>>
>> I don't think this check is neccessary. Afaik if the ds->GetMSC entry
>> point isn't defined then ds->ScheduleSwap isn't defined either, so
>> the !ds->ScheduleSwap  check at the beginning of the routine triggers
>> and the fallback path is taken, no?
>>
>
> no. It is perfectly valid to have ScheduleSwap without GetMSC. This is
> important for driver that can implement page flipping but doesn't  
> have simple
> way to expose msc count.
>

Ok, i'm not sure if such drivers should exist, and this snippet of  
code from DRI2ScreenInit...

     if (info->version >= 4) {
         ds->ScheduleSwap = info->ScheduleSwap;
         ds->ScheduleWaitMSC = info->ScheduleWaitMSC;
         ds->GetMSC = info->GetMSC;
         cur_minor = 3;
     } else {
         cur_minor = 1;
     }

... implied for me that ScheduleSwap and GetMSC come in one "all or  
nothing" package. But GetMSC could be a null pointer anyway, so it  
doesn't hurt to protect against such configs and i think your patch  
does the right thing in this case.

You can add a...

Signed-off-by: Mario Kleiner <mario.kleiner at tuebingen.mpg.de>

to your patch if you like. As far as i can see, such a driver would  
be limited to simple glXSwapBuffers() calls though if it doesn't know  
its own msc. It would also have to ignore the target_msc, divisor and  
remainder arguments to ScheduleSwap and only take swapInterval into  
account, as neither a glx client nor the server could provide any  
meaningful values for these arguments without knowing the msc.

-mario


>> -mario
>>
>>> Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
>>> ---
>>>  hw/xfree86/dri2/dri2.c |   11 +++++++----
>>>  1 files changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
>>> index 228c0e0..d9b9d57 100644
>>> --- a/hw/xfree86/dri2/dri2.c
>>> +++ b/hw/xfree86/dri2/dri2.c
>>> @@ -856,11 +856,14 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr
>>> pDraw, CARD64 target_msc,
>>>  	 * is moved to a crtc with a lower refresh rate, or a crtc that  
>>> just
>>>  	 * got enabled.
>>>  	 */
>>> -	if (!(*ds->GetMSC)(pDraw, &ust, &current_msc))
>>> -	    pPriv->last_swap_target = 0;
>>> +	if (ds->GetMSC) {
>>> +	    if (!(*ds->GetMSC)(pDraw, &ust, &current_msc))
>>> +		pPriv->last_swap_target = 0;
>>>
>>> -	if (current_msc < pPriv->last_swap_target)
>>> -	    pPriv->last_swap_target = current_msc;
>>> +	    if (current_msc < pPriv->last_swap_target)
>>> +		pPriv->last_swap_target = current_msc;
>>> +
>>> +	}
>>>
>>>  	/*
>>>  	 * Swap target for this swap is last swap target + swap interval
>>> since
>>> -- 
>>> 1.7.0.4
>>>
>>> _______________________________________________
>>> xorg-devel at lists.x.org: X.Org development
>>> Archives: http://lists.x.org/archives/xorg-devel
>>> Info: http://lists.x.org/mailman/listinfo/xorg-devel
>>
>> *********************************************************************
>> Mario Kleiner
>> Max Planck Institute for Biological Cybernetics
>> Spemannstr. 38
>> 72076 Tuebingen
>> Germany
>>
>> e-mail: mario.kleiner at tuebingen.mpg.de
>> office: +49 (0)7071/601-1623
>> fax:    +49 (0)7071/601-616
>> www:    http://www.kyb.tuebingen.mpg.de/~kleinerm
>> *********************************************************************
>> "For a successful technology, reality must take precedence
>> over public relations, for Nature cannot be fooled."
>> (Richard Feynman)

*********************************************************************
Mario Kleiner
Max Planck Institute for Biological Cybernetics
Spemannstr. 38
72076 Tuebingen
Germany

e-mail: mario.kleiner at tuebingen.mpg.de
office: +49 (0)7071/601-1623
fax:    +49 (0)7071/601-616
www:    http://www.kyb.tuebingen.mpg.de/~kleinerm
*********************************************************************
"For a successful technology, reality must take precedence
over public relations, for Nature cannot be fooled."
(Richard Feynman)