[vlc-devel] [PATCH] vlc_common: Add C++ memory management helpers
Hugo Beauzée-Luyssen
hugo at beauzee.fr
Fri Jul 13 10:17:57 CEST 2018
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
More information about the vlc-devel
mailing list