<html><head></head><body>Hi,<br><br>I don't see how wrong pointer cast is a security hazard. If the target fails to validate function prototypes at runtime, having proper function prototypes at compile time is not going to help. Conversely, if the target enforces type matching, then it will crash without exploit routes.<br><br>But the main point is that this is not specific to vout display open callback. If you think that function type mismatch should be an error, then use the corresponding -Werror-whatever option accordingly. We already have one such case in the build system for missing/implicit declarations.<br><br><div class="gmail_quote">Le 10 juillet 2019 15:07:59 GMT+03: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 2019-07-10 13:51, 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>A static_assert failure *is* an error. <br></blockquote><br>"more than an error" is an error.<br><br>Mismatched function pointer types should be a warning, not an error.<br>> <br><br>I strongly disagree. This is a security hazard left unattended.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #729fcf; padding-left: 1ex;">You're free to use Werror on your builds if you want.<br><br>Le 10 juillet 2019 14:35:17 GMT+03:00, Steve Lhomme <robux4@ycbcr.xyz> a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #ad7fa8; padding-left: 1ex;">On 2019-07-10 12:42, Rémi Denis-Courmont wrote:<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"> Hi,<br><br> As already stated yesterday, I don't see why this specific case<br></blockquote> grants an error any more than any other function pointer passing.<br><br> It's more than an error, it's a static assert. It won't compile if you<br> use the wrong signature. IMO it's a good step in better code safety.<br><br> One drawback of this approach is when you change a signature you have<br> to<br> change it in all modules, whereas before you could change the signature<br><br> and then each module individually. Not breaking compilation, just the<br> code running temporarily.<br><br> Warnings are usually ignored (see my other patches fixing long time<br> warnings).<br><br> We could have a static_assert in debug build and a warning in release<br> builds.<br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">In any case, this wrongly causes an error if zero is passed as<br></blockquote> deactivate callback or if NULL is an integer.<br><br> And if NULL is passed instead of nullptr in C++. It's easy enough to<br> use<br> the proper value in that case.<br><br> For NULL I wasn't sure if it's always a void* in C11. It is in Clang<br> (linux, win32), gcc (win32) and MSVC.<br><br> There's typeof(NULL) but it seems to be a GNU extension :<br> <a href="https://stackoverflow.com/questions/40143815/is-it-possible-to-implement-gnu-cs-typeofx-with-c11s-generic">https://stackoverflow.com/questions/40143815/is-it-possible-to-implement-gnu-cs-typeofx-with-c11s-generic</a><br><br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;">Le 10 juillet 2019 11:39:06 GMT+03:00, Steve Lhomme<br></blockquote><robux4@ycbcr.xyz> a écrit :<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;"><hr> include/vlc_vout_display.h | 17 +++++++++++++++++<br> 1 file changed, 17 insertions(+)<br><br> diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h<br> index fd4940cf6d..1cbd6b1891 100644<br> --- a/include/vlc_vout_display.h<br> +++ b/include/vlc_vout_display.h<br> @@ -215,6 +215,23 @@ typedef int<br></blockquote></blockquote>(*vout_display_open_cb)(vout_display_t<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #fcaf3e; padding-left: 1ex;"> *vd,<br> */<br> typedef void (*vout_display_close_cb)(vout_display_t *vd);<br><br> +#ifndef __cplusplus<br> +#define set_callbacks_display(Activate, Deactivate) \<br> + static_assert( _Generic( Activate, vout_display_open_cb: 1,<br> default: 0 ) == 1, \<br> + "Not a display activate callback"); \<br> + static_assert( _Generic( Deactivate, vout_display_close_cb: 1,<br> void*: 1, default: 0 ) == 1, \<br> + "Not a display deactivate callback"); \<br> + set_callbacks( Activate, Deactivate )<br> +#else /* __cplusplus */<br> +#define set_callbacks_display(Activate, Deactivate) \<br> + static_assert(<br> std::is_same<std::add_pointer<decltype(Activate)>::type,<br> vout_display_open_cb>::value, \<br> + "Not a display activate callback"); \<br> + static_assert(<br> std::is_same<std::add_pointer<decltype(Deactivate)>::type,<br> vout_display_close_cb>::value || \<br> +<br> std::is_same<decltype(Deactivate),std::nullptr_t>::value, \<br> + "Not a display deactivate callback"); \<br> + set_callbacks( Activate, Deactivate )<br> +#endif /* __cplusplus */<br> +<br> struct vout_display_t {<br> struct vlc_object_t obj;<br><br> -- <br> 2.17.1<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></blockquote>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez<br></blockquote>excuser ma brièveté.<br><blockquote class="gmail_quote" style="margin: 0pt 0pt 1ex 0.8ex; border-left: 1px solid #8ae234; padding-left: 1ex;"><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><br></blockquote>-- <br>Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser 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>