[PATCH 3/4] winevulkan: Remove unnecessary checks in wine_vkCmdExecuteCommands

[Liam Middlebrook]

Sure, so the implementation can choose to handle this undefined behavior how it 
likes. I usually like to err on the side of failing gracefully rather than 
catastrophically (that being said I don't think the cases I outline below would 
reasonably be classified as catastrophic failure, which I'd normally attribute to 
hangs, or system-level crashes, nothing that winevulkan should hopefully be getting 
itself into).

The decision to be made is whether the assumed boost in performance is worth the 
downsides of calling RtlAllocateHeap with a size of zero. I haven't taken the time 
to inspect RtlAllocateHeap carefully but I would presume there are two cases which 
could happen here:

1. We successfully allocate the memory, leave tmp_buffers as uninitialized, and then 
call to the driver, getting back whatever undefined behavior the driver has chosen 
is appropriate in this case.

2. We fail to allocate the memory and an ERR is logged indicated the failure to 
allocate memory.

While I think it's less-likely I'm more concerned about case 2, because it could 
confuse an end-user or developer into thinking that a legitimate out-of-memory case, 
or some WINE bug is happening, rather than invalid usage of the Vulkan API on the 
application's part.


Thanks,

Liam Middlebrook

On 3/5/20 4:19 PM, Joshua Ashton wrote:
> A size of zero violates the Vulkan spec so it should be a non-issue.
> 
> - Josh 🐸
> 
> ---- On Fri, 06 Mar 2020 00:10:40 +0000 *Liam Middlebrook <lmiddlebrook at nvidia.com 
> <mailto:lmiddlebrook at nvidia.com>>* wrote ----
> 
>     I'm not sure that we can remove this if we keep both the alloca and heap_alloc
>     implementations in-place, to my understanding RtlAllocateHeap would run into issues
>     when given an input size of zero.
> 
> 
>     Thanks,
> 
>     Liam Middlebrook
> 
>     On 3/5/20 9:03 AM, Joshua Ashton wrote:
>      > The Vulkan spec states commandBufferCount must be greater than zero and
>     pCommandBuffers must be a valid pointer.
>      >
>      > Signed-off-by: Joshua Ashton <joshua at froggi.es <mailto:joshua at froggi.es>>
>      > ---
>      > dlls/winevulkan/vulkan.c | 3 ---
>      > 1 file changed, 3 deletions(-)
>      >
>      > diff --git a/dlls/winevulkan/vulkan.c b/dlls/winevulkan/vulkan.c
>      > index 1cc1b4f61f..f767b86dc4 100644
>      > --- a/dlls/winevulkan/vulkan.c
>      > +++ b/dlls/winevulkan/vulkan.c
>      > @@ -517,9 +517,6 @@ void WINAPI wine_vkCmdExecuteCommands(VkCommandBuffer
>     buffer, uint32_t count,
>      >
>      > TRACE("%p %u %p\n", buffer, count, buffers);
>      >
>      > - if (!buffers || !count)
>      > - return;
>      > -
>      > tmp_buffers = WINEVULKAN_ALLOCA(count * sizeof(*tmp_buffers));
>      >
>      > for (i = 0; i < count; i++)
>      >
> 
>     -----------------------------------------------------------------------------------
>     This email message is for the sole use of the intended recipient(s) and may contain
>     confidential information. Any unauthorized review, use, disclosure or distribution
>     is prohibited. If you are not the intended recipient, please contact the sender by
>     reply email and destroy all copies of the original message.
>     -----------------------------------------------------------------------------------
> 
> 
>