From 11af96e8677f8d22a3bacb745eee09576fdb9eec Mon Sep 17 00:00:00 2001 From: James Laird Date: Wed, 3 Jan 2007 12:48:51 -0500 Subject: [svn-r13102] Found a bug: shared message list sizes above the maximum were not caught. Fixed bug and a related one where the number of indexes could be set above the maximum. Added tests for both bugs. Tested on kagiso and smirom. --- src/H5FL.c | 6 +++--- src/H5Fsuper.c | 6 +++--- src/H5Opublic.h | 7 +++++++ src/H5Pfcpl.c | 34 +++++++++++++++++----------------- src/H5SM.c | 10 +++++----- src/H5SMcache.c | 2 +- src/H5SMpkg.h | 3 --- src/H5SMprivate.h | 2 -- test/tsohm.c | 44 ++++++++++++++++++++++++++++++++------------ 9 files changed, 68 insertions(+), 46 deletions(-) diff --git a/src/H5FL.c b/src/H5FL.c index e75078f..7a20d28 100644 --- a/src/H5FL.c +++ b/src/H5FL.c @@ -1503,12 +1503,12 @@ H5FL_arr_malloc(H5FL_arr_head_t *head, size_t elem) if(H5FL_arr_init(head)<0) HGOTO_ERROR (H5E_RESOURCE, H5E_CANTINIT, NULL, "can't initialize 'array' blocks") + /* Sanity check that the number of elements is supported */ + assert(elem<=(unsigned) head->maxelem); + /* Get the set of the memory block */ mem_size=head->list_arr[elem].size; - /* Sanity check that the number of elements is supported */ - assert((int)elem<=head->maxelem); - /* Check for nodes available on the free list first */ if(head->list_arr[elem].list!=NULL) { /* Get a pointer to the block on the free list */ diff --git a/src/H5Fsuper.c b/src/H5Fsuper.c index 08eab0e..821981c 100644 --- a/src/H5Fsuper.c +++ b/src/H5Fsuper.c @@ -372,12 +372,12 @@ H5F_read_superblock(H5F_t *f, hid_t dxpl_id, H5G_loc_t *root_loc, haddr_t addr, * fcpl */ if(shared->sohm_addr != HADDR_UNDEF) { - unsigned index_flags[H5SM_MAX_NUM_INDEXES] = {0}; - unsigned minsizes[H5SM_MAX_NUM_INDEXES] = {0}; + unsigned index_flags[H5SM_MAX_NINDEXES] = {0}; + unsigned minsizes[H5SM_MAX_NINDEXES] = {0}; unsigned sohm_l2b; /* SOHM list-to-btree cutoff */ unsigned sohm_b2l; /* SOHM btree-to-list cutoff */ - HDassert(shared->sohm_nindexes > 0 && shared->sohm_nindexes <= H5SM_MAX_NUM_INDEXES); + HDassert(shared->sohm_nindexes > 0 && shared->sohm_nindexes <= H5SM_MAX_NINDEXES); /* Read in the shared OH message information if there is any */ if(H5SM_get_info(f, index_flags, minsizes, &sohm_l2b, &sohm_b2l, dxpl_id) < 0) diff --git a/src/H5Opublic.h b/src/H5Opublic.h index 573c3ec..4d4e59b 100644 --- a/src/H5Opublic.h +++ b/src/H5Opublic.h @@ -59,6 +59,13 @@ #define H5O_MESG_ATTR_FLAG 0x0010 /* Attribute Message. */ #define H5O_MESG_ALL_FLAG (H5O_MESG_SDSPACE_FLAG | H5O_MESG_DTYPE_FLAG | H5O_MESG_FILL_FLAG | H5O_MESG_PLINE_FLAG | H5O_MESG_ATTR_FLAG) +/* Maximum shared message values. Number of indexes is 8 to allow room to add + * new types of messages. + */ +/* JAMES: make these H5O* */ +#define H5SM_MAX_NINDEXES 8 +#define H5SM_MAX_LIST_ELEMS 5000 + /*******************/ /* Public Typedefs */ /*******************/ diff --git a/src/H5Pfcpl.c b/src/H5Pfcpl.c index 4c1db16..1b49360 100644 --- a/src/H5Pfcpl.c +++ b/src/H5Pfcpl.c @@ -75,9 +75,9 @@ /* Definitions for shared object header messages */ #define H5F_CRT_SHMSG_NINDEXES_SIZE sizeof(unsigned) #define H5F_CRT_SHMSG_NINDEXES_DEF (0) -#define H5F_CRT_SHMSG_INDEX_TYPES_SIZE sizeof(unsigned[H5SM_MAX_NUM_INDEXES]) +#define H5F_CRT_SHMSG_INDEX_TYPES_SIZE sizeof(unsigned[H5SM_MAX_NINDEXES]) #define H5F_CRT_SHMSG_INDEX_TYPES_DEF {0,0,0,0,0,0} -#define H5F_CRT_SHMSG_INDEX_MINSIZE_SIZE sizeof(unsigned[H5SM_MAX_NUM_INDEXES]) +#define H5F_CRT_SHMSG_INDEX_MINSIZE_SIZE sizeof(unsigned[H5SM_MAX_NINDEXES]) #define H5F_CRT_SHMSG_INDEX_MINSIZE_DEF {250,250,250,250,250,250} /* Definitions for shared object header list/btree phase change cutoffs */ #define H5F_CRT_SHMSG_LIST_MAX_SIZE sizeof(unsigned) @@ -158,8 +158,8 @@ H5P_fcrt_reg_prop(H5P_genclass_t *pclass) unsigned objectdir_ver = H5F_CRT_OBJ_DIR_VERS_DEF; /* Default object directory version # */ unsigned sharedheader_ver = H5F_CRT_SHARE_HEAD_VERS_DEF; /* Default shared header message version # */ unsigned num_sohm_indexes = H5F_CRT_SHMSG_NINDEXES_DEF; - unsigned sohm_index_flags[H5SM_MAX_NUM_INDEXES] = H5F_CRT_SHMSG_INDEX_TYPES_DEF; - unsigned sohm_index_minsizes[H5SM_MAX_NUM_INDEXES] = H5F_CRT_SHMSG_INDEX_MINSIZE_DEF; + unsigned sohm_index_flags[H5SM_MAX_NINDEXES] = H5F_CRT_SHMSG_INDEX_TYPES_DEF; + unsigned sohm_index_minsizes[H5SM_MAX_NINDEXES] = H5F_CRT_SHMSG_INDEX_MINSIZE_DEF; unsigned sohm_list_max = H5F_CRT_SHMSG_LIST_MAX_DEF; unsigned sohm_btree_min = H5F_CRT_SHMSG_BTREE_MIN_DEF; herr_t ret_value = SUCCEED; /* Return value */ @@ -689,7 +689,7 @@ done: * for this file. * * NINDEXES is the number of indexes for this file; it should - * be between 0 and H5SM_MAX_NUM_INDEXES. If nindexes is 0, + * be between 0 and H5SM_MAX_NINDEXES. If nindexes is 0, * SOHMs will be disabled for this file. * * MESG_TYPE_FLAGS is an array of message type flags (using @@ -711,7 +711,7 @@ herr_t H5Pset_shared_mesgs(hid_t plist_id, unsigned nindexes, const unsigned mesg_type_flags[]) { unsigned i; - unsigned type_flags[H5SM_MAX_NUM_INDEXES]; /* Full-sized array */ + unsigned type_flags[H5SM_MAX_NINDEXES]; /* Full-sized array */ H5P_genplist_t *plist; /* Property list pointer */ unsigned flags_used; /* type flags already specified. * Used to make sure a flag isn't used twice. @@ -722,7 +722,7 @@ H5Pset_shared_mesgs(hid_t plist_id, unsigned nindexes, const unsigned mesg_type_ H5TRACE3("e", "iIu*Iu", plist_id, nindexes, mesg_type_flags); /* Check arguments */ - if(nindexes > H5SM_MAX_NUM_INDEXES) + if(nindexes > H5SM_MAX_NINDEXES) HGOTO_ERROR(H5E_ARGS, H5E_BADRANGE, FAIL, "number of indexes is too large"); if(nindexes > 0 && !mesg_type_flags) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no type flags specified"); @@ -781,8 +781,8 @@ H5Pset_shared_mesg_nindexes(hid_t plist_id, unsigned nindexes) H5TRACE2("e", "iIu", plist_id, nindexes); /* Check argument */ - if (nindexes > H5SM_MAX_NUM_INDEXES) - HGOTO_ERROR(H5E_ARGS, H5E_BADRANGE, FAIL, "number of indexes is greater than H5SM_MAX_NUM_INDEXES"); + if (nindexes > H5SM_MAX_NINDEXES) + HGOTO_ERROR(H5E_ARGS, H5E_BADRANGE, FAIL, "number of indexes is greater than H5SM_MAX_NINDEXES"); /* Get the plist structure */ if(NULL == (plist = H5P_object_verify(plist_id, H5P_FILE_CREATE))) @@ -849,8 +849,8 @@ H5Pset_shared_mesg_index(hid_t plist_id, unsigned index_num, unsigned mesg_type_ { H5P_genplist_t *plist; /* Property list pointer */ unsigned nindexes; /* Number of SOHM indexes */ - unsigned type_flags[H5SM_MAX_NUM_INDEXES]; /* Array of mesg_type_flags*/ - unsigned minsizes[H5SM_MAX_NUM_INDEXES]; /* Array of min_mesg_sizes*/ + unsigned type_flags[H5SM_MAX_NINDEXES]; /* Array of mesg_type_flags*/ + unsigned minsizes[H5SM_MAX_NINDEXES]; /* Array of min_mesg_sizes*/ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_API(H5Pset_shared_mesg_index, FAIL) @@ -914,8 +914,8 @@ H5Pget_shared_mesg_index(hid_t plist_id, unsigned index_num, unsigned *mesg_type { H5P_genplist_t *plist; /* Property list pointer */ unsigned nindexes; /* Number of SOHM indexes */ - unsigned type_flags[H5SM_MAX_NUM_INDEXES]; /* Array of mesg_type_flags*/ - unsigned minsizes[H5SM_MAX_NUM_INDEXES]; /* Array of min_mesg_sizes*/ + unsigned type_flags[H5SM_MAX_NINDEXES]; /* Array of mesg_type_flags*/ + unsigned minsizes[H5SM_MAX_NINDEXES]; /* Array of min_mesg_sizes*/ herr_t ret_value=SUCCEED; /* Return value */ FUNC_ENTER_API(H5Pget_shared_mesg_index, FAIL); @@ -989,10 +989,10 @@ H5Pset_shared_mesg_phase_change(hid_t plist_id, unsigned max_list, unsigned min_ */ if(max_list + 1 < min_btree) HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "minimum B-tree value is greater than maximum list value") - if(max_list > 65535) - HGOTO_ERROR(H5E_ARGS, H5E_BADRANGE, FAIL, "max list value must be < 65536") - if(min_btree > 65535) - HGOTO_ERROR(H5E_ARGS, H5E_BADRANGE, FAIL, "min btree value must be < 65536") + if(max_list > H5SM_MAX_LIST_ELEMS) + HGOTO_ERROR(H5E_ARGS, H5E_BADRANGE, FAIL, "max list value is larger than H5SM_MAX_LIST_ELEMS") + if(min_btree > H5SM_MAX_LIST_ELEMS) + HGOTO_ERROR(H5E_ARGS, H5E_BADRANGE, FAIL, "min btree value is larger than H5SM_MAX_LIST_ELEMS") /* Avoid the strange case where max_list == 0 and min_btree == 1, so deleting the * last message in a B-tree makes it become an empty list. diff --git a/src/H5SM.c b/src/H5SM.c index ae00a51..65d2392 100755 --- a/src/H5SM.c +++ b/src/H5SM.c @@ -67,7 +67,7 @@ static herr_t H5SM_type_to_flag(unsigned type_id, unsigned *type_flag); /*********************/ H5FL_DEFINE(H5SM_master_table_t); -H5FL_ARR_DEFINE(H5SM_index_header_t, H5SM_MAX_INDEXES); +H5FL_ARR_DEFINE(H5SM_index_header_t, H5SM_MAX_NINDEXES); H5FL_DEFINE(H5SM_list_t); H5FL_ARR_DEFINE(H5SM_sohm_t, H5SM_MAX_LIST_ELEMS); @@ -105,8 +105,8 @@ H5SM_init(H5F_t *f, H5P_genplist_t * fc_plist, hid_t dxpl_id) haddr_t table_addr = HADDR_UNDEF; unsigned num_indexes; unsigned list_to_btree, btree_to_list; - unsigned index_type_flags[H5SM_MAX_NUM_INDEXES]; - unsigned minsizes[H5SM_MAX_NUM_INDEXES]; + unsigned index_type_flags[H5SM_MAX_NINDEXES]; + unsigned minsizes[H5SM_MAX_NINDEXES]; unsigned type_flags_used; unsigned x; hsize_t table_size; @@ -135,7 +135,7 @@ H5SM_init(H5F_t *f, H5P_genplist_t * fc_plist, hid_t dxpl_id) HGOTO_ERROR(H5E_PLIST, H5E_CANTGET, FAIL, "can't get SOHM message min sizes") /* Verify that values are valid */ - if(num_indexes > H5SM_MAX_NUM_INDEXES) + if(num_indexes > H5SM_MAX_NINDEXES) HGOTO_ERROR(H5E_PLIST, H5E_BADRANGE, FAIL, "number of indexes in property list is too large") /* Check that type flags weren't duplicated anywhere */ @@ -162,7 +162,7 @@ H5SM_init(H5F_t *f, H5P_genplist_t * fc_plist, hid_t dxpl_id) * min. */ HDassert(list_to_btree + 1 >= btree_to_list); - HDassert(table->num_indexes > 0 && table->num_indexes <= H5SM_MAX_NUM_INDEXES); + HDassert(table->num_indexes > 0 && table->num_indexes <= H5SM_MAX_NINDEXES); /* Allocate the SOHM indexes as an array. */ if(NULL == (table->indexes = (H5SM_index_header_t *)H5FL_ARR_MALLOC(H5SM_index_header_t, (size_t)table->num_indexes))) diff --git a/src/H5SMcache.c b/src/H5SMcache.c index 1786c74..78b5033 100644 --- a/src/H5SMcache.c +++ b/src/H5SMcache.c @@ -43,7 +43,7 @@ /****************/ /* Local Macros */ /****************/ -#define H5F_TABLEBUF_SIZE H5SM_TABLE_SIZEOF_MAGIC + 20 + (H5SM_MAX_INDEXES * 26) +#define H5F_TABLEBUF_SIZE H5SM_TABLE_SIZEOF_MAGIC + 20 + (H5SM_MAX_NINDEXES * 26) /* JAMES: should this change according to address size? */ #define H5F_LISTBUF_SIZE H5SM_LIST_SIZEOF_MAGIC + H5SM_MAX_LIST_ELEMS * 16 diff --git a/src/H5SMpkg.h b/src/H5SMpkg.h index 342b2bc..643b1fd 100755 --- a/src/H5SMpkg.h +++ b/src/H5SMpkg.h @@ -60,9 +60,6 @@ + (H5SM_SOHM_ENTRY_SIZE(f) * num_mesg) \ + H5SM_SIZEOF_CHECKSUM /* Checksum */ -#define H5SM_MAX_INDEXES 8 -#define H5SM_MAX_LIST_ELEMS 1000 - #define H5SM_B2_NODE_SIZE 512 #define H5SM_B2_SPLIT_PERCENT 100 #define H5SM_B2_MERGE_PERCENT 40 diff --git a/src/H5SMprivate.h b/src/H5SMprivate.h index 37d4689..9cfad58 100755 --- a/src/H5SMprivate.h +++ b/src/H5SMprivate.h @@ -29,8 +29,6 @@ /* Library Private Typedefs */ /****************************/ -#define H5SM_MAX_NUM_INDEXES 6 - /******************************/ /* Library Private Prototypes */ /******************************/ diff --git a/test/tsohm.c b/test/tsohm.c index 17b62ce..553a0af 100644 --- a/test/tsohm.c +++ b/test/tsohm.c @@ -22,29 +22,23 @@ #include "testhdf5.h" -/* Maximum number of SOHM indexes in a file. Should correspond - * to H5SM_MAX_NUM_INDEXES - */ -/* JAMES: get these three from default fcpl */ -#define MAX_INDEXES 6 - /* Default SOHM values */ #define DEF_NUM_INDEXES 0 -const unsigned def_type_flags[MAX_INDEXES] = {0,0,0,0,0,0}; -const unsigned def_minsizes[MAX_INDEXES] = {250,250,250,250,250,250}; +const unsigned def_type_flags[H5SM_MAX_NINDEXES] = {0,0,0,0,0,0}; +const unsigned def_minsizes[H5SM_MAX_NINDEXES] = {250,250,250,250,250,250}; #define DEF_L2B 50 #define DEF_B2L 40 /* Non-default SOHM values for testing */ /* JAMES: make these defined in function */ #define TEST_NUM_INDEXES 4 -const unsigned test_type_flags[MAX_INDEXES] = +const unsigned test_type_flags[H5SM_MAX_NINDEXES] = {H5O_MESG_FILL_FLAG, H5O_MESG_DTYPE_FLAG | H5O_MESG_ATTR_FLAG, H5O_MESG_SDSPACE_FLAG, H5O_MESG_PLINE_FLAG, 0, 0}; -const unsigned test_minsizes[MAX_INDEXES] = {0, 2, 40, 100, 3, 1000}; +const unsigned test_minsizes[H5SM_MAX_NINDEXES] = {0, 2, 40, 100, 3, 1000}; #define TEST_L2B 65 #define TEST_B2L 64 @@ -299,12 +293,16 @@ static void test_sohm_fcpl(void) /* Test giving bogus values to H5P* functions */ H5E_BEGIN_TRY { + /* Trying to create too many indexes should fail */ + ret = H5Pset_shared_mesg_nindexes(fcpl_id, H5SM_MAX_NINDEXES + 1); + VERIFY(ret, -1, "H5Pset_shared_mesg_nindexes"); + /* Trying to set index 0 or an index higher than the current number * of indexes should fail. */ - ret = H5Pset_shared_mesg_index(fcpl_id, 0, 0, 15 /* JAMES */); + ret = H5Pset_shared_mesg_index(fcpl_id, 0, 0, 15); VERIFY(ret, -1, "H5Pset_shared_mesg_index"); - ret = H5Pset_shared_mesg_index(fcpl_id, MAX_INDEXES + 1, 0, 15); + ret = H5Pset_shared_mesg_index(fcpl_id, H5SM_MAX_NINDEXES + 1, 0, 15); VERIFY(ret, -1, "H5Pset_shared_mesg_index"); ret = H5Pset_shared_mesg_index(fcpl_id, TEST_NUM_INDEXES + 1, 0, 15); VERIFY(ret, -1, "H5Pset_shared_mesg_index"); @@ -331,8 +329,16 @@ static void test_sohm_fcpl(void) */ ret = H5Pset_shared_mesg_phase_change(fcpl_id, 10, 12); VERIFY(ret, -1, "H5Pset_shared_mesg_phase_change"); + /* Setting them to extremely large values should also fail */ + ret = H5Pset_shared_mesg_phase_change(fcpl_id, H5SM_MAX_LIST_ELEMS + 1, 0); + VERIFY(ret, -1, "H5Pset_shared_mesg_phase_change"); + ret = H5Pset_shared_mesg_phase_change(fcpl_id, 10, H5SM_MAX_LIST_ELEMS + 10); + VERIFY(ret, -1, "H5Pset_shared_mesg_phase_change"); + ret = H5Pset_shared_mesg_phase_change(fcpl_id, H5SM_MAX_LIST_ELEMS, H5SM_MAX_LIST_ELEMS+1); + VERIFY(ret, -1, "H5Pset_shared_mesg_phase_change"); } H5E_END_TRY + /* Actually, the list max can be exactly 1 greater than the * btree min, but no more. Also, the errors above shouldn't * have corrupted the fcpl, although we do need to reset the @@ -344,6 +350,20 @@ static void test_sohm_fcpl(void) CHECK_I(ret, "H5Pset_shared_mesg_phase_change"); fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl_id, H5P_DEFAULT); CHECK_I(fid, "H5Fcreate"); + ret = H5Fclose(fid); + CHECK_I(ret, "H5Fclose"); + + /* Test edge cases; H5SM_MAX_NINDEXES and H5SM_MAX_LIST_ELEMS should be + * valid values. Also, creating a file with uninitialized indexes + * (indexes 3-5) should work. + */ + ret = H5Pset_shared_mesg_nindexes(fcpl_id, H5SM_MAX_NINDEXES); + CHECK_I(ret, "H5Pset_shared_mesg_nindexes"); + ret = H5Pset_shared_mesg_phase_change(fcpl_id, H5SM_MAX_LIST_ELEMS, H5SM_MAX_LIST_ELEMS); + CHECK_I(ret, "H5Pset_shared_mesg_phase_change"); + fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl_id, H5P_DEFAULT); + CHECK_I(fid, "H5Fcreate"); + /* Clean up */ ret = H5Pclose(fcpl_id); -- cgit v0.12