From 14e308b2e6eada778818abf53949ceef0e7b2a34 Mon Sep 17 00:00:00 2001 From: Binh-Minh Ribler Date: Thu, 20 Oct 2016 14:07:45 -0500 Subject: Description: - Removed calls to H5Tget_native_type in the PT code. The application would need to do that if desired. - Added Abhi's program to tests to verify the fix. - This fix might have fixed HDFFV-9927, HDFFV-9042, and the issue reported by Barbara Jones from Ametek as well. Platforms tested: Linux/32 2.6 (jam) Linux/64 (jelly) Darwin (osx1010test) --- hl/c++/test/ptableTest.cpp | 169 +++++++++++++++++++++++++++++---------------- hl/c++/test/ptableTest.h | 4 ++ hl/src/H5PT.c | 14 ++-- hl/test/test_packet.c | 48 ++++++++----- hl/test/test_packet_vlen.c | 67 +++++++----------- 5 files changed, 179 insertions(+), 123 deletions(-) diff --git a/hl/c++/test/ptableTest.cpp b/hl/c++/test/ptableTest.cpp index dd3d2ba..953ec91 100644 --- a/hl/c++/test/ptableTest.cpp +++ b/hl/c++/test/ptableTest.cpp @@ -52,9 +52,7 @@ int main(void) num_errors += SystemTest(); -#ifdef VLPT_REMOVED - num_errors += VariableLengthTest(); -#endif /* VLPT_REMOVED */ + num_errors += TestHDFFV_9758(); /* Terminate access to the file. */ err = H5Fclose(fileID); @@ -562,73 +560,128 @@ error: return 1; } -#ifdef VLPT_REMOVED -int VariableLengthTest(void) +/*------------------------------------------------------------------------- + * TestHDFFV_9758(): Test that a packet table with compound datatype which + * contain string type can be created and written correctly. (HDFFV-9758) + * + * Notes: + * Previously, data of the field that follows the string was read back + * as garbage when #pragma pack(1) is used. + * 2016/10/20 -BMR + *------------------------------------------------------------------------- + */ +#pragma pack(1) // no padding +const char* ABHI_PT("/abhiTest"); +const hsize_t NUM_PACKETS = 5; +const int STRING_LENGTH = 19; // including terminating NULL +int TestHDFFV_9758() { - long test_long; - short test_short; - hvl_t read_buf; - VL_PacketTable* test_VLPT; - PacketTable* new_pt; - - TESTING("variable-length packet tables") - - /* Create a variable length table */ - test_VLPT = new VL_PacketTable(fileID, "/VariableLengthTest", 1); - - /* Verify that the creation succeeded */ - if(! test_VLPT->IsValid()) - goto error; - - /* Append some packets */ - test_short = 9; - test_VLPT->AppendPacket(&test_short, sizeof(short)); - test_long = 16; - test_VLPT->AppendPacket(&test_long, sizeof(long)); - - /* Read them back and make sure they are correct */ - test_VLPT->GetNextPackets(1, &read_buf); - - if(read_buf.len != sizeof(short)) - goto error; - if(*(short *)(read_buf.p) != test_short) - goto error; - - /* Free the memory used by the read */ - test_VLPT->FreeReadbuff(1, &read_buf); - - /* Read the second record */ - test_VLPT->GetNextPackets(1, &read_buf); - - if(read_buf.len != sizeof(long)) - goto error; - if(*(long *)(read_buf.p) != test_long) - goto error; + hid_t strtype; + hid_t compound_type; + herr_t err; + struct s1_t + { + int a; + float b; + double c; + char d[STRING_LENGTH]; // null terminated string + int e; + }; - /* Free the memory used by the read */ - test_VLPT->FreeReadbuff(1, &read_buf); + s1_t s1[NUM_PACKETS]; + + for (hsize_t i = 0; i < NUM_PACKETS; i++) + { + s1[i].a = i; + s1[i].b = 1.f * static_cast(i * i); + s1[i].c = 1. / (i + 1); + sprintf(s1[i].d, "string%d", (int)i); + s1[i].e = 100+i; + } - /* Close the packet table */ - delete test_VLPT; + TESTING("the fix of issue HDFFV-9758") - /* Reopen the packet table and verify that it is variable length */ - new_pt = new PacketTable(fileID, "/VariableLengthTest"); + FL_PacketTable wrapper(fileID, H5P_DEFAULT, ABHI_PT, H5T_NATIVE_INT, 1); + if(! wrapper.IsValid()) + goto error; + + compound_type = H5Tcreate(H5T_COMPOUND, sizeof(s1_t)); + if (compound_type < 0) + goto error; + + err = H5Tinsert(compound_type, "a_name", HOFFSET(s1_t, a), H5T_NATIVE_INT); + if (err < 0) + goto error; + err = H5Tinsert(compound_type, "b_name", HOFFSET(s1_t, b), H5T_NATIVE_FLOAT); + if (err < 0) + goto error; + err = H5Tinsert(compound_type, "c_name", HOFFSET(s1_t, c), H5T_NATIVE_DOUBLE); + if (err < 0) + goto error; + + strtype = H5Tcopy (H5T_C_S1); + if (compound_type < 0) + goto error; + err = H5Tset_size (strtype, STRING_LENGTH); /* create string */ + if (err < 0) + goto error; + err = H5Tinsert(compound_type, "d_name", HOFFSET(s1_t, d), strtype); + if (err < 0) + goto error; + err = H5Tinsert(compound_type, "e_name", HOFFSET(s1_t, e), H5T_NATIVE_INT); + if (err < 0) + goto error; + +{ // so ptable will go out of scope + FL_PacketTable ptable(fileID, "/examplePacketTable", compound_type, 1); + if (not ptable.IsValid()) + goto error; + + for (size_t i = 0; i < NUM_PACKETS; i++) + { + /* Appends one packet at the current position */ + err = ptable.AppendPacket(s1 + i); + if (err < 0) goto error; + } - /* Verify that the open succeeded */ - if(! new_pt->IsValid()) - goto error; + const hsize_t count = ptable.GetPacketCount(err); + if (err < 0) + goto error; + + if (count != NUM_PACKETS) + { + std::cerr + << "Number of packets in packet table should be " << NUM_PACKETS + << " but is " << count << endl; + } - if(new_pt->IsVariableLength() != 1) - goto error; + ptable.ResetIndex(); - /* Close the packet table */ - delete new_pt; + for (size_t i = 0; i < NUM_PACKETS; i++) + { + s1_t s2; + memset(&s2, 0, sizeof(s1_t)); + err = ptable.GetNextPacket(&s2); + if (err < 0) + goto error; + + if (s2.a != s1[i].a || s2.e != s1[i].e) + goto error; + else if (HDstrcmp(s2.d, s1[i].d)) + goto error; + } +} PASSED(); return 0; error: + + H5E_BEGIN_TRY { + H5Fclose(fileID); + } H5E_END_TRY; + H5_FAILED(); return 1; } -#endif /* VLPT_REMOVED */ + diff --git a/hl/c++/test/ptableTest.h b/hl/c++/test/ptableTest.h index d351e34..4ee8967 100644 --- a/hl/c++/test/ptableTest.h +++ b/hl/c++/test/ptableTest.h @@ -52,6 +52,10 @@ int TestGetPacket(void); Test for unusual interactions between multiple packet tables. */ int SystemTest(void); +/* Create a packet table with compound type, which has a string type. Verify + that data was written and read correctly. */ +int TestHDFFV_9758(void); + /* Test the variable length dataset functionality */ int VariableLengthTest(void); diff --git a/hl/src/H5PT.c b/hl/src/H5PT.c index 647cbe8..5f0f94f 100644 --- a/hl/src/H5PT.c +++ b/hl/src/H5PT.c @@ -141,7 +141,9 @@ hid_t H5PTcreate(hid_t loc_id, if(H5Pclose(plistcopy_id) < 0) goto error; - if((table->type_id = H5Tget_native_type(dtype_id, H5T_DIR_DEFAULT)) < 0) + /* Make a copy of caller's datatype and save it in the table structure. + It will be closed when the table is closed */ + if((table->type_id = H5Tcopy(dtype_id)) < 0) goto error; H5PT_create_index(table); @@ -259,12 +261,11 @@ hid_t H5PTcreate_fl ( hid_t loc_id, if(H5Pclose(plist_id) < 0) goto error; + /* Make a copy of caller's datatype and save it in the table structure. + It will be closed when the table is closed */ if((table->type_id = H5Tcopy(dtype_id)) < 0) goto error; - if((table->type_id = H5Tget_native_type(table->type_id, H5T_DIR_DEFAULT)) < 0) - goto error; - H5PT_create_index(table); table->size = 0; @@ -352,8 +353,9 @@ hid_t H5PTopen( hid_t loc_id, if((type_id = H5Dget_type(table->dset_id)) < 0) goto error; - /* Get the table's native datatype */ - if((table->type_id = H5Tget_native_type(type_id, H5T_DIR_ASCEND)) < 0) + /* Make a copy of the datatype obtained and save it in the table structure. + It will be closed when the table is closed */ + if((table->type_id = H5Tcopy(type_id)) < 0) goto error; /* Close the disk datatype */ diff --git a/hl/test/test_packet.c b/hl/test/test_packet.c index e2ca2b5..1817ea4 100644 --- a/hl/test/test_packet.c +++ b/hl/test/test_packet.c @@ -95,8 +95,10 @@ make_particle_type(void) return FAIL; /* Insert fields. */ - string_type = H5Tcopy( H5T_C_S1 ); - H5Tset_size( string_type, (size_t)16 ); + if ((string_type = H5Tcopy(H5T_C_S1)) < 0) + return FAIL; + if (H5Tset_size(string_type, (size_t)16) < 0) + return FAIL; if ( H5Tinsert(type_id, "Name", HOFFSET(particle_t, name) , string_type ) < 0 ) return FAIL; @@ -133,8 +135,10 @@ static int create_hl_table(hid_t fid) herr_t status; /* Initialize the field field_type */ - string_type = H5Tcopy( H5T_C_S1 ); - H5Tset_size( string_type, (size_t)16 ); + if ((string_type = H5Tcopy(H5T_C_S1)) < 0) + return FAIL; + if (H5Tset_size(string_type, (size_t)16) < 0) + return FAIL; field_type[0] = string_type; field_type[1] = H5T_NATIVE_INT; field_type[2] = H5T_NATIVE_INT; @@ -152,12 +156,14 @@ static int create_hl_table(hid_t fid) field_names, part_offset, field_type, chunk_size, fill_data, compress, testPart ); -if(status<0) - return FAIL; -else - return SUCCEED; -} + if (H5Tclose(string_type) < 0) + return FAIL; + if(status<0) + return FAIL; + else + return SUCCEED; +} /*------------------------------------------------------------------------- @@ -183,7 +189,8 @@ static int test_create_close(hid_t fid) /* Create the table */ table = H5PTcreate_fl(fid, PT_NAME, part_t, (hsize_t)100, -1); - H5Tclose(part_t); + if (H5Tclose(part_t) < 0) + goto error; if( H5PTis_valid(table) < 0) goto error; if( H5PTis_varlen(table) != 0) @@ -248,7 +255,7 @@ static int test_append(hid_t fid) { herr_t err; hid_t table; - hsize_t count; + hsize_t count = 0; TESTING("H5PTappend"); @@ -458,7 +465,8 @@ static int test_big_table(hid_t fid) /* Create a new table */ table = H5PTcreate_fl(fid, "Packet Test Dataset2", part_t, (hsize_t)33, -1); - H5Tclose(part_t); + if (H5Tclose(part_t) < 0) + goto error; if( H5PTis_valid(table) < 0) goto error; @@ -536,7 +544,8 @@ static int test_opaque(hid_t fid) /* Create a new table */ table = H5PTcreate_fl(fid, "Packet Test Dataset3", part_t, (hsize_t)100, -1); - H5Tclose(part_t); + if( H5Tclose(part_t) < 0) + goto error; if( H5PTis_valid(table) < 0) goto error; @@ -743,9 +752,9 @@ static int test_rw_nonnative_dt(hid_t fid) /* Create a fixed-length packet table within the file */ /* This table's "packets" will be simple integers and it will use no compression */ if(H5Tget_order(H5T_NATIVE_INT) == H5T_ORDER_LE) { - ptable = H5PTcreate_fl(fid, "Packet Test Dataset, Non-native", H5T_STD_I32BE, (hsize_t)100, -1); + ptable = H5PTcreate(fid, "Packet Test Dataset, Non-native", H5T_STD_I32BE, (hsize_t)100, H5P_DEFAULT); } else { - ptable = H5PTcreate_fl(fid, "Packet Test Dataset, Non-native", H5T_STD_I32LE, (hsize_t)100, -1); + ptable = H5PTcreate(fid, "Packet Test Dataset, Non-native", H5T_STD_I32LE, (hsize_t)100, H5P_DEFAULT); } if(ptable == H5I_INVALID_HID) goto error; @@ -758,12 +767,14 @@ static int test_rw_nonnative_dt(hid_t fid) if( (err = H5PTappend(ptable, (size_t)4, &(writeBuffer[1]))) < 0) goto error; - if( (err = H5PTclose(ptable)) < 0) + /* if( (err = H5PTclose(ptable)) < 0) goto error; + */ /* Open the Packet table */ - if( (ptable = H5PTopen(fid, "Packet Test Dataset, Non-native")) < 0) + /* if( (ptable = H5PTopen(fid, "Packet Test Dataset, Non-native")) < 0) goto error; + */ /* Get the number of packets in the packet table. This should be five. */ if( (err = H5PTget_num_packets(ptable, &count)) < 0) @@ -973,7 +984,8 @@ int main(void) status = 1; /* Close the file */ - H5Fclose(fid); + if (H5Fclose(fid) < 0) + status = 1; return status; } diff --git a/hl/test/test_packet_vlen.c b/hl/test/test_packet_vlen.c index f1e4e00..a1e90a1 100644 --- a/hl/test/test_packet_vlen.c +++ b/hl/test/test_packet_vlen.c @@ -28,7 +28,6 @@ #define PT_COMP_VLEN "Dataset with Compound Type of VL types" #define PT_VLEN_VLEN "Dataset with VL of VL types" #define PT_FIXED_LEN "Fixed-length Packet Table" -#define SPACE3_RANK 1 #define L1_INCM 16 #define L2_INCM 8 #define NAME_BUF_SIZE 80 @@ -228,7 +227,9 @@ static int test_VLof_comptype(void) if (ptable == H5I_INVALID_HID) goto error; - /* Close the vlen datatype */ + /* Release the datatypes */ + if (H5Tclose(cmptype) < 0) + goto error; if (H5Tclose(vltype) < 0) goto error; @@ -311,13 +312,11 @@ static int test_compound_VL_VLtype(void) hvl_t v; } compVLVL_t; hid_t fid=H5I_INVALID_HID; /* Test file identifier */ - hid_t space=H5I_INVALID_HID; /* Dataspace identifier */ hid_t ptable=H5I_INVALID_HID; /* Packet table identifier */ hid_t vlatomic=H5I_INVALID_HID; /* Variable length datatype */ hid_t vlofvl=H5I_INVALID_HID; /* Variable length datatype */ hid_t comp_vlvl=H5I_INVALID_HID; /* ID of a compound datatype containing a VL of VL of atomic datatype */ - hsize_t dims1[] = {NRECORDS}; hsize_t count; /* Number of records in the table */ compVLVL_t writeBuf[NRECORDS];/* Buffer to hold data to be written */ compVLVL_t readBuf[NRECORDS]; /* Buffer to hold read data */ @@ -356,11 +355,6 @@ static int test_compound_VL_VLtype(void) if (fid < 0) goto error; - /* Create dataspace for datasets */ - space = H5Screate_simple(SPACE3_RANK, dims1, NULL); - if (space < 0) - goto error; - /* Create a VL datatype of an atomic type */ vlatomic = H5Tvlen_create (H5T_NATIVE_UINT); if (vlatomic < 0) @@ -394,7 +388,11 @@ static int test_compound_VL_VLtype(void) if (ptable == H5I_INVALID_HID) goto error; - /* Close the vlen datatype */ + /* Release datatypes */ + if (H5Tclose(vlatomic) < 0) + goto error; + if (H5Tclose(vlofvl) < 0) + goto error; if (H5Tclose(comp_vlvl) < 0) goto error; @@ -459,12 +457,6 @@ static int test_compound_VL_VLtype(void) if (ret < 0) goto error; - /* Release datatypes */ - if (H5Tclose(vlatomic) < 0) - goto error; - if (H5Tclose(vlofvl) < 0) - goto error; - /* Close the file */ if (H5Fclose(fid) < 0) goto error; @@ -547,7 +539,9 @@ static int test_VLof_VLtype(void) if (ptable == H5I_INVALID_HID) goto error; - /* Close the vlen datatype */ + /* Release datatypes */ + if (H5Tclose(vlatomic) < 0) + goto error; if (H5Tclose(vlofvl) < 0) goto error; @@ -771,6 +765,10 @@ static int adding_attribute(hid_t fid, const char *table_name, const char *attr_ if (H5Aclose(attr_id) < 0) goto error; + /* Close the dataspace */ + if (H5Sclose(space_id) < 0) + goto error; + /* Close the packet table */ if (H5PTclose(ptable) < 0) goto error; @@ -792,19 +790,12 @@ error: /* An error has occurred. Clean up and exit. */ static herr_t verify_attribute(hid_t fid, const char *table_name, const char *attr_name) { hid_t ptable=H5I_INVALID_HID; /* Packet table identifier */ - hid_t space_id=H5I_INVALID_HID; /* Dataspace for the attribute */ hid_t attr_id=H5I_INVALID_HID; /* Attribute identifier */ hid_t dset_id=H5I_INVALID_HID; /* Dataset associated with the pt */ - hsize_t dims[] = {ATTR_DIM}; /* Dimensions for dataspace */ int read_data[ATTR_DIM]; /* Output buffer */ int ii; herr_t ret = FAIL; /* Returned status from a callee */ - /* Create dataspace for attribute */ - space_id = H5Screate_simple(ATTR_RANK, dims, NULL); - if (space_id < 0) - goto error; - /* Open the named packet table */ ptable = H5PTopen(fid, table_name); if (ptable < 0) @@ -1229,7 +1220,9 @@ static int testfl_VLof_comptype(void) if (ptable == H5I_INVALID_HID) goto error; - /* Close the vlen datatype */ + /* Release the datatypes */ + if (H5Tclose(cmptype) < 0) + goto error; if (H5Tclose(vltype) < 0) goto error; @@ -1312,13 +1305,11 @@ static int testfl_compound_VL_VLtype(void) hvl_t v; } compVLVL_t; hid_t fid=H5I_INVALID_HID; /* Test file identifier */ - hid_t space=H5I_INVALID_HID; /* Dataspace identifier */ hid_t ptable=H5I_INVALID_HID; /* Packet table identifier */ hid_t vlatomic=H5I_INVALID_HID; /* Variable length datatype */ hid_t vlofvl=H5I_INVALID_HID; /* Variable length datatype */ hid_t comp_vlvl=H5I_INVALID_HID; /* ID of a compound datatype containing a VL of VL of atomic datatype */ - hsize_t dims1[] = {NRECORDS}; hsize_t count; /* Number of records in the table */ compVLVL_t writeBuf[NRECORDS];/* Buffer to hold data to be written */ compVLVL_t readBuf[NRECORDS]; /* Buffer to hold read data */ @@ -1357,11 +1348,6 @@ static int testfl_compound_VL_VLtype(void) if (fid < 0) goto error; - /* Create dataspace for datasets */ - space = H5Screate_simple(SPACE3_RANK, dims1, NULL); - if (space < 0) - goto error; - /* Create a VL datatype of an atomic type */ vlatomic = H5Tvlen_create (H5T_NATIVE_UINT); if (vlatomic < 0) @@ -1395,7 +1381,11 @@ static int testfl_compound_VL_VLtype(void) if (ptable == H5I_INVALID_HID) goto error; - /* Close the vlen datatype */ + /* Release datatypes */ + if (H5Tclose(vlatomic) < 0) + goto error; + if (H5Tclose(vlofvl) < 0) + goto error; if (H5Tclose(comp_vlvl) < 0) goto error; @@ -1460,12 +1450,6 @@ static int testfl_compound_VL_VLtype(void) if (ret < 0) goto error; - /* Release datatypes */ - if (H5Tclose(vlatomic) < 0) - goto error; - if (H5Tclose(vlofvl) < 0) - goto error; - /* Close the file */ if (H5Fclose(fid) < 0) goto error; @@ -1548,7 +1532,9 @@ static int testfl_VLof_VLtype(void) if (ptable == H5I_INVALID_HID) goto error; - /* Close the vlen datatype */ + /* Release datatypes */ + if (H5Tclose(vlatomic) < 0) + goto error; if (H5Tclose(vlofvl) < 0) goto error; @@ -1654,7 +1640,6 @@ int test_packet_table_with_varlen(void) if (test_accessors() < 0) status = FAIL; - /************************************************************************** Calling test functions for deprecated function H5PTcreate_fl Mar 2016, -BMR -- cgit v0.12