[svn-r17216] Purpose: Improve readability

Description:
    Divided Attribute::read into specific private functions for fixed-
    and variable-len string data: p_read_fixed_len and p_read_variable_len.

    Added test to write/read array of strings to attributes.
Platforms tested:
    Linux/32 2.6 (jam)
    FreeBSD/64 6.3 (liberty)
    SunOS 5.10 (linew)

3 files changed, 213 insertions, 106 deletions

diff --git a/c++/src/H5Attribute.cpp b/c++/src/H5Attribute.cpp
index a844b77..b1ab73e 100644
--- a/c++/src/H5Attribute.cpp
+++ b/c++/src/H5Attribute.cpp
@@ -150,50 +150,6 @@ void Attribute::read( const DataType& mem_type, void *buf ) const
 }
 
 //--------------------------------------------------------------------------
-// Function:	Attribute::getInMemDataSize
-///\brief	Gets the size in memory of the attribute's data.
-///\exception	H5::AttributeIException
-// Programmer	Binh-Minh Ribler - 2000
-//--------------------------------------------------------------------------
-size_t Attribute::getInMemDataSize() const
-{
-    // Get the data type of this attribute
-    hid_t mem_type_id = H5Aget_type(id);
-    if (mem_type_id <= 0)
-    {
-	throw AttributeIException("Attribute::getDataSize", "H5Aget_type failed");
-    }
-
-    // Get the data type's size
-    hid_t native_type = H5Tget_native_type(mem_type_id, H5T_DIR_DEFAULT);
-    if (native_type < 0)
-    {
-	throw AttributeIException("Attribute::read", "H5Tget_native_type failed");
-    }
-    size_t type_size = H5Tget_size(native_type);
-    if (type_size == 0)
-    {
-	throw AttributeIException("Attribute::read", "H5Tget_size failed");
-    }
-
-    // Get number of elements of the attribute
-    hid_t space_id = H5Aget_space(id);
-    if (space_id < 0)
-    {
-	throw AttributeIException("Attribute::read", "H5Aget_space failed");
-    }
-    hssize_t num_elements = H5Sget_simple_extent_npoints(space_id);
-    if (num_elements < 0)
-    {
-	throw AttributeIException("Attribute::read", "H5Sget_simple_extent_npoints failed");
-    }
-
-    // Calculate and return the size of the data
-    size_t data_size = type_size * num_elements;
-    return(data_size);
-}
-
-//--------------------------------------------------------------------------
 // Function:	Attribute::read
 ///\brief	This is an overloaded member function, provided for convenience.
 ///		It reads a \a H5std_string from this attribute.
@@ -206,73 +162,76 @@ size_t Attribute::getInMemDataSize() const
 //		Corrected a misunderstanding that H5Aread would allocate
 //		space for the buffer.  Obtained the attribute size and
 //		allocated memory properly. - BMR
+//	Apr 2009
+//		Used getInMemDataSize to get attribute data size. - BMR
+//	Jul 2009
+//		Divided into specific private functions for fixed- and
+//		variable-len string data: p_read_fixed_len and 
+//		p_read_variable_len.  This should improve readability.
 //--------------------------------------------------------------------------
 void Attribute::read(const DataType& mem_type, H5std_string& strg) const
 {
-
     // 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");
+        throw AttributeIException("Attribute::read", "H5Tis_variable_str failed");
     }
 
-    // Prepare and call C API to read attribute.
-    char *strg_C;
-    herr_t ret_value = 0;
-    size_t attr_size;
-    if (!is_variable_len)	// only allocate for fixed-len string
+    if (!is_variable_len)       // only allocate for fixed-len string
     {
-	// Get the size of the attribute's data
-	attr_size = getInMemDataSize();
-
-	if (attr_size > 0)
-	{
-	    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
-	    HDstrcpy(strg_C, "");
+        p_read_fixed_len(mem_type, strg);
     }
     else
     {
-	// no allocation for variable-len string; C library will
-	ret_value = H5Aread(id, mem_type.getId(), &strg_C);
+        p_read_variable_len(mem_type, strg);
     }
-    if( ret_value < 0 )
+}
+
+//--------------------------------------------------------------------------
+// Function:	Attribute::getInMemDataSize
+///\brief	Gets the size in memory of the attribute's data.
+///\return	Size of data (in memory)
+///\exception	H5::AttributeIException
+// Programmer	Binh-Minh Ribler - Apr 2009
+//--------------------------------------------------------------------------
+size_t Attribute::getInMemDataSize() const
+{
+    // Get the data type of this attribute
+    hid_t mem_type_id = H5Aget_type(id);
+    if (mem_type_id <= 0)
     {
-	if (!is_variable_len)	// only de-allocate for fixed-len string
-	    delete []strg_C;
-	throw AttributeIException("Attribute::read", "H5Aread failed");
+	throw AttributeIException("Attribute::getInMemDataSize", "H5Aget_type failed");
     }
 
-    if( ret_value < 0 )
+    // Get the data type's size
+    hid_t native_type = H5Tget_native_type(mem_type_id, H5T_DIR_DEFAULT);
+    if (native_type < 0)
     {
-	if (!is_variable_len)	// only de-allocate for fixed-len string
-	    delete []strg_C;
-	throw AttributeIException("Attribute::read", "H5Aread failed");
+	throw AttributeIException("Attribute::getInMemDataSize", "H5Tget_native_type failed");
+    }
+    size_t type_size = H5Tget_size(native_type);
+    if (type_size == 0)
+    {
+	throw AttributeIException("Attribute::getInMemDataSize", "H5Tget_size failed");
     }
 
-    // Get string from the C char* and release resource allocated locally
-    if (!is_variable_len)
+    // Get number of elements of the attribute
+    hid_t space_id = H5Aget_space(id);
+    if (space_id < 0)
     {
-	if (strg_C != "")
-	    strg_C[attr_size] = '\0';
-	strg = strg_C;
-	delete []strg_C;
+	throw AttributeIException("Attribute::getInMemDataSize", "H5Aget_space failed");
     }
-    // Get string from the C char* and release resource allocated by C API
-    else
+    hssize_t num_elements = H5Sget_simple_extent_npoints(space_id);
+    if (num_elements < 0)
     {
-	strg = strg_C;
-	HDfree(strg_C);
+	throw AttributeIException("Attribute::getInMemDataSize", "H5Sget_simple_extent_npoints failed");
     }
+
+    // Calculate and return the size of the data
+    size_t data_size = type_size * num_elements;
+    return(data_size);
 }
 
 //--------------------------------------------------------------------------
@@ -300,26 +259,6 @@ DataSpace Attribute::getSpace() const
 }
 
 //--------------------------------------------------------------------------
-// Function:	Attribute::p_get_type (private)
-// Purpose	Gets the datatype of this attribute.
-// Return	Id of the datatype
-// Exception	H5::AttributeIException
-// Description
-// 		This private function is used in AbstractDs.
-// Programmer	Binh-Minh Ribler - 2000
-//--------------------------------------------------------------------------
-hid_t Attribute::p_get_type() const
-{
-   hid_t type_id = H5Aget_type( id );
-   if( type_id > 0 )
-      return( type_id );
-   else
-   {
-      throw AttributeIException("", "H5Aget_type failed");
-   }
-}
-
-//--------------------------------------------------------------------------
 // Function:	Attribute::getFileName
 ///\brief	Gets the name of the file, in which this attribute belongs.
 ///\return	File name
@@ -441,6 +380,96 @@ hid_t Attribute::getId() const
 }
 
 //--------------------------------------------------------------------------
+// Function:	Attribute::p_get_type (private)
+// Purpose	Gets the datatype of this attribute.
+// Return	Id of the datatype
+// Exception	H5::AttributeIException
+// Description
+// 		This private function is used in AbstractDs.
+// Programmer	Binh-Minh Ribler - 2000
+//--------------------------------------------------------------------------
+hid_t Attribute::p_get_type() const
+{
+   hid_t type_id = H5Aget_type( id );
+   if( type_id > 0 )
+      return( type_id );
+   else
+   {
+      throw AttributeIException("", "H5Aget_type failed");
+   }
+}
+
+//--------------------------------------------------------------------------
+// Function:	Attribute::p_read_fixed_len (private)
+// brief	Reads a fixed length \a H5std_string from an attribute.
+// param	mem_type  - IN: Attribute datatype (in memory)
+// param	strg      - IN: Buffer for read string
+// exception	H5::AttributeIException
+// Programmer	Binh-Minh Ribler - Jul, 2009
+// Modification
+//	Jul 2009
+//		Separated the fixed length case from the original
+//		Attribute::read
+//--------------------------------------------------------------------------
+void Attribute::p_read_fixed_len(const DataType& mem_type, H5std_string& strg) const
+{
+    // Only allocate for fixed-len string.
+
+    // Get the size of the attribute's data
+    size_t attr_size = getInMemDataSize();
+
+    // If there is data, allocate buffer and read it.
+    if (attr_size > 0)
+    {
+	char *strg_C = NULL;
+
+	strg_C = new char [(size_t)attr_size+1];
+	herr_t ret_value = H5Aread(id, mem_type.getId(), strg_C);
+
+	if( ret_value < 0 )
+	{
+	    delete []strg_C;	// de-allocate for fixed-len string
+	    throw AttributeIException("Attribute::read", "H5Aread failed");
+	}
+
+	// Get string from the C char* and release resource allocated locally
+	strg = strg_C;
+	delete []strg_C;
+    }
+}
+
+//--------------------------------------------------------------------------
+// Function:	Attribute::p_read_variable_len (private)
+// brief	Reads a variable length \a H5std_string from an attribute.
+// param	mem_type  - IN: Attribute datatype (in memory)
+// param	strg      - IN: Buffer for read string
+// exception	H5::AttributeIException
+// Programmer	Binh-Minh Ribler - Jul, 2009
+// Modification
+//	Jul 2009
+//		Separated the variable length case from the original
+//		Attribute::read
+//--------------------------------------------------------------------------
+void Attribute::p_read_variable_len(const DataType& mem_type, H5std_string& strg) const
+{
+
+    // Prepare and call C API to read attribute.
+    char *strg_C;
+
+    // Read attribute, no allocation for variable-len string; C library will
+    herr_t ret_value = H5Aread(id, mem_type.getId(), &strg_C);
+
+    if( ret_value < 0 )
+    {
+	throw AttributeIException("Attribute::read", "H5Aread failed");
+    }
+
+    // Get string from the C char* and release resource allocated by C API
+    strg = strg_C;
+    HDfree(strg_C);
+}
+
+//--------------------------------------------------------------------------
 // Function:    Attribute::p_setId
 ///\brief       Sets the identifier of this object to a new value.
 ///
diff --git a/c++/src/H5Attribute.h b/c++/src/H5Attribute.h
index 00a08a5..abece10 100644
--- a/c++/src/H5Attribute.h
+++ b/c++/src/H5Attribute.h
@@ -82,6 +82,10 @@ class H5_DLLCPP Attribute : public AbstractDs, public IdComponent {
 	// sub-types
 	virtual hid_t p_get_type() const;
 
+	// Reads variable or fixed len strings
+	void p_read_variable_len(const DataType& mem_type, H5std_string& strg) const;
+	void p_read_fixed_len(const DataType& mem_type, H5std_string& strg) const;
+
 	// do not inherit H5Object::iterateAttrs
 	int iterateAttrs() { return 0; }
 
diff --git a/c++/test/tvlstr.cpp b/c++/test/tvlstr.cpp
index 2c22903..0abab1b 100644
--- a/c++/test/tvlstr.cpp
+++ b/c++/test/tvlstr.cpp
@@ -670,6 +670,77 @@ static void test_read_vl_string_attribute()
     }
 } // test_read_vl_string_attribute
 
+
+/*-------------------------------------------------------------------------
+ * Function:	test_vl_stringarray_attribute
+ *
+ * Purpose:	Test writing/reading VL string array to/from attributes.
+ *
+ * Return:	None
+ *
+ * Programmer:	Binh-Minh Ribler
+ *		July, 2009
+ *
+ *-------------------------------------------------------------------------
+ */
+const H5std_string ATTRSTRARR_NAME("StringArray_attr");
+
+static void test_vl_stringarray_attribute()
+{
+    const char *string_att_array[SPACE1_DIM1]= {
+        "Line 1", "Line 2", "Line 3", "Line 4"
+        };   // Information to write
+
+    // Output message about test being performed
+    SUBTEST("Testing writing/reading VL String Array on attribute");
+
+    try {
+	// Open the file
+	H5File file1(FILENAME, H5F_ACC_RDWR);
+
+	// Create a datatype to refer to.
+	StrType tid1(0, H5T_VARIABLE);
+
+	// Open the root group.
+	Group root = file1.openGroup("/");
+
+        // Create dataspace for datasets.
+        hsize_t dims1[] = {SPACE1_DIM1};
+        DataSpace att_space(SPACE1_RANK, dims1);
+
+	// Create an attribute for the root group.
+	Attribute gr_attr = root.createAttribute(ATTRSTRARR_NAME, tid1, att_space);
+
+	// Write data to the attribute.
+	gr_attr.write(tid1, string_att_array);
+
+	// Read and verify the attribute string as a string of chars.
+	// Note: reading by array of H5std_string doesn't work yet.
+	char *string_att_check[SPACE1_DIM1];
+	gr_attr.read(tid1, &string_att_check);
+
+	int ii;
+	for (ii = 0; ii < SPACE1_DIM1; ii++)
+	{
+	    if(HDstrcmp(string_att_check[ii], string_att_array[ii])!=0)
+		TestErrPrintf("Line %d: Attribute data different: written=%s,read=%s\n",__LINE__, string_att_check[ii], string_att_check[ii]);
+
+	    HDfree(string_att_check[ii]);  // note: no need for std::string test
+	}
+
+	// Close group's attribute.
+	gr_attr.close();
+	file1.close();
+
+	PASSED();
+    } // end try block
+
+    // Catch all exceptions.
+    catch (Exception E) {
+	issue_fail_msg("test_string_attr()", __LINE__, __FILE__, E.getCDetailMsg());
+    }
+}   // test_vl_stringarray_attribute()
+
 /* Helper routine for test_vl_rewrite() */
 static void write_scalar_dset(H5File& file, DataType& type, DataSpace& space,
 				char *name, char *data)
@@ -823,6 +894,9 @@ void test_vlstrings()
     test_write_vl_string_attribute();
     test_read_vl_string_attribute();
 
+    // Test using VL string array in attributes
+    test_vl_stringarray_attribute();
+
     // Test writing VL datasets in files with lots of unlinking
     test_vl_rewrite();