[PATCH v4 2/5] winegstreamer: Move the conditional cleanup to the create error path.

[Alexandre Julliard]

Rémi Bernon <rbernon at codeweavers.com> writes:

> On 2/22/22 19:35, Rémi Bernon wrote:
>> On 2/22/22 19:05, Zebediah Figura (she/her) wrote:
>>> On 2/22/22 12:03, Rémi Bernon wrote:
>>>> On 2/22/22 18:43, Zebediah Figura wrote:
>>>>>    +out_free_sink_caps:
>>>>> +    gst_caps_unref(sink_caps);
>>>>> +out_free_src_pad:
>>>>> +    gst_object_unref(transform->my_src);
>>>>> +out_free_src_caps:
>>>>> +    gst_caps_unref(src_caps);
>>>>> +out_free_transform:
>>>>> +    free(transform);
>>>>> +    GST_ERROR("Failed to create winegstreamer transform.");
>>>>>        return status;
>>>>>    }
>>>>
>>>> I hesitated with an approach like this. I haven't seen goto dipatch like
>>>> this elsewhere so I thought it wasn't the preferred style.
>>>
>>> I haven't seen it used elsewhere in Wine, but it was adopted in a couple
>>> of places in vkd3d, and I personally like it.
>> 
>> Tbh I'm not sure I do, I find it much harder to reason with,
>> especially when there's many different error paths or stage like
>> here, though I'm not the maintainer here.
>> 
>
> Experiencing it a bit more after rebasing my local patches I think it
> also makes re-odering of code much more difficult as you need to be
> very careful to update the gotos correctly as well as with potential 
> reordering of the cleanup labels and steps.

I have to agree, with more than a couple of goto targets it becomes
painful to check for correctness, and looks awful to maintain. IMHO even
nested if() blocks would be more readable. I'd recommend finding some
other way.

-- 
Alexandre Julliard
julliard at winehq.org