[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

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");
 }