use unique_ptr for memory management in constraints and subschema

include/valijson/constraints/basic_constraint.hpp

@@ -36,14 +36,19 @@ struct BasicConstraint: Constraint
         return visitor.visit(*static_cast<const ConstraintType*>(this));
     }
 
-    Constraint * clone(CustomAlloc allocFn, CustomFree) const override
+    OwningPointer clone(CustomAlloc allocFn, CustomFree freeFn) const override
     {
-        void *ptr = allocFn(sizeof(ConstraintType));
+        // smart pointer to automatically free raw memory on exception
+        auto ptr = std::unique_ptr<void, CustomFree>(allocFn(sizeof(ConstraintType)), freeFn);
         if (!ptr) {
             throwRuntimeError("Failed to allocate memory for cloned constraint");
         }
 
-        return new (ptr) ConstraintType(*static_cast<const ConstraintType*>(this));
+        // constructor might throw but the memory will be taken care of anyways
+        (void)new (ptr.get()) ConstraintType(*static_cast<const ConstraintType*>(this));
+
+        // reassign managed memory to smart pointer that will also destroy object instance
+        return OwningPointer(static_cast<const Constraint*>(ptr.release()), freeFn);
     }
 
 protected:

include/valijson/constraints/concrete_constraints.hpp

@@ -903,26 +903,19 @@ public:
         return visitor.visit(*static_cast<const PolyConstraint*>(this));
     }
 
-    Constraint * clone(CustomAlloc allocFn, CustomFree freeFn) const override
+    OwningPointer clone(CustomAlloc allocFn, CustomFree freeFn) const override
     {
-        void *ptr = allocFn(sizeOf());
+        // smart pointer to automatically free raw memory on exception
+        auto ptr = std::unique_ptr<void, CustomFree>(allocFn(sizeOf()), freeFn);
         if (!ptr) {
             throwRuntimeError("Failed to allocate memory for cloned constraint");
         }
 
-#if VALIJSON_USE_EXCEPTIONS
-        try {
-#endif
-            return cloneInto(ptr);
-#if VALIJSON_USE_EXCEPTIONS
-        } catch (...) {
-            freeFn(ptr);
-            throw;
-        }
-#else
-        // pretend to use freeFn to avoid warning in GCC 8.3
-        (void)freeFn;
-#endif
+        // constructor might throw but the memory will be taken care of anyways
+        (void)cloneInto(ptr.get());
+
+        // reassign managed memory to smart pointer that will also destroy object instance
+        return OwningPointer(static_cast<const Constraint*>(ptr.release()), freeFn);
     }
 
     virtual bool validate(const adapters::Adapter &target,

include/valijson/constraints/constraint.hpp

@@ -1,5 +1,8 @@
 #pragma once
 
+#include <memory>
+#include <type_traits>
+
 namespace valijson {
 namespace constraints {
 
@@ -18,6 +21,28 @@ struct Constraint
     /// Typedef for custom free-like function
     typedef void (*CustomFree)(void *);
 
+    /// Deleter type to be used with std::unique_ptr / std::shared_ptr
+    /// @tparam  T  Const or non-const type (same as the one used in unique_ptr/shared_ptr)
+    template<typename T>
+    struct CustomDeleter
+    {
+        CustomDeleter(CustomFree freeFn)
+          : m_freeFn(freeFn) { }
+
+        void operator()(T *ptr) const
+        {
+            auto *nonconst = const_cast<typename std::remove_const<T>::type *>(ptr);
+            nonconst->~T();
+            m_freeFn(nonconst);
+        }
+
+    private:
+        CustomFree m_freeFn;
+    };
+
+    /// Exclusive-ownership pointer to automatically handle deallocation
+    typedef std::unique_ptr<const Constraint, CustomDeleter<const Constraint>> OwningPointer;
+
     /**
      * @brief  Virtual destructor.
      */
@@ -42,7 +67,7 @@ struct Constraint
      *
      * @returns  an owning-pointer to the new constraint.
      */
-    virtual Constraint * clone(CustomAlloc, CustomFree) const = 0;
+    virtual OwningPointer clone(CustomAlloc, CustomFree) const = 0;
 
 };
 

include/valijson/subschema.hpp

@@ -79,11 +79,6 @@ public:
 #if VALIJSON_USE_EXCEPTIONS
         try {
 #endif
-            for (auto constConstraint : m_constraints) {
-                auto *constraint = const_cast<Constraint *>(constConstraint);
-                constraint->~Constraint();
-                m_freeFn(constraint);
-            }
             m_constraints.clear();
 #if VALIJSON_USE_EXCEPTIONS
         } catch (const std::exception &e) {
@@ -106,18 +101,8 @@ public:
      */
     void addConstraint(const Constraint &constraint)
     {
-        Constraint *newConstraint = constraint.clone(m_allocFn, m_freeFn);
-#if VALIJSON_USE_EXCEPTIONS
-        try {
-#endif
-            m_constraints.push_back(newConstraint);
-#if VALIJSON_USE_EXCEPTIONS
-        } catch (...) {
-            newConstraint->~Constraint();
-            m_freeFn(newConstraint);
-            throw;
-        }
-#endif
+        // the vector allocation might throw but the constraint memory will be taken care of anyways
+        m_constraints.push_back(constraint.clone(m_allocFn, m_freeFn));
     }
 
     /**
@@ -134,7 +119,7 @@ public:
     bool apply(ApplyFunction &applyFunction) const
     {
         bool allTrue = true;
-        for (const Constraint *constraint : m_constraints) {
+        for (auto &&constraint : m_constraints) {
             allTrue = applyFunction(*constraint) && allTrue;
         }
 
@@ -153,7 +138,7 @@ public:
      */
     bool applyStrict(ApplyFunction &applyFunction) const
     {
-        for (const Constraint *constraint : m_constraints) {
+        for (auto &&constraint : m_constraints) {
             if (!applyFunction(*constraint)) {
                 return false;
             }
@@ -296,7 +281,7 @@ private:
     bool m_alwaysInvalid;
 
     /// List of pointers to constraints that apply to this schema.
-    std::vector<const Constraint *> m_constraints;
+    std::vector<Constraint::OwningPointer> m_constraints;
 
     /// Schema description (optional)
     opt::optional<std::string> m_description;