<html><head></head><body>Hi,<br><br>Strictly speaking, there is a problem if one or more frames are not C. It's still possible that:<br>- cleanup handlers are pushed from C code called from C++ code -> we cannot prevent that,<br>- cleanup handlers are pushed by inline header code that is cancelable in C, but not in C++. That's why cleanup handlers are defined to no-ops in C++ atm and this patch breaks that (theoretical) scenario.<br><br>I'm not against the patch because it will catch the most obvious mistakes, at little cosy, but in all honesty, it's not entirely correct.<br><br><div class="gmail_quote">Le 11 février 2020 10:51:31 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">
<pre class="k9mail">On 2020-02-11 9:45, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">Hi,<br><br>I considered doing that, but it potentially wrecks common C/C++ code (in <br>headers), if the code is cancellable in C. Same problem with <br>vlc_cancel(), which is not strictly wrong in C++.<br></blockquote><br>If the code is cancelable in C, vlc_cancel() can still be called from C <br>(and in fact even from C++).<br><br>If the thread is created from C++ and then calls C code which push/pops <br>a cleanup routine, it should not impact the C++ part of the code. After <br>the vlc_testcancel(), the C code will normally go up the stack and end <br>up in the C++ part.<br><br>If the thread is created in C++, the C++ part cannot push/pop a cleanup <br>routine as we can't guarantee it will work (in portable code). So IMO it <br>makes sense to forbid that part only.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">But if you prefer that way, I can live with it.<br><br>Le 11 février 2020 09:45:01 GMT+02:00, Steve Lhomme <robux4@ycbcr.xyz> a <br>écrit :<br><br> It may work for some C++ code only used on Linux. But if the module is compiled<br> on other platforms it will assert.<hr> include/vlc_threads.h | 10 ++++------<br> 1 file changed, 4 insertions(+), 6 deletions(-)<br><br> diff --git a/include/vlc_threads.h b/include/vlc_threads.h<br> index 0e27ea803fa..5a75f4ad1f8 100644<br> --- a/include/vlc_threads.h<br> +++ b/include/vlc_threads.h<br> @@ -1084,12 +1084,10 @@ struct vlc_cleanup_t<br> vlc_control_cancel (NULL); \<br> } while (0)<br> # else<br> -/* Those macros do not work in C++. However common C/C++ helpers may call them<br> - * anyway - this is fine if the code is never cancelled in C++ case.<br> - * So define the macros to do nothing.<br> - */<br> -# define vlc_cleanup_push(routine, arg) do { (routine, arg)<br> -# define vlc_cleanup_pop() } while (0)<br> +# define vlc_cleanup_push(routine, arg) \<br> + static_assert(false, "don't use vlc_cleanup_push in portable C++ code")<br> +# define vlc_cleanup_pop() \<br> + static_assert(false, "don'o't use vlc_cleanup_pop in portable C++ code")<br> # endif<br> <br> #endif /* !LIBVLC_USE_PTHREAD_CLEANUP */<br><br><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser <br>ma brièveté.<hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a><br><br></blockquote><hr>vlc-devel mailing list<br>To unsubscribe or modify your subscription options:<br><a href="https://mailman.videolan.org/listinfo/vlc-devel">https://mailman.videolan.org/listinfo/vlc-devel</a></pre></blockquote></div><br>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.</body></html>