[Mesa-dev] [PATCH 2/7] EGL: Implement eglLabelObjectKHR

[Emil Velikov]

On 9 September 2016 at 18:51, Adam Jackson <ajax at redhat.com> wrote:
> On Fri, 2016-09-09 at 11:27 +0100, Emil Velikov wrote:
>> > On 8 September 2016 at 18:46, Adam Jackson <ajax at redhat.com> wrote:
>> > From: Kyle Brenneman <kbrenneman at nvidia.com>
>>
>> Added a label to the _EGLThreadInfo, _EGLDisplay, and EGLResource
>> structs. Implemented the function eglLabelObjectKHR.
>>
>>
>> Coding style of the new hunk follows the GLVND one, which is _not_
>> what we use in mesa/egl. Please don't do that ?
>
> Fixed in next version.
>
>> b) eglTerminate
>> It detaches/unlinks only contexts and surfaces (bug?). Thus even when
>> the display is no longer initialized we will get get to this point and
>> _eglCheckResource() will return true.
>
> Almost certainly a bug, patch in next version of the series. Not sure
> if any tests cover this bug, but I guess I'll find out.
>
That was my inclination as well (existing bug) although I wasn't 100%.
And no we don't have tests which check (attempts to exploit) the
leak(s).

>> > +/**
>> > + * Returns the label set for the current thread.
>> > + */
>> > +EGLLabelKHR _eglGetThreadLabel(void)
>> > +{
>> > +   _EGLThreadInfo *t = _eglGetCurrentThread();
>> > +   return t->Label;
>>
>> Shouldn't the label be cleared in eglReleaseThread ?
>>
>
> It isn't, not explicitly, but eglReleaseThread ->
> _eglDestroyCurrentThread -> _eglDestroyThreadInfo which frees the
> struct containing the label.
>
Right, thanks!
Emil