[Mesa-dev] [PATCH 3/3] [Bug 38970] [bisected]piglit glx/glx-pixmap-multi failed

[Rahul Jain]

On Wed, Oct 19, 2016 at 1:24 AM, Nicolai Hähnle <nhaehnle at gmail.com> wrote:

> On 18.10.2016 19:23, Ian Romanick wrote:
>
>> On 09/29/2016 01:55 PM, Anutex wrote:
>>
>>> I tried to debug this issue with changing the condition to check only
>>> bad magic and Error.
>>> And the test passed.
>>>
>>> Though i am not sure what is the correct behaviour if we are in this
>>> condition.
>>> May be we should make some  other condition if the Hash Table have the
>>> bucket data.
>>> ---
>>>  src/glx/dri2_glx.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
>>> index af388d9..a1fd9ff 100644
>>> --- a/src/glx/dri2_glx.c
>>> +++ b/src/glx/dri2_glx.c
>>> @@ -411,12 +411,13 @@ dri2CreateDrawable(struct glx_screen *base, XID
>>> xDrawable,
>>>        return NULL;
>>>     }
>>>
>>> -   if (__glxHashInsert(pdp->dri2Hash, xDrawable, pdraw)) {
>>> +   if (__glxHashInsert(pdp->dri2Hash, xDrawable, pdraw) == -1) {
>>>
>>
Instead here can we implement some thing like
if already in table then get drawable and return it (As Ian told)
else  destroy Drawables. (same as default)


>> I'm not 100% sure the existing code is wrong.  __glxHashInsert returns
>> -1 for an error, and it returns 1 if the key is already in the hash
>> table.  In that case we'll leak the memory for the new pdraw, right?
>> That also seems bad.
>>
>> It seems like instead the code should look up xDrawable in the hash
>> table and return the value that's already there.  Maybe.  I haven't
>> looked at this code in years, so I may be forgetting some subtlety.
>>
>
> dri2DestroyDrawable destroys the pdraw though. It also removes the
> xDrawable entry in the hash table without checking whether it points at
> pdraw or not, so on the surface that looks pretty bogus if we create a
> GLXDrawable twice.
>
> _However_, the real question is what the hash is used for in the first
> place. It looks to me like the hash is actually pretty pointless in the
> pixmap case. And it just so happens that the GLX spec forbids creating a
> GLXDrawable from a Window twice, but it doesn't forbid creating a
> GLXDrawable from a Pixmap twice.
>
> Then again, my GLX knowledge is basically zero, so what do I know :)
>
> Nicolai
>
>
>>        (*psc->core->destroyDrawable) (pdraw->driDrawable);
>>>        DRI2DestroyDrawable(psc->base.dpy, xDrawable);
>>>        free(pdraw);
>>>        return None;
>>>     }
>>> +
>>>
>>>
>> Spurious whitespace change.
>>
>>     /*
>>>      * Make sure server has the same swap interval we do for the new
>>>
>>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>


-- 
Thanks,
Rahul jain
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161020/5111466e/attachment.html>