From bd995868ee2b1206f77f9c9fe03b6936473a82c8 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Mon, 12 Oct 2015 23:57:28 -0500 Subject: [svn-r28047] Purpose: Fix memory leaks Description: - Implemented the friend function void f_PropList_setId(PropList* plist, hid_t new_id) to work around the same problem described in trunk r26655, for the API DataSet::getCreatePlist() - Cleaned up some comments and obsolete functions Platforms tested: Linux/32 2.6 (jam) Linux/64 (platypus) Darwin (osx1010test) --- c++/src/H5AbstractDs.cpp | 12 ------------ c++/src/H5CppDoc.h | 8 ++++---- c++/src/H5DataSet.cpp | 26 +++++++++++++++++++++++++- c++/src/H5DataType.cpp | 3 +-- c++/src/H5IdComponent.cpp | 20 +++----------------- c++/src/H5Location.h | 3 --- c++/src/H5PropList.h | 3 +++ 7 files changed, 36 insertions(+), 39 deletions(-) diff --git a/c++/src/H5AbstractDs.cpp b/c++/src/H5AbstractDs.cpp index 5929444..06b3e22 100644 --- a/c++/src/H5AbstractDs.cpp +++ b/c++/src/H5AbstractDs.cpp @@ -52,18 +52,6 @@ AbstractDs::AbstractDs(){} AbstractDs::AbstractDs(const hid_t ds_id){} //-------------------------------------------------------------------------- -// Function: AbstractDs copy constructor -///\brief Copy constructor: makes a copy of the original AbstractDs object. -// Programmer Binh-Minh Ribler - 2000 -// *** Deprecation warning *** -// This constructor is no longer appropriate because the data member "id" had -// been moved to the sub-classes. It is removed from 1.8.15 because it is -// a noop and it can be generated by the compiler if needed. -//-------------------------------------------------------------------------- -//-------------------------------------------------------------------------- -// AbstractDs::AbstractDs(const AbstractDs& original){} - -//-------------------------------------------------------------------------- // Function: AbstractDs::getTypeClass ///\brief Returns the class of the datatype that is used by this /// object, which can be a dataset or an attribute. diff --git a/c++/src/H5CppDoc.h b/c++/src/H5CppDoc.h index 388fc25..2420586 100644 --- a/c++/src/H5CppDoc.h +++ b/c++/src/H5CppDoc.h @@ -29,10 +29,10 @@ * The C++ API provides C++ wrappers for the HDF5 C Library. * * It is assumed that the user has knowledge of the - * + * * HDF5 file format and its components. * For more information on the HDF5 C Library, see the - * + * * HDF5 Software Documentation page. * * Because the HDF5 C Library maps very well to @@ -57,8 +57,8 @@ * * The HDF5 C++ API is included with the HDF5 source code and can * be obtained from - * - * http://www.hdfgroup.org/HDF5/release/obtainsrc.html. + * + * https://www.hdfgroup.org/HDF5/release/obtainsrc.html. * * Please refer to the release_docs/INSTALL file under the top directory * of the HDF5 source code for information about installing, building, diff --git a/c++/src/H5DataSet.cpp b/c++/src/H5DataSet.cpp index 0fc9105..059da85 100644 --- a/c++/src/H5DataSet.cpp +++ b/c++/src/H5DataSet.cpp @@ -60,6 +60,12 @@ DataSet::DataSet() : H5Object(), AbstractDs(), id(H5I_INVALID_HID) {} ///\brief Creates an DataSet object using the id of an existing dataset. ///\param existing_id - IN: Id of an existing dataset // Programmer Binh-Minh Ribler - 2000 +// Description +// incRefCount() is needed here to prevent the id from being closed +// prematurely. That is, when application uses the id of an +// existing DataSet object to create another DataSet object. So, +// when one of those objects is deleted, the id will be closed if +// the reference counter is only 1. //-------------------------------------------------------------------------- DataSet::DataSet(const hid_t existing_id) : H5Object(), AbstractDs() { @@ -172,8 +178,10 @@ DSetCreatPropList DataSet::getCreatePlist() const { throw DataSetIException("DataSet::getCreatePlist", "H5Dget_create_plist failed"); } + // create and return the DSetCreatPropList object - DSetCreatPropList create_plist(create_plist_id); // ok to use existing id const + DSetCreatPropList create_plist; + f_PropList_setId(&create_plist, create_plist_id); return(create_plist); } @@ -772,6 +780,22 @@ void DataSet::p_setId(const hid_t new_id) // reset object's id to the given id id = new_id; } + +//-------------------------------------------------------------------------- +// Function: f_PropList_setId - friend +// Purpose: This function is friend to class H5::PropList so that it +// can set PropList::id in order to work around a problem +// described in the JIRA issue HDFFV-7947. +// Applications shouldn't need to use it. +// param dset - IN/OUT: DataSet object to be changed +// param new_id - IN: New id to set +// Programmer Binh-Minh Ribler - 2015 +//-------------------------------------------------------------------------- +void f_PropList_setId(PropList* plist, hid_t new_id) +{ + plist->p_setId(new_id); +} + #endif // DOXYGEN_SHOULD_SKIP_THIS //-------------------------------------------------------------------------- diff --git a/c++/src/H5DataType.cpp b/c++/src/H5DataType.cpp index b352f2d..1bbabe3 100644 --- a/c++/src/H5DataType.cpp +++ b/c++/src/H5DataType.cpp @@ -239,8 +239,7 @@ DataType& DataType::operator=( const DataType& rhs ) { if (this != &rhs) { - id = rhs.id; - incRefCount(); // increment number of references to this id + setId(rhs.id); } return(*this); } diff --git a/c++/src/H5IdComponent.cpp b/c++/src/H5IdComponent.cpp index 19d68cf..60735f0 100644 --- a/c++/src/H5IdComponent.cpp +++ b/c++/src/H5IdComponent.cpp @@ -52,20 +52,6 @@ bool IdComponent::H5dontAtexit_called = false; IdComponent::IdComponent(const hid_t h5_id) {} //-------------------------------------------------------------------------- -// Function: IdComponent copy constructor -// Purpose: This noop copy constructor is removed as a result of the data -// member "id" being moved down to sub-classes. (Mar 2015) -// Parameters: original - IN: IdComponent instance to copy -// Programmer Binh-Minh Ribler - 2000 -// -// *** Deprecation warning *** -// This constructor is no longer appropriate because the data member "id" had -// been moved to the sub-classes. It is removed from 1.8.15 because it is -// a noop and it can be generated by the compiler if needed. -//-------------------------------------------------------------------------- -// IdComponent::IdComponent(const IdComponent& original) {} - -//-------------------------------------------------------------------------- // Function: IdComponent::incRefCount ///\brief Increment reference counter for a given id. // Programmer Binh-Minh Ribler - May 2005 @@ -231,7 +217,7 @@ IdComponent& IdComponent::operator=( const IdComponent& rhs ) //-------------------------------------------------------------------------- // Function: IdComponent::setId ///\brief Sets the identifier of this object to a new value. -/// +///\param new_id - IN: New identifier to be set to ///\exception H5::IdComponentException when the attempt to close the HDF5 /// object fails // Description: @@ -246,8 +232,8 @@ IdComponent& IdComponent::operator=( const IdComponent& rhs ) // C++ API object, which will be destroyed properly, and so // p_setId does not call incRefCount. On the other hand, the // public version setId is used by other applications, in which -// the id passed to setId already has a reference count, so setId -// must call incRefCount. +// the id passed to setId is that of another C++ API object, so +// setId must call incRefCount. //-------------------------------------------------------------------------- void IdComponent::setId(const hid_t new_id) { diff --git a/c++/src/H5Location.h b/c++/src/H5Location.h index 79a8d5c..9e4ec05 100644 --- a/c++/src/H5Location.h +++ b/c++/src/H5Location.h @@ -150,9 +150,6 @@ class H5_DLLCPP H5Location : public IdComponent { // Creates a copy of an existing object giving the location id. H5Location(const hid_t loc_id); - // Copy constructor. - // H5Location(const H5Location& original); - // Creates a reference to an HDF5 object or a dataset region. void p_reference(void* ref, const char* name, hid_t space_id, H5R_type_t ref_type) const; diff --git a/c++/src/H5PropList.h b/c++/src/H5PropList.h index be04451..7f6ee31 100644 --- a/c++/src/H5PropList.h +++ b/c++/src/H5PropList.h @@ -127,6 +127,9 @@ class H5_DLLCPP PropList : public IdComponent { // Dynamically allocates the PropList global constant static PropList* getConstant(); + // Friend function to set PropList id. For library use only. + friend void f_PropList_setId(PropList* plist, hid_t new_id); + #endif // DOXYGEN_SHOULD_SKIP_THIS }; -- cgit v0.12