[vlc-devel] [PATCH] vlc_common: Add C++ memory management helpers

[Filip Roséen]

Hi Hugo,

The helpers currently follow the same naming scheme as we have for
other VLC-functions, though we could just as well put them inside
a new namespace instead of using the `vlc_` prefix.

On 2018-07-12 15:20, Hugo Beauzée-Luyssen wrote:

> Those are coming from my medialibrary branch, but since I plan on using
> them elsewhere at some point, might as well make them VLC wide
> Those helpers won't accept void* so they can't directly wrap a malloc
> call, but we could have additional wrapper that perform the
> reinterpret_cast for the caller.
> ---
>  include/vlc_common.h | 55 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 55 insertions(+)
> 
> diff --git a/include/vlc_common.h b/include/vlc_common.h
> index c88503c1a5..606f5fedf7 100644
> --- a/include/vlc_common.h
> +++ b/include/vlc_common.h
> @@ -1170,6 +1170,61 @@ static inline char *xstrdup (const char *str)
>      return ptr;
>  }
>  
> +/******************************************************************************
> + * C++ memory management helpers
> + ******************************************************************************/
> +
> +#ifdef __cplusplus

Helpers are always nice, but one immediate reaction to the patch is
that these four, as a consequence will force modules written in `C++`
to be compiled as at least `C++11`, something we formerly did not
demand.

This could be circumvented by introducing `#if __cplusplus >= 201103`
so that we do not force an error diagnostic down someones throat who
are for whatever stuck with an older compiler. One could argue that we
should simply say *"hey, you need at least c++11 to compile a vlc
module written in c++"*, though I don't think we should make such
unnecessary requirement.

> +
> +#include <memory>
> +

An anonymous namespace would in my opinion be neater than declaring
each of these entities as individually `static`.

> +// Wraps a C pointer to be released with free.
> +// ex: vlc_wrap_cptr( var_GetString( "foo" ) );
> +template <typename T>
> +static inline std::unique_ptr<T, void (*)(void*)> vlc_wrap_cptr( T* ptr ) noexcept
> +{
> +    return std::unique_ptr<T, decltype( &free )>( ptr, &free );
> +}
> +
> +// Wraps a C array to be released with free.
> +// This is necessary to allow the operator[] to be used
> +template <typename T>
> +static inline std::unique_ptr<T[], void (*)(void*)> vlc_wrap_carray( T* ptr ) noexcept
> +{
> +    return std::unique_ptr<T[], decltype( &free )>( ptr, &free );
> +}
> +
> +// Wraps a pointer with a custom releaser
> +// ex: auto ptr = vlc_wrap_cptr( input_item, &input_item_Release );
> +template <typename T, typename Releaser>
> +static inline auto vlc_wrap_cptr( T* ptr, Releaser&& r )
> +    noexcept( noexcept( std::unique_ptr<T,
> +                        typename std::decay<decltype( r )>::type> {
> +                            ptr, std::forward<Releaser>( r )
> +                        } ) )
> +    -> std::unique_ptr<T, typename std::decay<decltype( r )>::type>
> +{
> +    return std::unique_ptr<T, typename std::decay<decltype( r )>::type>{
> +                ptr, std::forward<Releaser>( r )
> +    };
> +}

`std::decay` lives in `<type_traits>`, and `std::forward` in `<utility>`.

> +// Wraps a C array with a custom releaser
> +template <typename T, typename Releaser>
> +static inline auto vlc_wrap_carray( T* ptr, Releaser&& r )
> +    noexcept( noexcept( std::unique_ptr<T[],
> +                    typename std::decay<decltype( r )>::type> {
> +                        ptr, std::forward<Releaser>( r )
> +                    } ) )
> +    -> std::unique_ptr<T[], typename std::decay<decltype( r )>::type>
> +{
> +    return std::unique_ptr<T[], typename std::decay<decltype( r )>::type>{
> +        ptr, std::forward<Releaser>( r )
> +    };
> +}

As far as I am aware, both of the above overloads with custom
releasers are doing more than they are required in terms of
`noexcept`. If `Releaser` in `std::unique_ptr<class T, class Releaser>`
is not an *lvalue-reference*, it shall not throw an exception in its
*move-* and/or *copy-constructor* (which one depends on the reference
type of `r`).

> +
> +#endif
> +
>  /*****************************************************************************
>   * libvlc features
>   *****************************************************************************/
> -- 
> 2.18.0
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180712/55ddf45c/attachment-0001.html>