[PATCH 1/5] winegstreamer: Fix multiple leaks in failure pathes.

[Derek Lesho]

On 9/6/21 1:37 PM, Nikolay Sivov wrote:

>
> On 9/6/21 8:23 PM, Zebediah Figura wrote:
>
> In addition to points Zebediah made.
>
>> @@ -1211,7 +1211,8 @@ static HRESULT WINAPI
>> media_source_Shutdown(IMFMediaSource *iface)
>>          source->state = SOURCE_SHUTDOWN;
>>    -    unix_funcs->wg_parser_disconnect(source->wg_parser);
>> +    if (source->stream_count)
>> +        unix_funcs->wg_parser_disconnect(source->wg_parser);
>>          if (source->read_thread)
> This is awkward because it implies some relationship between wg_parser
> being initialized and able to disconnect when stream_count is set. I
> think it's cleaner to let wg_parser_disconnect() handle null, or change
> condition to if (wg_parser).
The point here is that it's possible connection fails, in which case 
wg_parser is initialized but is not connected.  There is no case right 
now where stream_count is unset but the source is connected, so I opted 
to use it.  In the end, I will move this check into the constructor, as 
Zebediah suggested.
>
>>          unix_funcs->wg_parser_destroy(source->wg_parser);
>>    -    if (source->stream_count)
>> +    if (source->streams)
>>            free(source->streams);
> This should be redundant, no?
No, it is possible for calloc to fail, in which case stream_count is set 
but not the array.
>
>> @@ -1231,6 +1232,9 @@ static HRESULT WINAPI
>> media_source_Shutdown(IMFMediaSource *iface)
>>        {
>>            struct media_stream *stream = source->streams[i];
>>    +        if (!stream)
>> +            continue;
>> +
>>            stream->state = STREAM_SHUTDOWN;
> I think expected way would be to have steam_count consistent with a
> number of non-null streams, so you don't have to check for this.
So we should be modifying stream_count again in the failure path?  I 
don't agree that this is more clear but I will do so.
>