[vlc-devel] [PATCH v4 3/5] display: add macro to set the callbacks and check type of the Open/Close

[Rémi Denis-Courmont]

Hi,

A static_assert failure *is* an error. Mismatched function pointer types should be a warning, not an error.

You're free to use Werror on your builds if you want.

Le 10 juillet 2019 14:35:17 GMT+03:00, Steve Lhomme <robux4 at ycbcr.xyz> a écrit :
>On 2019-07-10 12:42, Rémi Denis-Courmont wrote:
>> Hi,
>> 
>> As already stated yesterday, I don't see why this specific case
>grants an error any more than any other function pointer passing.
>
>It's more than an error, it's a static assert. It won't compile if you 
>use the wrong signature. IMO it's a good step in better code safety.
>
>One drawback of this approach is when you change a signature you have
>to 
>change it in all modules, whereas before you could change the signature
>
>and then each module individually. Not breaking compilation, just the 
>code running temporarily.
>
>Warnings are usually ignored (see my other patches fixing long time 
>warnings).
>
>We could have a static_assert in debug build and a warning in release 
>builds.
>
>> In any case, this wrongly causes an error if zero is passed as
>deactivate callback or if NULL is an integer.
>
>And if NULL is passed instead of nullptr in C++. It's easy enough to
>use 
>the proper value in that case.
>
>For NULL I wasn't sure if it's always a void* in C11. It is in Clang 
>(linux, win32), gcc (win32) and MSVC.
>
>There's typeof(NULL) but it seems to be a GNU extension :
>https://stackoverflow.com/questions/40143815/is-it-possible-to-implement-gnu-cs-typeofx-with-c11s-generic
>
>> Le 10 juillet 2019 11:39:06 GMT+03:00, Steve Lhomme
><robux4 at ycbcr.xyz> a écrit :
>>> ---
>>> include/vlc_vout_display.h | 17 +++++++++++++++++
>>> 1 file changed, 17 insertions(+)
>>>
>>> diff --git a/include/vlc_vout_display.h b/include/vlc_vout_display.h
>>> index fd4940cf6d..1cbd6b1891 100644
>>> --- a/include/vlc_vout_display.h
>>> +++ b/include/vlc_vout_display.h
>>> @@ -215,6 +215,23 @@ typedef int
>(*vout_display_open_cb)(vout_display_t
>>> *vd,
>>>   */
>>> typedef void (*vout_display_close_cb)(vout_display_t *vd);
>>>
>>> +#ifndef __cplusplus
>>> +#define set_callbacks_display(Activate, Deactivate) \
>>> +    static_assert( _Generic( Activate, vout_display_open_cb: 1,
>>> default: 0 ) == 1, \
>>> +                  "Not a display activate callback"); \
>>> +    static_assert( _Generic( Deactivate, vout_display_close_cb: 1,
>>> void*: 1, default: 0 ) == 1, \
>>> +                  "Not a display deactivate callback"); \
>>> +    set_callbacks( Activate, Deactivate )
>>> +#else /* __cplusplus */
>>> +#define set_callbacks_display(Activate, Deactivate) \
>>> +    static_assert(
>>> std::is_same<std::add_pointer<decltype(Activate)>::type,
>>> vout_display_open_cb>::value, \
>>> +                  "Not a display activate callback"); \
>>> +    static_assert(
>>> std::is_same<std::add_pointer<decltype(Deactivate)>::type,
>>> vout_display_close_cb>::value || \
>>> +
>>> std::is_same<decltype(Deactivate),std::nullptr_t>::value, \
>>> +                  "Not a display deactivate callback"); \
>>> +    set_callbacks( Activate, Deactivate )
>>> +#endif /* __cplusplus */
>>> +
>>> struct vout_display_t {
>>>      struct vlc_object_t obj;
>>>
>>> -- 
>>> 2.17.1
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>> 
>> -- 
>> 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
>> 
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20190710/dcc23772/attachment.html>