On Wed, Oct 19, 2016 at 1:24 AM, Nicolai Hähnle <nhaeh...@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>>
--
Thanks,
Rahul jain
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
