[svn-r16787] Description:

    Fixed to pass parameters to H5Awrite/H5Aread correctly so that
    all Attribute::write and Attribute::read methods work correctly
    for both fixed-length and variable-length string attributes.

    Added more test cases.

Platforms tested:
    Linux/32 2.6 (jam)
    FreeBSD/64 6.3 (liberty)
    SunOS 5.10 (linew)

2 files changed, 156 insertions, 48 deletions

diff --git a/c++/src/H5Attribute.cpp b/c++/src/H5Attribute.cpp
index c90076c..d80286b 100644
--- a/c++/src/H5Attribute.cpp
+++ b/c++/src/H5Attribute.cpp
@@ -34,6 +34,7 @@
 #include "H5DataSpace.h"
 #include "H5File.h"
 #include "H5Attribute.h"
+#include "H5private.h"		// for HDfree
 
 #ifndef H5_NO_NAMESPACE
 namespace H5 {
@@ -95,23 +96,40 @@ void Attribute::write( const DataType& mem_type, const void *buf ) const
 //--------------------------------------------------------------------------
 // Function:	Attribute::write
 ///\brief	This is an overloaded member function, provided for convenience.
-///		It writes a \a std::string to this attribute.
+///		It writes a \a H5std_string to this attribute.
 ///\param	mem_type  - IN: Attribute datatype (in memory)
 ///\param	strg      - IN: Data to be written
 ///\exception	H5::AttributeIException
 // Programmer	Binh-Minh Ribler - Apr, 2003
 //--------------------------------------------------------------------------
-void Attribute::write( const DataType& mem_type, const H5std_string& strg ) const
+void Attribute::write(const DataType& mem_type, const H5std_string& strg) const
 {
-   // Convert string to C-string
-   const char* strg_C;
-   strg_C = strg.c_str();  // strg_C refers to the contents of strg as a C-str
+    // Check if this attribute has variable-len string or fixed-len string and
+    // proceed appropriately.
+    htri_t is_variable_len = H5Tis_variable_str(mem_type.getId());
+    if (is_variable_len < 0)
+    {
+	throw AttributeIException("Attribute::write", "H5Tis_variable_str failed");
+    }
+    // Convert string to C-string
+    const char* strg_C;
+    strg_C = strg.c_str();  // strg_C refers to the contents of strg as a C-str
+    herr_t ret_value = 0;
 
-   herr_t ret_value = H5Awrite( id, mem_type.getId(), &strg_C );
-   if( ret_value < 0 )
-   {
-      throw AttributeIException("Attribute::write", "H5Awrite failed");
-   }
+    // Pass string in differently depends on variable or fixed length
+    if (!is_variable_len)
+    {
+	ret_value = H5Awrite(id, mem_type.getId(), strg_C);
+    }
+    else
+    {
+	// passing third argument by address
+	ret_value = H5Awrite(id, mem_type.getId(), &strg_C);
+    }
+    if (ret_value < 0)
+    {
+	throw AttributeIException("Attribute::write", "H5Awrite failed");
+    }
 }
 
 //--------------------------------------------------------------------------
@@ -127,14 +145,14 @@ void Attribute::read( const DataType& mem_type, void *buf ) const
    herr_t ret_value = H5Aread( id, mem_type.getId(), buf );
    if( ret_value < 0 )
    {
-      throw AttributeIException("Attribute::read", "H5Aread  failed");
+      throw AttributeIException("Attribute::read", "H5Aread failed");
    }
 }
 
 //--------------------------------------------------------------------------
 // Function:	Attribute::read
 ///\brief	This is an overloaded member function, provided for convenience.
-///		It reads a \a std::string from this attribute.
+///		It reads a \a H5std_string from this attribute.
 ///\param	mem_type  - IN: Attribute datatype (in memory)
 ///\param	strg      - IN: Buffer for read string
 ///\exception	H5::AttributeIException
@@ -147,29 +165,59 @@ 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 [(size_t)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.
+    htri_t is_variable_len = H5Tis_variable_str(mem_type.getId());
+    if (is_variable_len < 0)
+    {
+	throw AttributeIException("Attribute::write", "H5Tis_variable_str failed");
+    }
+
+    // Prepare and call C API to read attribute.
+    char *strg_C;
+    herr_t ret_value = 0;
+    if (!is_variable_len)	// only allocate for fixed-len string
+    {
+	strg_C = new char [(size_t)attr_size+1];
+	if (strg_C == NULL)
+	{
+	    throw AttributeIException("Attribute::read", "Unable to allocate buffer to read the attribute");
+	}
+	ret_value = H5Aread(id, mem_type.getId(), strg_C);
+    }
+    else
+    {
+	// no allocation for variable-len string; C library will
+	ret_value = H5Aread(id, mem_type.getId(), &strg_C);
+    }
+
+    if( ret_value < 0 )
+    {
+	if (!is_variable_len)	// only de-allocate for fixed-len string
+	    delete []strg_C;
+	throw AttributeIException("Attribute::read", "H5Aread failed");
+    }
 
-   // Get 'string' from the C char* and release resource
-   strg_C[attr_size] = '\0';
-   strg = strg_C;
-   delete []strg_C;
+    // 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);
+    }
 }
 
 //--------------------------------------------------------------------------
@@ -342,21 +390,21 @@ hid_t Attribute::getId() const
 ///\brief       Sets the identifier of this object to a new value.
 ///
 ///\exception   H5::IdComponentException when the attempt to close the HDF5
-///             object fails
+///		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.
+//		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
 //--------------------------------------------------------------------------
 void Attribute::p_setId(const hid_t new_id)
 {
     // handling references to this old id
     try {
-        close();
+	close();
     }
     catch (Exception close_error) {
-        throw AttributeIException("Attribute::p_setId", close_error.getDetailMsg());
+	throw AttributeIException("Attribute::p_setId", close_error.getDetailMsg());
     }
    // reset object's id to the given id
    id = new_id;
diff --git a/c++/test/tattr.cpp b/c++/test/tattr.cpp
index 73b5a91..fc39c2b 100644
--- a/c++/test/tattr.cpp
+++ b/c++/test/tattr.cpp
@@ -1180,20 +1180,26 @@ static void test_attr_dtype_shared()
 **
 ****************************************************************/
 /* Info for a string attribute */
-const H5std_string ATTRSTR_NAME("String_attr");
+const H5std_string ATTR1_FL_STR_NAME("String_attr 1");
+const H5std_string ATTR2_FL_STR_NAME("String_attr 2");
+const H5std_string ATTR_VL_STR_NAME("String_attr");
 const H5std_string ATTRSTR_DATA("String Attribute");
+const int ATTR_LEN = 17;
 
 static void test_string_attr()
 {
     // Output message about test being performed
-    SUBTEST("Testing Basic Attribute Writing Functions");
+    SUBTEST("Testing I/O on FL and VL String Attributes");
 
     try {
 	// Create file
 	H5File fid1(FILENAME, H5F_ACC_RDWR);
 
-	// Create a variable length string datatype to refer to.
-	StrType type(0, H5T_VARIABLE);
+	//
+	// Fixed-lenth string attributes
+	//
+	// Create a fixed-length string datatype to refer to.
+	StrType fls_type(0, ATTR_LEN);
 
 	// Open the root group.
 	Group root = fid1.openGroup("/");
@@ -1201,24 +1207,78 @@ static void test_string_attr()
 	// Create dataspace for the attribute.
 	DataSpace att_space (H5S_SCALAR);
 
+	/* Test Attribute::write(...,const void *buf) with Fixed len string */
+
 	// Create an attribute for the root group.
-	Attribute gr_attr = root.createAttribute(ATTRSTR_NAME, type, att_space);
+	Attribute gr_flattr1 = root.createAttribute(ATTR1_FL_STR_NAME, fls_type, att_space);
 
 	// Write data to the attribute.
-	gr_attr.write(type, ATTRSTR_DATA);
+	gr_flattr1.write(fls_type, ATTRSTR_DATA.c_str());
+
+	/* Test Attribute::write(...,const H5std_string& strg) with FL string */
+
+	// Create an attribute for the root group.
+	Attribute gr_flattr2 = root.createAttribute(ATTR2_FL_STR_NAME, fls_type, att_space);
+
+	// Write data to the attribute.
+	gr_flattr2.write(fls_type, ATTRSTR_DATA);
+
+	/* Test Attribute::read(...,void *buf) with FL string */
 
 	// Read and verify the attribute string as a string of chars.
+	char flstring_att_check[ATTR_LEN];
+	gr_flattr1.read(fls_type, flstring_att_check);
+	if(HDstrcmp(flstring_att_check, ATTRSTR_DATA.c_str())!=0)
+	    TestErrPrintf("Line %d: Attribute data different: ATTRSTR_DATA=%s,flstring_att_check=%s\n",__LINE__, ATTRSTR_DATA.c_str(), flstring_att_check);
+
+	/* Test Attribute::read(...,H5std_string& strg) with FL string */
+
+	// Read and verify the attribute string as an std::string.
+	H5std_string read_flstr1;
+	gr_flattr1.read(fls_type, read_flstr1);
+	if (read_flstr1 != ATTRSTR_DATA)
+	    TestErrPrintf("Line %d: Attribute data different: ATTRSTR_DATA=%s,read_flstr1=%s\n",__LINE__, ATTRSTR_DATA.c_str(), read_flstr1.c_str());
+
+	// Read and verify the attribute string as a string of chars.
+	HDstrcpy(flstring_att_check, "");
+	gr_flattr2.read(fls_type, flstring_att_check);
+	if(HDstrcmp(flstring_att_check, ATTRSTR_DATA.c_str())!=0)
+	    TestErrPrintf("Line %d: Attribute data different: ATTRSTR_DATA=%s,flstring_att_check=%s\n",__LINE__, ATTRSTR_DATA.c_str(), flstring_att_check);
+
+	/* Test Attribute::read(...,H5std_string& strg) with FL string */
+
+	// Read and verify the attribute string as an std::string.
+	H5std_string read_flstr2;
+	gr_flattr2.read(fls_type, read_flstr2);
+	if (read_flstr2 != ATTRSTR_DATA)
+	    TestErrPrintf("Line %d: Attribute data different: ATTRSTR_DATA=%s,read_flstr2=%s\n",__LINE__, ATTRSTR_DATA.c_str(), read_flstr2.c_str());
+
+	//
+	// Variable-lenth string attributes
+	//
+	// Create a variable length string datatype to refer to.
+	StrType vls_type(0, H5T_VARIABLE);
+
+	// Create an attribute for the root group.
+	Attribute gr_vlattr = root.createAttribute(ATTR_VL_STR_NAME, vls_type, att_space);
+
+	// Write data to the attribute.
+	gr_vlattr.write(vls_type, ATTRSTR_DATA);
+
+	/* Test Attribute::read(...,void *buf) with Variable len string */
+	// Read and verify the attribute string as a string of chars.
 	char *string_att_check;
-	gr_attr.read(type, &string_att_check);
+	gr_vlattr.read(vls_type, &string_att_check);
 	if(HDstrcmp(string_att_check, ATTRSTR_DATA.c_str())!=0)
 	    TestErrPrintf("Line %d: Attribute data different: ATTRSTR_DATA=%s,string_att_check=%s\n",__LINE__, ATTRSTR_DATA.c_str(), string_att_check);
+	HDfree(string_att_check);
 
+	/* Test Attribute::read(...,H5std_string& strg) with VL string */
 	// Read and verify the attribute string as an std::string.
 	H5std_string read_str;
-	gr_attr.read(type, read_str);
+	gr_vlattr.read(vls_type, read_str);
 	if (read_str != ATTRSTR_DATA)
 	    TestErrPrintf("Line %d: Attribute data different: ATTRSTR_DATA=%s,read_str=%s\n",__LINE__, ATTRSTR_DATA.c_str(), read_str.c_str());
-
 	PASSED();
     } // end try block