From 8283ab0ba4a90dfb847dd6e18694cef6344aab5a Mon Sep 17 00:00:00 2001 From: Jan Schlamelcher Date: Wed, 1 Aug 2012 17:52:11 +0200 Subject: [PATCH] Added new class template "OFResourceHandle" for RAII-style (leak free) programming and used it to fix some memory leaks in storescu --- dcmnet/apps/storescu.cc | 6 +- dcmnet/include/dcmtk/dcmnet/assoc.h | 23 ++++++- ofstd/include/dcmtk/ofstd/ofhand.h | 138 +++++++++++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 3 deletions(-) create mode 100644 ofstd/include/dcmtk/ofstd/ofhand.h diff --git a/dcmnet/apps/storescu.cc b/dcmnet/apps/storescu.cc index 471ea46..69eb4b0 100644 --- a/dcmnet/apps/storescu.cc +++ b/dcmnet/apps/storescu.cc @@ -195,7 +195,7 @@ int main(int argc, char *argv[]) T_ASC_Parameters *params; DIC_NODENAME localHost; DIC_NODENAME peerHost; - T_ASC_Association *assoc; + T_ASC_Association_Handle assoc; DcmAssociationConfiguration asccfg; // handler for association configuration profiles #ifdef HAVE_GUSI_H @@ -962,7 +962,7 @@ int main(int argc, char *argv[]) /* create association, i.e. try to establish a network connection to another */ /* DICOM application. This call creates an instance of T_ASC_Association*. */ OFLOG_INFO(storescuLogger, "Requesting Association"); - cond = ASC_requestAssociation(net, params, &assoc); + cond = ASC_requestAssociation(net, params, &assoc.get()); if (cond.bad()) { if (cond == DUL_ASSOCIATIONREJECTED) { T_ASC_RejectParameters rej; @@ -1060,6 +1060,7 @@ int main(int argc, char *argv[]) } } +#if 0 /* destroy the association, i.e. free memory of T_ASC_Association* structure. This */ /* call is the counterpart of ASC_requestAssociation(...) which was called above. */ cond = ASC_destroyAssociation(&assoc); @@ -1067,6 +1068,7 @@ int main(int argc, char *argv[]) OFLOG_FATAL(storescuLogger, DimseCondition::dump(temp_str, cond)); return 1; } +#endif /* drop the network, i.e. free memory of T_ASC_Network* structure. This call */ /* is the counterpart of ASC_initializeNetwork(...) which was called above. */ cond = ASC_dropNetwork(&net); diff --git a/dcmnet/include/dcmtk/dcmnet/assoc.h b/dcmnet/include/dcmtk/dcmnet/assoc.h index d6651e1..d9b1517 100644 --- a/dcmnet/include/dcmtk/dcmnet/assoc.h +++ b/dcmnet/include/dcmtk/dcmnet/assoc.h @@ -1,6 +1,6 @@ /* * - * Copyright (C) 1994-2011, OFFIS e.V. + * Copyright (C) 1994-2012, OFFIS e.V. * All rights reserved. See COPYRIGHT file for details. * * This software and supporting documentation were partly developed by @@ -97,6 +97,7 @@ #include "dcmtk/dcmnet/dicom.h" #include "dcmtk/dcmnet/lst.h" #include "dcmtk/dcmnet/dul.h" +#include "dcmtk/ofstd/ofhand.h" /* ** Constant Definitions @@ -605,6 +606,26 @@ ASC_dropAssociation(T_ASC_Association * association); DCMTK_DCMNET_EXPORT OFCondition ASC_destroyAssociation(T_ASC_Association ** association); +/** This struct wraps a T_ASC_Association pointer into a OFResourceHandle and + * this way enables the RAII paradigm for the T_ASC_Association struct. + */ +struct T_ASC_Association_Handle : public OFResourceHandle +< + T_ASC_Association, + T_ASC_Association_Handle +> +{ + /** Will be called by the OFResourceHandle's destructor to cleanup. + * @note it is very important for this method to take a reference to the + * T_ASC_Association pointer as parameter, since ASC_destroyAssociation sets + * that pointer to NULL to prevent "double destruction". + */ + inline void free( T_ASC_Association*& assoc ) + { + ASC_destroyAssociation( &assoc ); + } +}; + /// @deprecated Please use OFString& ASC_printRejectParameters(OFString&, T_ASC_RejectParameters*) instead. DCMTK_DCMNET_EXPORT void ASC_printRejectParameters( diff --git a/ofstd/include/dcmtk/ofstd/ofhand.h b/ofstd/include/dcmtk/ofstd/ofhand.h new file mode 100644 index 0000000..f20d4c3 --- /dev/null +++ b/ofstd/include/dcmtk/ofstd/ofhand.h @@ -0,0 +1,138 @@ +/* + * + * Copyright (C) 2012, OFFIS e.V. + * All rights reserved. See COPYRIGHT file for details. + * + * This software and supporting documentation were developed by + * + * OFFIS e.V. + * R&D Division Health + * Escherweg 2 + * D-26121 Oldenburg, Germany + * + * + * Module: ofstd + * + * Author: Jan Schlamelcher + * + * Purpose: Resource Handle template for RAII-style (leak free) programming. + * + */ +#ifndef OFHAND_H +#define OFHAND_H +#include "dcmtk/config/osconfig.h" +#include "dcmtk/ofstd/ofcast.h" + +/** This template class allows to create small wrapper classes for resource pointers to + * enable Resource Acquisition Is Initialization (RAII) style (leak free) working with old + * C-style resource objects. + * OFResourceHandle will therefore call the free() method of a supplied "destruction" + * class in its destructor. It uses the curiously recurring template pattern (CRTP) to + * call that free() method although it is a member function from the inheriting class C. + * @param T the type of the underlying resource, e.g. XDisplay for creating XDisplay* + * compatible handles. + * @param C the inheriting "destruction" class that provides the free() method. + * @note A resource handle can not be copy constructed or copy assigned to prevent corruption + * of the underlying resource pointer. However with a compiler that supports c++11 move + * construction and assignment may be used. + * @see T_ASC_Association_Handle for an example that uses this template. + */ +template +class OFResourceHandle +{ +public: +// move support for c++11, aswell as explicit copy semantic deletion. +#if __cplusplus >= 201103L + /// Explicit deletion if copy constructor (only c++11) + OFResourceHandle( const OFResourceHandle& ) = delete; + /// Explicit deletion of copy assignment (only c++11) + OFResourceHandle& operator=( const OFResourceHandle& ) = delete; + + /** Constructor using nullptr as default parameter (only c++11) + * @param t the actual resource pointer (defaults to nullptr). + */ + OFResourceHandle( T* const t = nullptr ) + : m_Resource( t ) {} + + /// Move constructor (only c++11) + OFResourceHandle( OFResourceHandle&& other ) + : m_Resource( other.m_Resource ) + { + other.m_Resource = nullptr; + } + + /// Move assignment (only c++11) + OFResourceHandle& operator=( OFResourceHandle&& other ) + { + if( &other != this ) + { + OFstatic_cast( C&, *this ).free( m_Resource ); + m_Resource = other.m_Resource; + other.m_Resource = nullptr; + } + + return *this; + } +#else + /** Default constructor. + * @param t the actual resource pointer (defaults to NULL). + */ + OFResourceHandle( T* const t = NULL ) + : m_Resource( t ) {} +#endif + + /// Destructor, calls C::free( ). + ~OFResourceHandle() + { + OFstatic_cast( C&, *this ).free( m_Resource ); + } + + /// Cast operator to behave like a normal T* in when required. + operator T*() const + { + return m_Resource; + } + + /** Explicitly get a reference to the underlying resource pointer. + * For example to get the address of that pointer (the address operator + * can not be overloaded). + */ + T*& get() + { + return m_Resource; + } + + /** Explicitly get a reference to the underlying resource pointer (const version). + * For example to get the address of that pointer (the address operator + * can not be overloaded). + */ + T* const & get() const + { + return m_Resource; + } + + /// Access members of the underlying resource pointer. + T* operator->() const + { + return m_Resource; + } + + /// Dereference the underlying resource pointer. + T& operator*() const + { + return *m_Resource; + } + +private: +// disable copy semantics (the hard way) when not having c++11. +#if __cplusplus < 201103L + /// Declare copy constructor private (pre c++11). + OFResourceHandle( const OFResourceHandle& ); + /// Declare copy assignment private (pre c++11). + OFResourceHandle& operator=( const OFResourceHandle& ); +#endif + + /// The actual resource pointer. + T* m_Resource; +}; +#endif -- 1.7.2.5