[Mesa-dev] [Nouveau] [PATCH mesa] clover: Fix pipe_grid_info.indirect not being initialized

[Francisco Jerez]

Samuel Pitoiset <samuel.pitoiset at gmail.com> writes:

> On 03/14/2016 02:29 PM, Samuel Pitoiset wrote:
>>
>>
>> On 03/14/2016 02:26 PM, Hans de Goede wrote:
>>> Hi,
>>>
>>> On 14-03-16 14:01, Samuel Pitoiset wrote:
>>>>
>>>>
>>>> On 03/14/2016 01:50 PM, Hans de Goede wrote:
>>>>> After pipe_grid_info.indirect was introduced, clover was not modified
>>>>> to set it causing it to pass uninitialized memory for it to
>>>>> launch_grid.
>>>>>
>>>>> This commit fixes this by zero-ing the entire pipe_grid_info struct
>>>>> when
>>>>> declaring it, to avoid similar problems popping-up in the future.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
>>>>> ---
>>>>>   src/gallium/state_trackers/clover/core/kernel.cpp | 2 +-
>>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp
>>>>> b/src/gallium/state_trackers/clover/core/kernel.cpp
>>>>> index 8396be9..dad66aa 100644
>>>>> --- a/src/gallium/state_trackers/clover/core/kernel.cpp
>>>>> +++ b/src/gallium/state_trackers/clover/core/kernel.cpp
>>>>> @@ -55,7 +55,7 @@ kernel::launch(command_queue &q,
>>>>>      const auto reduced_grid_size =
>>>>>         map(divides(), grid_size, block_size);
>>>>>      void *st = exec.bind(&q, grid_offset);
>>>>> -   struct pipe_grid_info info;
>>>>> +   struct pipe_grid_info info = { 0, };
>>>>
>>>> Right, good catch, it's my fault.
>>>>
>>>> = { 0 }; is enough btw.
>>>
>>> I prefer to add the "," to make clear that we are initializing the
>>> entire struct,
>>> I read it as  ", ...".
>>
>> Well, usually we use { 0 } in mesa, try to grep and you will see. :-)
>> There is only 3 occurrences of { 0, }, but I think they are quite old.
>
> Of course, I'm not really against this ",", but I just want consistency 
> with the other parts.
>

In C++ '{}' is standard, more concise, and works for a wider range of
types regardless of their layout ('{ 0 }' is valid or not depending on
what the first member of the struct is, while '{}' works regardless, in
C++11 it can even be used to initialize non-POD types with custom
constructors), so it should be generally preferred instead.

Don't bother to resend just because of my nitpicking, I'll fix it up
before I push the last revision of your change, which is:

Reviewed-by: Francisco Jerez <currojerez at riseup.net>

>>
>>>
>>>> This should be backported to mesa 11.2 I guess, could you please send
>>>> a v2 with this minor fix and add the cc thing?
>>>
>>> Sure, as soon as we're done bikeshedding on the "," :)
>>>
>>> Regards,
>>>
>>> Hans
>>
>
> -- 
> -Samuel
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160314/cf015f49/attachment.sig>