[Mesa-dev] [PATCH 1/2] st/va: release held locks in error paths

[Grazvydas Ignotas]

On Tue, Jan 16, 2018 at 10:15 AM, Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
> Reviewed-by: Christian König <christian.koenig at amd.com>
>
> Do you have commit right by now or should Leo or I commit that for you?

Yes I do.

>
> Thanks for the help,
> Christian.
>
>
> Am 15.01.2018 um 23:03 schrieb Grazvydas Ignotas:
>>
>> Found with the help of following Coccinelle semantic patch:
>> // <smpl>
>> @@
>> expression E;
>> @@
>>
>>    \(pthread_mutex_lock\|mtx_lock\|simple_mtx_lock\)(E)
>>    ...
>> (
>>    \(pthread_mutex_unlock\|mtx_unlock\|simple_mtx_unlock\)(E);
>>    ...
>>    return ...;
>> |
>> + maybe need_unlock(E);
>>    return ...;
>> )
>> // </smpl>
>>
>> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>
>> ---
>>   src/gallium/state_trackers/va/config.c  | 4 +++-
>>   src/gallium/state_trackers/va/image.c   | 4 +++-
>>   src/gallium/state_trackers/va/picture.c | 4 +++-
>>   3 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/state_trackers/va/config.c
>> b/src/gallium/state_trackers/va/config.c
>> index 25043d6..7bc031a 100644
>> --- a/src/gallium/state_trackers/va/config.c
>> +++ b/src/gallium/state_trackers/va/config.c
>> @@ -306,12 +306,14 @@ vlVaDestroyConfig(VADriverContextP ctx, VAConfigID
>> config_id)
>>         return VA_STATUS_ERROR_INVALID_CONTEXT;
>>        mtx_lock(&drv->mutex);
>>      config = handle_table_get(drv->htab, config_id);
>>   -   if (!config)
>> +   if (!config) {
>> +      mtx_unlock(&drv->mutex);
>>         return VA_STATUS_ERROR_INVALID_CONFIG;
>> +   }
>>        FREE(config);
>>      handle_table_remove(drv->htab, config_id);
>>      mtx_unlock(&drv->mutex);
>>   diff --git a/src/gallium/state_trackers/va/image.c
>> b/src/gallium/state_trackers/va/image.c
>> index 86ae868..3f892c9 100644
>> --- a/src/gallium/state_trackers/va/image.c
>> +++ b/src/gallium/state_trackers/va/image.c
>> @@ -546,12 +546,14 @@ vlVaPutImage(VADriverContextP ctx, VASurfaceID
>> surface, VAImageID image,
>>                                             tex,
>>                                             0,
>>                                             PIPE_TRANSFER_WRITE |
>>                                             PIPE_TRANSFER_DISCARD_RANGE,
>>                                             &dst_box, &transfer);
>> -            if (map == NULL)
>> +            if (map == NULL) {
>> +               mtx_unlock(&drv->mutex);
>>                  return VA_STATUS_ERROR_OPERATION_FAILED;
>> +            }
>>                 u_copy_nv12_from_yv12((const void * const*) data, pitches,
>> i, j,
>>                                     transfer->stride, tex->array_size,
>>                                     map, dst_box.width, dst_box.height);
>>               pipe_transfer_unmap(drv->pipe, transfer);
>> diff --git a/src/gallium/state_trackers/va/picture.c
>> b/src/gallium/state_trackers/va/picture.c
>> index 8951573..cfcf986 100644
>> --- a/src/gallium/state_trackers/va/picture.c
>> +++ b/src/gallium/state_trackers/va/picture.c
>> @@ -675,13 +675,15 @@ vlVaEndPicture(VADriverContextP ctx, VAContextID
>> context_id)
>>               dst_rect.x1 = src_rect.x1 = surf->templat.width;
>>               dst_rect.y1 = src_rect.y1 = surf->templat.height;
>>               vl_compositor_yuv_deint_full(&drv->cstate, &drv->compositor,
>>                                            old_buf, surf->buffer,
>>                                            &src_rect, &dst_rect,
>> VL_COMPOSITOR_WEAVE);
>> -         } else
>> +         } else {
>>               /* Can't convert from progressive to interlaced yet */
>> +            mtx_unlock(&drv->mutex);
>>               return VA_STATUS_ERROR_INVALID_SURFACE;
>> +         }
>>         }
>>           old_buf->destroy(old_buf);
>>         context->target = surf->buffer;
>>      }
>
>