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

[Zebediah Figura (she/her)]

On 2/22/22 13:37, Rémi Bernon wrote:
> 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 guess the advantage is that it makes it easier to conditionally clean
up in cases that can't be simply expressed as "if (x) free(x)". That
doesn't apply here (yet), but there is something to be said for consistency.

Still, if it becomes too much of a hassle, I'm open to reverting this
commit, or reverting to your v3; I guess I don't care quite that much.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220222/e1da8d2a/attachment.sig>