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) {

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

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to