[svn-r15386] Purpose: Fix bug

Description:
    A bug in reference counter was exposed when Ray fixed H5Awrite in
    the main library.  ::setId() called incRefCount when it shouldn't.
    Made sure that id's reference counter is manually incremented
    properly in copy constructor and operator= only.  The main library
    handles the rest.

Platforms tested:
    SunOS 5.10 (linew)
    Linux 2.6 (kagiso)
    FreeBSD (duty)
This commit is contained in:
Binh-Minh Ribler
2008-07-18 15:49:32 -05:00
parent 99e21e85c9
commit d6dc8a4dfd
9 changed files with 91 additions and 188 deletions

View File

@@ -26,7 +26,6 @@
#include "H5PropList.h"
#include "H5Object.h"
#include "H5AbstractDs.h"
#include "H5Attribute.h"
#include "H5FaccProp.h"
#include "H5FcreatProp.h"
#include "H5DcreatProp.h"
@@ -34,6 +33,8 @@
#include "H5DataType.h"
#include "H5DataSpace.h"
#include "H5File.h"
#include "H5Attribute.h"
#include "H5private.h" // for HDfree
#ifndef H5_NO_NAMESPACE
namespace H5 {
@@ -147,29 +148,46 @@ void Attribute::read( const DataType& mem_type, void *buf ) const
//--------------------------------------------------------------------------
void Attribute::read( const DataType& mem_type, H5std_string& strg ) const
{
// Get the attribute size and allocate temporary C-string for C API
hsize_t attr_size = H5Aget_storage_size(id);
if (attr_size <= 0)
{
throw AttributeIException("Attribute::read", "Unable to get attribute size before reading");
}
char* strg_C = new char [attr_size+1];
if (strg_C == NULL)
{
throw AttributeIException("Attribute::read", "Unable to allocate buffer to read the attribute");
}
// Get the size of the attribute data.
hsize_t attr_size = H5Aget_storage_size(id);
if (attr_size <= 0)
{
throw AttributeIException("Attribute::read", "Unable to get attribute size before reading");
}
// Call C API to get the attribute data, a string of chars
herr_t ret_value = H5Aread(id, mem_type.getId(), &strg_C);
if( ret_value < 0 )
{
throw AttributeIException("Attribute::read", "H5Aread failed");
}
// Check if this attribute has variable-len string or fixed-len string and
// proceed appropriately.
bool is_variable_len = H5Tis_variable_str(mem_type.getId());
char *strg_C;
void *ptr;
if (!is_variable_len) // only allocate for fixed-len string
{
strg_C = new char [attr_size+1];
ptr = strg_C;
}
else
{
// no allocation for variable-len string; C library will
ptr = &strg_C;
}
// Get 'string' from the C char* and release resource
strg_C[attr_size] = '\0';
strg = strg_C;
delete []strg_C;
// Call C API to get the attribute data, a string of chars
herr_t ret_value = H5Aread(id, mem_type.getId(), ptr);
// Get string from the C char* and release resource allocated locally
if (!is_variable_len)
{
strg_C[attr_size] = '\0';
strg = strg_C;
delete []strg_C;
}
// Get string from the C char* and release resource allocated by C API
else
{
strg = strg_C;
HDfree(strg_C);
}
ptr = NULL;
}
//--------------------------------------------------------------------------
@@ -217,11 +235,11 @@ hid_t Attribute::p_get_type() const
}
//--------------------------------------------------------------------------
// Function: Attribute::getFileName
///\brief Gets the name of the file, in which this attribute belongs.
///\return File name
///\exception H5::IdComponentException
// Programmer Binh-Minh Ribler - Jul, 2004
// Function: Attribute::getFileName
///\brief Gets the name of the file, in which this attribute belongs.
///\return File name
///\exception H5::IdComponentException
// Programmer Binh-Minh Ribler - Jul, 2004
//--------------------------------------------------------------------------
H5std_string Attribute::getFileName() const
{
@@ -381,9 +399,6 @@ void Attribute::setId(const hid_t new_id)
}
// reset object's id to the given id
id = new_id;
// increment the reference counter of the new id
incRefCount();
}
//--------------------------------------------------------------------------
@@ -402,8 +417,10 @@ void Attribute::close()
{
throw AttributeIException("Attribute::close", "H5Aclose failed");
}
// reset the id because the attribute that it represents is now closed
id = 0;
// reset the id when the attribute that it represents is no longer
// referenced
if (getCounter() == 0)
id = 0;
}
}
@@ -419,18 +436,11 @@ void Attribute::close()
//--------------------------------------------------------------------------
Attribute::~Attribute()
{
int counter = getCounter(id);
if (counter > 1)
{
decRefCount(id);
try {
close();
}
else if (counter == 1)
{
try {
close();
} catch (Exception close_error) {
cerr << "Attribute::~Attribute - " << close_error.getDetailMsg() << endl;
}
catch (Exception close_error) {
cerr << "Attribute::~Attribute - " << close_error.getDetailMsg() << endl;
}
}

View File

@@ -15,11 +15,6 @@
#include <string>
// remove when done
#include <iostream>
using std::cerr;
using std::endl;
#include "H5Include.h"
#include "H5Exception.h"
#include "H5IdComponent.h"

View File

@@ -69,11 +69,10 @@ DataSet::DataSet(const hid_t existing_id) : AbstractDs(), H5Object()
///\param original - IN: DataSet instance to copy
// Programmer Binh-Minh Ribler - 2000
//--------------------------------------------------------------------------
DataSet::DataSet(const DataSet& original) : AbstractDs(original), H5Object()
DataSet::DataSet(const DataSet& original) : AbstractDs(original), H5Object(original)
{
id = original.getId();
incRefCount(); // increment number of references to this id
}
//--------------------------------------------------------------------------
@@ -559,7 +558,7 @@ void DataSet::setId(const hid_t new_id)
id = new_id;
// increment the reference counter of the new id
incRefCount();
//incRefCount();
}
//--------------------------------------------------------------------------
@@ -578,8 +577,10 @@ void DataSet::close()
{
throw DataSetIException("DataSet::close", "H5Dclose failed");
}
// reset the id because the dataset that it represents is now closed
id = 0;
// reset the id when the dataset that it represents is no longer
// referenced
if (getCounter() == 0)
id = 0;
}
}

View File

@@ -593,9 +593,6 @@ void DataSpace::setId(const hid_t new_id)
}
// reset object's id to the given id
id = new_id;
// increment the reference counter of the new id
incRefCount();
}
//--------------------------------------------------------------------------
@@ -615,8 +612,10 @@ void DataSpace::close()
{
throw DataSpaceIException("DataSpace::close", "H5Sclose failed");
}
// reset the id because the dataspace that it represents is now closed
id = 0;
// reset the id when the dataspace that it represents is no longer
// referenced
if (getCounter() == 0)
id = 0;
}
}
@@ -632,16 +631,10 @@ void DataSpace::close()
//--------------------------------------------------------------------------
DataSpace::~DataSpace()
{
int counter = getCounter(id);
if (counter > 1)
decRefCount(id);
else if (counter == 1)
{
try {
close();
} catch (Exception close_error) {
cerr << "DataSpace::~DataSpace - " << close_error.getDetailMsg() << endl;
}
try {
close();
} catch (Exception close_error) {
cerr << "DataSpace::~DataSpace - " << close_error.getDetailMsg() << endl;
}
}

View File

@@ -717,9 +717,6 @@ void DataType::setId(const hid_t new_id)
}
// reset object's id to the given id
id = new_id;
// increment the reference counter of the new id
incRefCount();
}
//--------------------------------------------------------------------------
@@ -738,8 +735,10 @@ void DataType::close()
{
throw DataTypeIException(inMemFunc("close"), "H5Tclose failed");
}
// reset the id because the datatype that it represents is now closed
id = 0;
// reset the id when the datatype that it represents is no longer
// referenced
if (getCounter() == 0)
id = 0;
}
}
@@ -755,19 +754,11 @@ void DataType::close()
//--------------------------------------------------------------------------
DataType::~DataType()
{
int counter = getCounter(id);
if (counter > 1)
{
decRefCount(id);
}
else if (counter == 1)
{
try {
close();
} catch (Exception close_error) {
cerr << inMemFunc("~DataType - ") << close_error.getDetailMsg() << endl;
}
}
}
#ifndef H5_NO_NAMESPACE
} // end namespace

View File

@@ -754,9 +754,6 @@ void H5File::setId(const hid_t new_id)
}
// reset object's id to the given id
id = new_id;
// increment the reference counter of the new id
incRefCount();
}
//--------------------------------------------------------------------------
@@ -775,8 +772,10 @@ void H5File::close()
{
throw FileIException("H5File::close", "H5Fclose failed");
}
// reset the id because the file that it represents is now closed
id = 0;
// reset the id when the file that it represents is no longer
// referenced
if (getCounter() == 0)
id = 0;
}
}
@@ -813,18 +812,10 @@ void H5File::throwException(const H5std_string& func_name, const H5std_string& m
//--------------------------------------------------------------------------
H5File::~H5File()
{
int counter = getCounter(id);
if (counter > 1)
{
decRefCount(id);
}
else if (counter == 1)
{
try {
close();
} catch (Exception close_error) {
cerr << "H5File::~H5File - " << close_error.getDetailMsg() << endl;
}
try {
close();
} catch (Exception close_error) {
cerr << "H5File::~H5File - " << close_error.getDetailMsg() << endl;
}
}

View File

@@ -192,9 +192,6 @@ void Group::setId(const hid_t new_id)
}
// reset object's id to the given id
id = new_id;
// increment the reference counter of the new id
incRefCount();
}
//--------------------------------------------------------------------------
@@ -206,8 +203,6 @@ void Group::setId(const hid_t new_id)
//--------------------------------------------------------------------------
void Group::close()
{
/* cerr << "Group::close/p_valid_id" << endl;
*/
if (p_valid_id(id))
{
herr_t ret_value = H5Gclose( id );
@@ -215,8 +210,10 @@ void Group::close()
{
throw GroupIException("Group::close", "H5Gclose failed");
}
// reset the id because the group that it represents is now closed
id = 0;
// reset the id when the group that it represents is no longer
// referenced
if (getCounter() == 0)
id = 0;
}
}

View File

@@ -177,58 +177,12 @@ IdComponent& IdComponent::operator=( const IdComponent& rhs )
// copy the data members from the rhs object
setId(rhs.getId());
/* id = rhs.id;
*/
incRefCount(getId()); // a = b, so there are two objects with the same
// hdf5 id
}
return *this;
}
//--------------------------------------------------------------------------
// Function: IdComponent::setId
///\brief Sets the identifier of this object to a new value.
///
///\exception H5::IdComponentException when the attempt to close the HDF5
/// object fails
// Description:
// The underlaying reference counting in the C library ensures
// that the current valid id of this object is properly closed.
// Then the object's id is reset to the new id.
// Programmer Binh-Minh Ribler - 2000
//--------------------------------------------------------------------------
#if 0
void IdComponent::setId(const hid_t new_id)
{
// handling references to this old id
try {
close();
}
catch (Exception close_error) {
throw IdComponentException(inMemFunc("copy"), close_error.getDetailMsg());
}
// reset object's id to the given id
/* id = new_id;
*/
setId(new_id);
// increment the reference counter of the new id
incRefCount();
}
#endif
//--------------------------------------------------------------------------
// Function: IdComponent::getId
///\brief Returns the id of this object
///\return HDF5 id
// Programmer Binh-Minh Ribler - 2000
//--------------------------------------------------------------------------
/*
hid_t IdComponent::getId () const
{
return(id);
}
*/
//--------------------------------------------------------------------------
// Function: IdComponent destructor
///\brief Noop destructor.
@@ -273,10 +227,7 @@ H5std_string IdComponent::inMemFunc(const char* func_name) const
///\brief Default constructor.
// Programmer Binh-Minh Ribler - 2000
//--------------------------------------------------------------------------
IdComponent::IdComponent() {
/* setId(-1);
*/
}
IdComponent::IdComponent() {}
//--------------------------------------------------------------------------
// Function: IdComponent::p_get_file_name (protected)
@@ -335,31 +286,6 @@ hid_t IdComponent::p_dereference(void* ref)
return(temp_id);
}
//--------------------------------------------------------------------------
// Function: IdComponent::p_reference (protected)
// Purpose Creates a reference to an HDF5 object or a dataset region.
// Parameters
// name - IN: Name of the object to be referenced
// dataspace - IN: Dataspace with selection
// ref_type - IN: Type of reference; default to \c H5R_DATASET_REGION
// Return A reference
// Exception H5::IdComponentException
// Notes This function is incorrect, and will be removed in the near
// future after notifying users of the new APIs ::reference's.
// BMR - Oct 8, 2006
// Programmer Binh-Minh Ribler - May, 2004
//--------------------------------------------------------------------------
/* void* IdComponent::p_reference(const char* name, hid_t space_id, H5R_type_t ref_type) const
{
hobj_ref_t ref;
herr_t ret_value = H5Rcreate(&ref, getId(), name, ref_type, space_id);
if (ret_value < 0)
{
throw IdComponentException("", "H5Rcreate failed");
}
return (reinterpret_cast<void*>(ref));
}
*/
//
// Local functions used in this class
//

View File

@@ -53,7 +53,7 @@ PropList::PropList() : IdComponent(), id(0) {}
///\param original - IN: The original property list to copy
// Programmer Binh-Minh Ribler - 2000
//--------------------------------------------------------------------------
PropList::PropList(const PropList& original) : IdComponent(original)
PropList::PropList(const PropList& original) : IdComponent(original)
{
id = original.getId();
incRefCount(); // increment number of references to this id
@@ -245,9 +245,6 @@ void PropList::setId(const hid_t new_id)
}
// reset object's id to the given id
id = new_id;
// increment the reference counter of the new id
incRefCount();
}
//--------------------------------------------------------------------------
@@ -266,8 +263,10 @@ void PropList::close()
{
throw PropListIException(inMemFunc("close"), "H5Pclose failed");
}
// reset the id because the property list that it represents is now closed
id = 0;
// reset the id when the property list that it represents is no longer
// referenced
if (getCounter() == 0)
id = 0;
}
}