From 27a7a56496e10cb77322fa2eb2b4fe88e957baa6 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Tue, 7 Oct 2014 15:31:39 -0500 Subject: [svn-r25681] Description: Correct errors in ID wrapping code, add some small optimizations, and make certain that all IDs of a given class can be used, especially when wrapping around. Also, enable ID reuse for error classes/values, property lists and datatypes. Includes regression testing for ID wrapping. Note that this change does not apply to the trunk, since that ID manager code is using 64-bit IDs and doesn't worry about wrapping around. Tested on: MacOSX/64 10.9.5 (amazon) w/debug, C++, FORTRAN & parallel --- src/H5E.c | 6 ++-- src/H5I.c | 95 +++++++++++++++++++++++++++++++-------------------------- src/H5Pint.c | 2 +- src/H5T.c | 2 +- test/tid.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 156 insertions(+), 48 deletions(-) diff --git a/src/H5E.c b/src/H5E.c index 18e1e33..3fe2a83 100644 --- a/src/H5E.c +++ b/src/H5E.c @@ -122,7 +122,7 @@ H5FL_DEFINE_STATIC(H5E_msg_t); /* Error class ID class */ static const H5I_class_t H5I_ERRCLS_CLS[1] = {{ H5I_ERROR_CLASS, /* ID class value */ - 0, /* Class flags */ + H5I_CLASS_REUSE_IDS, /* Class flags */ 0, /* # of reserved IDs for class */ (H5I_free_t)H5E_unregister_class /* Callback routine for closing objects of this class */ }}; @@ -130,7 +130,7 @@ static const H5I_class_t H5I_ERRCLS_CLS[1] = {{ /* Error message ID class */ static const H5I_class_t H5I_ERRMSG_CLS[1] = {{ H5I_ERROR_MSG, /* ID class value */ - 0, /* Class flags */ + H5I_CLASS_REUSE_IDS, /* Class flags */ 0, /* # of reserved IDs for class */ (H5I_free_t)H5E_close_msg /* Callback routine for closing objects of this class */ }}; @@ -138,7 +138,7 @@ static const H5I_class_t H5I_ERRMSG_CLS[1] = {{ /* Error stack ID class */ static const H5I_class_t H5I_ERRSTK_CLS[1] = {{ H5I_ERROR_STACK, /* ID class value */ - 0, /* Class flags */ + H5I_CLASS_REUSE_IDS, /* Class flags */ 0, /* # of reserved IDs for class */ (H5I_free_t)H5E_close_stack /* Callback routine for closing objects of this class */ }}; diff --git a/src/H5I.c b/src/H5I.c index 28cd32e..38c1100 100644 --- a/src/H5I.c +++ b/src/H5I.c @@ -814,7 +814,7 @@ H5I__wrapped_cb(void *_item, void UNUSED *_key, void *_udata) HDassert(udata); /* Break out if we see a free ID */ - if(udata->nextid != item->id) { + if(udata->nextid != (ID_MASK & item->id)) { /* Sanity check */ HDassert(item->id > udata->nextid); @@ -874,6 +874,40 @@ H5I_register(H5I_type_t type, const void *object, hbool_t app_ref) /* If no available ID structure, then create a new id for use, and * allocate a new struct to house it. */ else { + /* + * This next section of code checks for the 'nextid' getting too large and + * wrapping around, thus necessitating checking for duplicate IDs being + * handed out. + */ + if(type_ptr->nextid > (hid_t)ID_MASK) + type_ptr->wrapped = TRUE; + + /* + * If we've wrapped around then we need to check for duplicate id's being + * handed out. + */ + if(type_ptr->wrapped) { + H5I_wrap_ud_t udata; /* User data for iteration */ + herr_t iter_status; /* Iteration status */ + + /* Set up user data for iteration */ + udata.nextid = (hid_t)type_ptr->cls->reserved; + + /* Iterate over all the ID nodes, looking for a gap in the ID sequence */ + if((iter_status = H5SL_iterate(type_ptr->ids, H5I__wrapped_cb, &udata)) < 0) + HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "ID iteration failed") + + /* If we didn't break out of the iteration and we're at the max. ID, we've used all the IDs */ + if(0 == iter_status && udata.nextid >= ID_MASK) + HGOTO_ERROR(H5E_ATOM, H5E_NOIDS, FAIL, "no IDs available in type") + + /* Sanity check */ + HDassert(udata.nextid < ID_MASK); + + /* Retain the next ID for the class */ + type_ptr->nextid = udata.nextid; + } /* end if */ + /* Allocate new ID struct */ if(NULL == (id_ptr = H5FL_MALLOC(H5I_id_info_t))) HGOTO_ERROR(H5E_ATOM, H5E_NOSPACE, FAIL, "memory allocation failed") @@ -895,40 +929,6 @@ H5I_register(H5I_type_t type, const void *object, hbool_t app_ref) HGOTO_ERROR(H5E_ATOM, H5E_CANTINSERT, FAIL, "can't insert ID node into skip list") type_ptr->id_count++; - /* - * This next section of code checks for the 'nextid' getting too large and - * wrapping around, thus necessitating checking for duplicate IDs being - * handed out. - */ - if(type_ptr->nextid > (hid_t)ID_MASK) - type_ptr->wrapped = TRUE; - - /* - * If we've wrapped around then we need to check for duplicate id's being - * handed out. - */ - if(type_ptr->wrapped) { - H5I_wrap_ud_t udata; /* User data for iteration */ - herr_t iter_status; /* Iteration status */ - - /* Set up user data for iteration */ - udata.nextid = (hid_t)type_ptr->cls->reserved; - - /* Iterate over all the ID nodes, looking for a gap in the ID sequence */ - if((iter_status = H5SL_iterate(type_ptr->ids, H5I__wrapped_cb, &udata)) < 0) - HGOTO_ERROR(H5E_ATOM, H5E_BADITER, FAIL, "ID iteration failed") - - /* If we didn't break out of the iteration and we're at the max. ID, we've used all the IDs */ - if(0 == iter_status && udata.nextid >= ID_MASK) - HGOTO_ERROR(H5E_ATOM, H5E_NOIDS, FAIL, "no IDs available in type") - - /* Sanity check */ - HDassert(udata.nextid < ID_MASK); - - /* Retain the next ID for the class */ - type_ptr->nextid = udata.nextid; - } /* end if */ - /* Set return value */ ret_value = id_ptr->id; @@ -1249,13 +1249,24 @@ H5I__remove_common(H5I_id_type_t *type_ptr, hid_t id) /* (Casting away const OK -QAK) */ ret_value = (void *)curr_id->obj_ptr; - /* If there's room, and we can save IDs of this type, then - save the struct (and its ID) for future re-use */ - if((type_ptr->cls->flags & H5I_CLASS_REUSE_IDS) - && (type_ptr->avail_count < MAX_FREE_ID_STRUCTS)) { - if(H5SL_insert(type_ptr->avail_ids, curr_id, &curr_id->id) < 0) - HGOTO_ERROR(H5E_ATOM, H5E_CANTINSERT, NULL, "can't insert available ID node into skip list") - type_ptr->avail_count++; + /* See if we can reuse IDs of this type */ + if(type_ptr->cls->flags & H5I_CLASS_REUSE_IDS) { + /* See if we can decrement the next ID for the ID class */ + if(type_ptr->nextid == (ID_MASK & (curr_id->id + 1))) { + type_ptr->nextid--; + curr_id = H5FL_FREE(H5I_id_info_t, curr_id); + } /* end if */ + else { + /* Store the ID on the available ID list, for later */ + if((type_ptr->avail_count < MAX_FREE_ID_STRUCTS) + && (type_ptr->id_count > 1)) { + if(H5SL_insert(type_ptr->avail_ids, curr_id, &curr_id->id) < 0) + HGOTO_ERROR(H5E_ATOM, H5E_CANTINSERT, NULL, "can't insert available ID node into skip list") + type_ptr->avail_count++; + } + else + curr_id = H5FL_FREE(H5I_id_info_t, curr_id); + } /* end else */ } /* end if */ /* Otherwise, just toss it. */ else diff --git a/src/H5Pint.c b/src/H5Pint.c index d6ca6bc..8bc0b6a 100644 --- a/src/H5Pint.c +++ b/src/H5Pint.c @@ -305,7 +305,7 @@ static const H5I_class_t H5I_GENPROPCLS_CLS[1] = {{ /* Generic Property List ID class */ static const H5I_class_t H5I_GENPROPLST_CLS[1] = {{ H5I_GENPROP_LST, /* ID class value */ - 0, /* Class flags */ + H5I_CLASS_REUSE_IDS, /* Class flags */ 0, /* # of reserved IDs for class */ (H5I_free_t)H5P_close /* Callback routine for closing objects of this class */ }}; diff --git a/src/H5T.c b/src/H5T.c index 41ec1a7..942174c 100644 --- a/src/H5T.c +++ b/src/H5T.c @@ -529,7 +529,7 @@ H5FL_DEFINE_STATIC(H5T_path_t); /* Datatype ID class */ static const H5I_class_t H5I_DATATYPE_CLS[1] = {{ H5I_DATATYPE, /* ID class value */ - 0, /* Class flags */ + H5I_CLASS_REUSE_IDS, /* Class flags */ 8, /* # of reserved IDs for class */ (H5I_free_t)H5T_close /* Callback routine for closing objects of this class */ }}; diff --git a/test/tid.c b/test/tid.c index a9c2d24..7b4f5c1 100644 --- a/test/tid.c +++ b/test/tid.c @@ -533,6 +533,103 @@ out: return -1; } +/* 'Fake' free routine for ID wrapping test */ +static herr_t fake_free(void *obj) +{ + /* Shut compilers up */ + obj = obj; + + return(0); +} + + /* Test boundary cases with lots of IDs */ + +/* Type IDs range from 0 to ID_MASK before wrapping around. The code will assign */ +/* IDs in sequential order until ID_MASK IDs have been given out, at which */ +/* point it will search for type IDs that were allocated but have since been */ +/* closed. */ +/* This test will allocate IDs up to ID_MASK, ensure that IDs wrap around */ +/* to low values successfully, ensure that an error is thrown when all possible */ +/* type IDs are taken, then ensure that deleting types frees up their IDs. */ +/* NOTE: this test depends on the implementation of IDs, so may break */ +/* if the implementation changes. */ +static int test_id_wrap(void) +{ + H5I_type_t testType; /* ID class for testing */ + hid_t *id_array; /* Array of IDs allocated */ + hid_t test_id; /* Test ID */ + void *obj; /* Object pointer returned for ID */ + unsigned u; /* Local index variable */ + herr_t status; /* Status from routine */ + + /* Allocate array for storing IDs */ + id_array = (hid_t *)HDmalloc((ID_MASK + 1) * sizeof(hid_t)); + CHECK(id_array, NULL, "HDmalloc"); + + /* Register type for testing */ + testType = H5Iregister_type((size_t)8, 0, (H5I_free_t)fake_free); + CHECK(testType, H5I_BADID, "H5Iregister_type"); + if(testType == H5I_BADID) + goto out; + + /* Get IDs, up to the maximum possible */ + for(u = 0; u <= ID_MASK; u++) { + id_array[u] = H5Iregister(testType, &id_array[u]); + CHECK(id_array[u], FAIL, "H5Iregister"); + if(id_array[u] < 0) + goto out; + } /* end for */ + + /* There should be no room at the inn for a new ID */ + H5E_BEGIN_TRY + test_id = H5Iregister(testType, id_array); + H5E_END_TRY + VERIFY(test_id, H5I_BADID, "H5Iregister_type"); + if(test_id != H5I_BADID) + goto out; + + /* Release the first ID in the array */ + obj = H5Iremove_verify(id_array[0], testType); + CHECK(obj, NULL, "H5Iremove_verify"); + if(NULL == obj) + goto out; + VERIFY(obj, &id_array[0], "H5Iremove_verify"); + if(&id_array[0] != obj) + goto out; + + /* Register another object, should be room now, but will wraparound */ + test_id = H5Iregister(testType, &id_array[0]); + CHECK(test_id, FAIL, "H5Iregister"); + if(test_id < 0) + goto out; + VERIFY(test_id, id_array[0], "H5Iregister"); + if(id_array[0] != test_id) + goto out; + + /* Release all IDs, unregister the ID class and free the array */ + for(u = 0; u <= ID_MASK; u++) { + obj = H5Iremove_verify(id_array[u], testType); + CHECK(obj, NULL, "H5Iremove_verify"); + if(NULL == obj) + goto out; + VERIFY(obj, &id_array[u], "H5Iremove_verify"); + if(&id_array[u] != obj) + goto out; + } /* end for */ + + status = H5Idestroy_type(testType); + CHECK(status, FAIL, "H5Idestroy_type"); + if(status < 0) + goto out; + + HDfree(id_array); + + return(0); + +out: + return(-1); +} + void test_ids(void) { if (basic_id_test() < 0) TestErrPrintf("Basic ID test failed\n"); @@ -540,5 +637,5 @@ void test_ids(void) if (test_is_valid() < 0) TestErrPrintf("H5Iis_valid test failed\n"); if (test_get_type() < 0) TestErrPrintf("H5Iget_type test failed\n"); if (test_id_type_list() < 0) TestErrPrintf("ID type list test failed\n"); - + if (test_id_wrap() < 0) TestErrPrintf("ID wraparound test failed\n"); } -- cgit v0.12