[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.
This commit is contained in:
James Laird 2007-01-03 12:48:51 -05:00
parent b5a2e7c634
commit 11af96e867
9 changed files with 68 additions and 46 deletions

View File

@ -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 */

View File

@ -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)

View File

@ -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 */
/*******************/

View File

@ -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.

View File

@ -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)))

View File

@ -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

View File

@ -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

View File

@ -29,8 +29,6 @@
/* Library Private Typedefs */
/****************************/
#define H5SM_MAX_NUM_INDEXES 6
/******************************/
/* Library Private Prototypes */
/******************************/

View File

@ -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);