[Mesa-dev] [PATCH v2] nv50: avoid freeing the symbols if they're about to be stored

[Samuel Pitoiset]

On 02/03/2016 09:55 PM, Ilia Mirkin wrote:
> On Wed, Feb 3, 2016 at 3:47 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>>
>>
>> On 02/03/2016 08:00 PM, Ilia Mirkin wrote:
>>>
>>> Spotted by Coverity
>>>
>>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>>> ---
>>>
>>> v1 -> v2: forgot to remove the original free...
>>>
>>>    src/gallium/drivers/nouveau/nv50/nv50_program.c | 9 +++++++--
>>>    1 file changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/nv50/nv50_program.c
>>> b/src/gallium/drivers/nouveau/nv50/nv50_program.c
>>> index 888d62e..a67ef28 100644
>>> --- a/src/gallium/drivers/nouveau/nv50/nv50_program.c
>>> +++ b/src/gallium/drivers/nouveau/nv50/nv50_program.c
>>> @@ -369,7 +369,6 @@ nv50_program_translate(struct nv50_program *prog,
>>> uint16_t chipset,
>>>          NOUVEAU_ERR("shader translation failed: %i\n", ret);
>>>          goto out;
>>>       }
>>> -   FREE(info->bin.syms);
>>
>>
>> I would prefer to be consistent with what nvc0 currently does, so:
>>
>> if (prog->type != PIPE_SHADER_COMPUTE)
>>    FREE(info->bin.syms);
>>
>> And you could remove the below hunk.
>
> You don't think keeping the use (or freeing) of the syms together is a
> good idea? Having them be disconnected just makes it harder to track
> expectations...

Well, feel free to keep your patch as is, it's indeed a bit more 
readable... but the other way looked fine to me too.

By the way, Coverity is not always wrong. :-)

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

>
>    -ilia
>