[Mesa-dev] [PATCH] vulkan/wsi/x11: Fix behavior of vkGetPhysicalDeviceSurfaceFormatsKHR

[Eduardo Lima Mitev]

On 10/25/2016 11:25 AM, Dave Airlie wrote:
> Reviewed-by : Dave Airlie <airlied at redhat.com <mailto:airlied at redhat.com>>
> 

Thanks Dave. I will merge the patch later in the afternoon.

Eduardo

> On 25 Oct. 2016 18:39, "Eduardo Lima Mitev" <elima at igalia.com
> <mailto:elima at igalia.com>> wrote:
>>
>> x11_surface_get_formats() is currently assering that the number of
>> elements in pSurfaceFormats must be greater or equal than the number
>> of formats available. This is buggy because later that number of
>> elements is copied from the internal formats' array, so if
>> pSurfaceFormatCount is greater, it will overflow it.
>>
>> On top of that, this assertion violates the spec. From the Vulkan 1.0
>> (revision 32, with KHR extensions), page 579 of the PDF:
>>
>>     "If pSurfaceFormats is NULL, then the number of format pairs supported
>>      for the given surface is returned in pSurfaceFormatCount. Otherwise,
>>      pSurfaceFormatCount must point to a variable set by the user to the
>>      number of elements in the pSurfaceFormats array, and on return the
>>      variable is overwritten with the number of structures actually
> written
>>      to pSurfaceFormats. If the value of pSurfaceFormatCount is less than
>>      the number of format pairs supported, at most pSurfaceFormatCount
>>      structures will be written. If pSurfaceFormatCount is smaller than
>>      the number of format pairs supported for the given surface,
>>      VK_INCOMPLETE will be returned instead of VK_SUCCESS to indicate that
>>      not all the available values were returned."
>>
>> So, the correct behavior is: If pSurfaceFormatCount is greater than the
>> internal number of formats, then it is clamped to that many formats. But
>> if it is lesser than it, then pSurfaceFormatCount formats are returned,
>> and the call returns VK_INCOMPLETE.
>> ---
>>  src/vulkan/wsi/wsi_common_x11.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/vulkan/wsi/wsi_common_x11.c
> b/src/vulkan/wsi/wsi_common_x11.c
>> index b5832c6..548352e 100644
>> --- a/src/vulkan/wsi/wsi_common_x11.c
>> +++ b/src/vulkan/wsi/wsi_common_x11.c
>> @@ -394,11 +394,16 @@ x11_surface_get_formats(VkIcdSurfaceBase *surface,
>>        return VK_SUCCESS;
>>     }
>>
>> -   assert(*pSurfaceFormatCount >= ARRAY_SIZE(formats));
>> +   VkResult result = VK_SUCCESS;
>> +
>> +   if (*pSurfaceFormatCount > ARRAY_SIZE(formats))
>> +      *pSurfaceFormatCount = ARRAY_SIZE(formats);
>> +   else if (*pSurfaceFormatCount < ARRAY_SIZE(formats))
>> +      result = VK_INCOMPLETE;
>> +
>>     typed_memcpy(pSurfaceFormats, formats, *pSurfaceFormatCount);
>> -   *pSurfaceFormatCount = ARRAY_SIZE(formats);
>>
>> -   return VK_SUCCESS;
>> +   return result;
>>  }
>>
>>  static VkResult
>> --
>> 2.8.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>