[Mesa-dev] [PATCH 7/7] winsys/radeon: fix a race condition in initialization of radeon_winsys::screen

[Christian König]

Am 09.04.2014 05:44, schrieb Michel Dänzer:
> On Mit, 2014-04-09 at 02:15 +0200, Marek Olšák wrote:
>> From: Marek Olšák <marek.olsak at amd.com>
>>
>> Create the screen in the winsys while the mutex is locked.
>> This also results in a nice code cleanup!
> [...]
>> diff --git a/src/gallium/targets/egl-static/egl_pipe.c b/src/gallium/targets/egl-static/egl_pipe.c
>> index eb1cff9..ce734fb 100644
>> --- a/src/gallium/targets/egl-static/egl_pipe.c
>> +++ b/src/gallium/targets/egl-static/egl_pipe.c
>> @@ -119,19 +119,9 @@ pipe_r300_create_screen(int fd)
>>   {
>>   #if _EGL_PIPE_R300
>>      struct radeon_winsys *sws;
>> -   struct pipe_screen *screen;
>> -
>> -   sws = radeon_drm_winsys_create(fd);
>> -   if (!sws)
>> -      return NULL;
>> -
>> -   screen = r300_screen_create(sws);
>> -   if (!screen)
>> -      return NULL;
>>   
>> -   screen = debug_screen_wrap(screen);
>> -
>> -   return screen;
>> +   sws = radeon_drm_winsys_create(fd, r300_screen_create);
>> +   return sws ? debug_screen_wrap(sws->screen) : NULL;
> I think it would be clearer to keep this as:
>
>     sws = radeon_drm_winsys_create(fd, r300_screen_create);
>     if (!sws)
>        return NULL;
>
>     return debug_screen_wrap(sws->screen);
>
>
> Either way though, the series is
>
> Reviewed-by: Michel Dänzer <michel.daenzer at amd.com>

I actually like the shorter form, but anyway thanks allot for this it 
removes quite a bunch of todos from my list. Didn't though that creating 
the screen from the winsys was ok with the design, but when you are fine 
with it I'm not about to complain.

For the series:

Reviewed-by: Christian König <christian.koenig at amd.com>