[vlc-devel] [PATCH 2/2] filters: use filter_Flush() instead of accessing the callback directly

[Steve Lhomme]

On 2020-10-12 18:08, Alexandre Janniaux wrote:
> Hi,
> 
> I agree with Rémi, why trying to put unknown code in the
> equation while you want to call only known code?

There are actually 2 different use cases I didn't realize before. Some 
filters call flush on Close. They should probably not do it by 
themselves. It should be done globally in filter_Close().

Then there's some error handling which *is* a local call and probably 
doesn't the generic call.

> There's a difference between the module public API and its
> internal handling.
> 
> Regards,
> --
> Alexandre Janniaux
> Videolabs
> 
> On Mon, Oct 12, 2020 at 04:45:47PM +0200, Steve Lhomme wrote:
>> On 2020-10-12 16:34, Rémi Denis-Courmont wrote:
>>> Le maanantaina 12. lokakuuta 2020, 17.16.47 EEST Steve Lhomme a écrit :
>>>> It's easier to find out who flushes filters and when.
>>>
>>> This is a very bad practice. Modules should not call into themselves via the
>>> core functions. This is prone to reentrancy bugs if filter_Flush() ever chagnes
>>> to do something else than call the flush callback.
>>
>> If it changes, then the local callback will change accordingly. Either it
>> will need to be update because the signature changed and the compilation
>> will fail, or because this call's signature changed. I prefer compiler
>> errors to internal calls mismatching after an API changed.
>>
>> The reentrancy issue is a real problem.
>>
>>>> It's also easier to reorder the code without having the Flush callback above
>>>> a lot of common code.
>>>
>>> There's no real common code here. Different function calls different function.
>>>
>>> --
>>> レミ・デニ-クールモン
>>> http://www.remlab.net/
>>>
>>>
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>>
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>