[PATCH 2/3] scrobj: Fix some leaks on error paths (Coverity).

[Sven Baars]

On 21-10-19 12:14, Jacek Caban wrote:
> Hi Sven,
> 
> On 19/10/2019 15:20, Sven Baars wrote:
>> Signed-off-by: Sven Baars <sven.wine at gmail.com>
>> ---
>>   dlls/scrobj/scrobj.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/dlls/scrobj/scrobj.c b/dlls/scrobj/scrobj.c
>> index ce5a3dedd1..8d23c65f55 100644
>> --- a/dlls/scrobj/scrobj.c
>> +++ b/dlls/scrobj/scrobj.c
>> @@ -1491,6 +1491,7 @@ static HRESULT parse_scriptlet_public(struct
>> scriptlet_factory *factory)
>>               if (!wcsicmp(member_iter->name, member->name))
>>               {
>>                   FIXME("Duplicated member %s\n",
>> debugstr_w(member->name));
>> +                heap_free(member);
>>                   return E_FAIL;
>>               }
>>           }
>> @@ -1532,7 +1533,11 @@ static HRESULT parse_scriptlet_public(struct
>> scriptlet_factory *factory)
>>                   if (!(parameter = heap_alloc(sizeof(*parameter))))
>> return E_OUTOFMEMORY;
>>                     hres = read_xml_value(factory, &parameter->name);
>> -                if (FAILED(hres)) return hres;
>> +                if (FAILED(hres))
>> +                {
>> +                    heap_free(parameter);
>> +                    return hres;
>> +                }
>>                   list_add_tail(&member->u.parameters,
>> &parameter->entry);
>>                   if (!empty && FAILED(hres =
>> expect_end_element(factory))) return hres;
>>               }
> 
> 
> You could just move list_add_tail() call to be done just after heap_alloc.
> 
> 
> Thanks,
> 
> Jacek
> 

Hi Jacek,

But then you have a "parameter", whose only member is "name", that does
not have a name, since the name is set by read_xml_value, but that
method failed, meaning the name has not been set. Therefore I thought it
was better not to add it to the list.

I actually don't really understand what this code is supposed to do
anyway. The "parameters" do never seem to be used anywhere. The only
"scriptlet_member" members that appear to be used are "type" and "name"
(and maybe "flags" for validation). Is this for a future implementation?

Best,
Sven