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

[Hugo Beauzée-Luyssen]

On Thu, Jul 12, 2018, at 11:17 PM, Filip Roséen wrote:
> 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.
> 

That's my current plan, new file, new namespace for C++ helpers! Although the issue here is that having this function non-static or in a non-anonymous namespace will trigger a warning if -Wnoexcept-type is enabled (which it is when using -Wall) so maybe this functions will be in an anonymous namespace? 
Or we can disable the warning specifically, which I believe would be safe since we only care about our C ABI (AFAIK)

> 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>`.

Very good point

> 
> > +// 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
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel


-- 
  Hugo Beauzée-Luyssen
  hugo at beauzee.fr