[vlc-devel] [PATCH] threads: forbid the use of vlc_cleanup_push/pop in portable C++ code

[Steve Lhomme]

On 2020-02-11 9:45, Rémi Denis-Courmont wrote:
> Hi,
> 
> I considered doing that, but it potentially wrecks common C/C++ code (in 
> headers), if the code is cancellable in C. Same problem with 
> vlc_cancel(), which is not strictly wrong in C++.

If the code is cancelable in C, vlc_cancel() can still be called from C 
(and in fact even from C++).

If the thread is created from C++ and then calls C code which push/pops 
a cleanup routine, it should not impact the C++ part of the code. After 
the vlc_testcancel(), the C code will normally go up the stack and end 
up in the C++ part.

If the thread is created in C++, the C++ part cannot push/pop a cleanup 
routine as we can't guarantee it will work (in portable code). So IMO it 
makes sense to forbid that part only.

> But if you prefer that way, I can live with it.
> 
> Le 11 février 2020 09:45:01 GMT+02:00, Steve Lhomme <robux4 at ycbcr.xyz> a 
> écrit :
> 
>     It may work for some C++ code only used on Linux. But if the module is compiled
>     on other platforms it will assert.
>     ------------------------------------------------------------------------
>       include/vlc_threads.h | 10 ++++------
>       1 file changed, 4 insertions(+), 6 deletions(-)
> 
>     diff --git a/include/vlc_threads.h b/include/vlc_threads.h
>     index 0e27ea803fa..5a75f4ad1f8 100644
>     --- a/include/vlc_threads.h
>     +++ b/include/vlc_threads.h
>     @@ -1084,12 +1084,10 @@ struct vlc_cleanup_t
>               vlc_control_cancel (NULL); \
>           } while (0)
>       # else
>     -/* Those macros do not work in C++. However common C/C++ helpers may call them
>     - * anyway - this is fine if the code is never cancelled in C++ case.
>     - * So define the macros to do nothing.
>     - */
>     -#  define vlc_cleanup_push(routine, arg) do { (routine, arg)
>     -#  define vlc_cleanup_pop() } while (0)
>     +#  define vlc_cleanup_push(routine, arg) \
>     +    static_assert(false, "don't use vlc_cleanup_push in portable C++ code")
>     +#  define vlc_cleanup_pop() \
>     +    static_assert(false, "don'o't use vlc_cleanup_pop in portable C++ code")
>       # endif
>       
>       #endif /* !LIBVLC_USE_PTHREAD_CLEANUP */
> 
> 
> -- 
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser 
> ma brièveté.
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>