diff --git a/include/valijson/constraints/basic_constraint.hpp b/include/valijson/constraints/basic_constraint.hpp index fd149a0..f7d3ce6 100644 --- a/include/valijson/constraints/basic_constraint.hpp +++ b/include/valijson/constraints/basic_constraint.hpp @@ -36,14 +36,20 @@ struct BasicConstraint: Constraint return visitor.visit(*static_cast(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 + typedef std::unique_ptr RawOwningPointer; + auto ptr = RawOwningPointer(static_cast(allocFn(sizeof(ConstraintType))), freeFn); if (!ptr) { throwRuntimeError("Failed to allocate memory for cloned constraint"); } - return new (ptr) ConstraintType(*static_cast(this)); + // constructor might throw but the memory will be taken care of anyways + (void)new (ptr.get()) ConstraintType(*static_cast(this)); + + // implicitly convert to smart pointer that will also destroy object instance + return ptr; } protected: diff --git a/include/valijson/constraints/concrete_constraints.hpp b/include/valijson/constraints/concrete_constraints.hpp index 55cc124..f6abb63 100644 --- a/include/valijson/constraints/concrete_constraints.hpp +++ b/include/valijson/constraints/concrete_constraints.hpp @@ -903,26 +903,20 @@ public: return visitor.visit(*static_cast(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 + typedef std::unique_ptr RawOwningPointer; + auto ptr = RawOwningPointer(static_cast(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()); + + // implicitly convert to smart pointer that will also destroy object instance + return ptr; } virtual bool validate(const adapters::Adapter &target, diff --git a/include/valijson/constraints/constraint.hpp b/include/valijson/constraints/constraint.hpp index 03b6444..5b3ae3d 100644 --- a/include/valijson/constraints/constraint.hpp +++ b/include/valijson/constraints/constraint.hpp @@ -1,5 +1,8 @@ #pragma once +#include +#include + 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 + struct CustomDeleter + { + CustomDeleter(CustomFree freeFn) + : m_freeFn(freeFn) { } + + void operator()(T *ptr) const + { + auto *nonconst = const_cast::type *>(ptr); + nonconst->~T(); + m_freeFn(nonconst); + } + + private: + CustomFree m_freeFn; + }; + + /// Exclusive-ownership pointer to automatically handle deallocation + typedef std::unique_ptr> 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; }; diff --git a/include/valijson/subschema.hpp b/include/valijson/subschema.hpp index 68acf1d..3cb8ba6 100644 --- a/include/valijson/subschema.hpp +++ b/include/valijson/subschema.hpp @@ -79,11 +79,6 @@ public: #if VALIJSON_USE_EXCEPTIONS try { #endif - for (auto constConstraint : m_constraints) { - auto *constraint = const_cast(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 m_constraints; + std::vector m_constraints; /// Schema description (optional) opt::optional m_description;