[Mesa-dev] [PATCH 05/10] gbm: handle queryImage() failure through rest of gbm_dri_bo_import()

[Emil Velikov]

On 17 October 2017 at 11:12, Eric Engestrom <eric.engestrom at imgtec.com> wrote:
> On Monday, 2017-10-16 16:04:07 +0000, Emil Velikov wrote:
>> From: Emil Velikov <emil.velikov at collabora.com>
>>
>> Fixes: 6a7dea93fa7 ("dri: Rework planar image interface")
>> Cc: Jakob Bornecrantz <jakob.bornecrantz at collabora.co.uk>
>> Signed-off-by: Emil Velikov <emil.velikov at collabora.com>
>> ---
>>  src/gbm/backends/dri/gbm_dri.c | 23 +++++++++++++++--------
>>  1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
>> index 9c9066e6661..a6c80cf1ec7 100644
>> --- a/src/gbm/backends/dri/gbm_dri.c
>> +++ b/src/gbm/backends/dri/gbm_dri.c
>> @@ -1048,14 +1048,21 @@ gbm_dri_bo_import(struct gbm_device *gbm,
>>     bo->base.gbm = gbm;
>>     bo->base.format = gbm_format;
>>
>> -   dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_WIDTH,
>> -                          (int*)&bo->base.width);
>> -   dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_HEIGHT,
>> -                          (int*)&bo->base.height);
>> -   dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_STRIDE,
>> -                          (int*)&bo->base.stride);
>> -   dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_HANDLE,
>> -                          &bo->base.handle.s32);
>> +   query = dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_WIDTH,
>> +                                  (int*)&bo->base.width);
>> +   query &= dri->image->queryImage(bo->image, __DRI_IMAGE_ATTRIB_HEIGHT,
>
> With bitmasks, you really need to have 0/1 here. Someone could return `2`,
> which would count as `true` as bool, but `1 & 2 == 0`, which is not what
> we want here.
> Please add `!!` in front of the queryImage() calls.
>
I've assumed that won't be the case since we already have a few &= instances.
Regardless, suggestion makes sense.

My earlier comment (use standard types) won't fly for
s/GLboolean/bool/ since it's an ABI break.

-Emil