From: "Sergio Gómez Del Real" <sdelreal@codeweavers.com>
Subject: Re: [PATCH 02/10] mf: Do some error checkings in _Load().
Message-Id: <a3048625-1cc6-a770-19ff-47cc4493a560@codeweavers.com>
Date: Mon, 22 Jun 2020 19:24:34 -0500
In-Reply-To: <810c0a0d-5f81-11fa-11ef-a2a905cf2678@codeweavers.com>
References: <20200615014158.6836-1-sdelreal@codeweavers.com> <20200615014158.6836-2-sdelreal@codeweavers.com> <810c0a0d-5f81-11fa-11ef-a2a905cf2678@codeweavers.com>

On 22/06/20 6:37 a. m., Nikolay Sivov wrote:

>
> On 6/15/20 4:41 AM, Sergio Gómez Del Real wrote:
>> +    if (FAILED(IMFTopology_GetNodeCount(input_topology, &count))
>> +        || count < 2)
>> +    {
>> +        hr = MF_E_TOPO_UNSUPPORTED;
>> +        return hr;
>> +    }
> This should come up later as more generic case of zero branches, or
> badly incomplete output topology.
The reason I insist with this is that this case specifically returns 
MF_E_TOPO_UNSUPPORTED. Continuing with processing could return a 
different, mistaken, error.
>> +
>> +    if (FAILED(hr = MFCreateTopology(output_topology)))
>> +        return hr;
>> +
>> +    i = 0;
>> +    while (SUCCEEDED(IMFTopology_GetNode(input_topology, i++, &node)))
>>       {
> Won't moving creation call before the loop leak output topology on error
> case?
> I seems unlikely that on error condition it still returns empty output
> topology.
Right. There could be an error later on, if output node doesn't have 
IMFStreamSink interface, which would leak output_topology. I'll also add 
error checking for _CloneFrom().