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

Steve Lhomme robux4 at ycbcr.xyz
Wed Jul 10 13:35:17 CEST 2019


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
> 


More information about the vlc-devel mailing list