[svn-r16653] Bug fix #1503 - H5Iget_type failed unexpected when an invalid ID was passed in. I put

some argument check in the internal function H5I_find_id and took out the assertion check.
I also removed the argument check in H5Iis_valid because it's in H5I_find_id now.

Tested on jam - simple change.  Tested v1.8 already. 

2 files changed, 61 insertions, 20 deletions

diff --git a/src/H5I.c b/src/H5I.c
index c6fcb17..14dbb1f 100644
--- a/src/H5I.c
+++ b/src/H5I.c
@@ -1848,39 +1848,34 @@ done:
  * Function:	H5Iis_valid
  *
  * Purpose:	Check if the given id is valid.  And id is valid if it is in
- *          use and has an application reference count of at least 1.
+ *              use and has an application reference count of at least 1.
  *
- * Return:	Success: TRUE if the id is valid, FALSE otherwise.
+ * Return:	Success:        TRUE if the id is valid, FALSE otherwise.
  *
- *          Failure:	Negative (never fails currently)
+ *              Failure:	Negative (never fails currently)
  *
  * Programmer:  Neil Fortner
  *              Friday, October 31, 2008 (boo)
  *
+ * Modifications:
+ *              Raymond Lu
+ *              1 April 2009 (Believe it or not!)
+ *              Moved the argument check down to H5I_find_id because other
+ *              caller functions may pass in some invalid IDs to H5I_find_id.
+ *              It used to do assertion check.
  *-------------------------------------------------------------------------
  */
 htri_t
 H5Iis_valid(hid_t id)
 {
-    H5I_id_type_t   *type_ptr;          /* ptr to ID's type */
     H5I_id_info_t   *id_ptr;            /* ptr to the ID */
-    H5I_type_t      type;               /* ID's type */
     htri_t          ret_value = TRUE;   /* Return value */
 
     FUNC_ENTER_API(H5Iis_valid, FAIL)
     H5TRACE1("t", "i", id);
 
-    type = H5I_TYPE(id);
-    /* Check for conditions that would cause H5I_find_id to throw an assertion */
-    if (type <= H5I_BADID || type >= H5I_next_type)
-        HGOTO_DONE(FALSE);
-
-    type_ptr = H5I_id_type_list_g[type];
-    if (!type_ptr || type_ptr->count <= 0)
-        ret_value = FALSE;
-
     /* Find the ID */
-    else if (NULL == (id_ptr = H5I_find_id(id)))
+    if (NULL == (id_ptr = H5I_find_id(id)))
         ret_value = FALSE;
 
     /* Check if the found id is an internal id */
@@ -2012,7 +2007,9 @@ done:
  * Programmer:
  *
  * Modifications:
- *
+ *              Raymond Lu
+ *              1 April 2009 (Believe it or not!)
+ *              Added argument check, took away assertion check.
  *-------------------------------------------------------------------------
  */
 static H5I_id_info_t *
@@ -2029,10 +2026,12 @@ H5I_find_id(hid_t id)
 
     /* Check arguments */
     type = H5I_TYPE(id);
-    HDassert(type > H5I_BADID && type < H5I_next_type);
-    type_ptr = H5I_id_type_list_g[type];
+    if (type <= H5I_BADID || type >= H5I_next_type)
+        HGOTO_DONE(NULL);
 
-    HDassert(type_ptr && type_ptr->count > 0);
+    type_ptr = H5I_id_type_list_g[type];
+    if (!type_ptr || type_ptr->count <= 0)
+        HGOTO_DONE(NULL);
 
     /* Get the bucket in which the ID is located */
     hash_loc = (unsigned)H5I_LOC(id, type_ptr->hash_size);
@@ -2057,6 +2056,7 @@ H5I_find_id(hid_t id)
     /* Set the return value */
     ret_value = id_ptr;
 
+done:
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5I_find_id() */
 
diff --git a/test/tid.c b/test/tid.c
index f3c00fc..9bb93e7 100644
--- a/test/tid.c
+++ b/test/tid.c
@@ -394,7 +394,7 @@ static int test_is_valid(void)
 
     /* Check that an id of -1 is invalid */
     tri_ret = H5Iis_valid(-1);
-    VERIFY(tri_ret, FALSE, "H5Iis_valid");
+    VERIFY(tri_ret, FALSE, "H4Iis_valid");
     if (tri_ret != FALSE)
         goto out;
 
@@ -408,6 +408,46 @@ out:
     return -1;
 }
 
+/* Test the H5Iget_type function */
+static int test_get_type(void)
+{
+    hid_t   dtype;           /* datatype id */
+    H5I_type_t  type_ret;    /* return value */
+
+    /* Create a datatype id */
+    dtype = H5Tcopy(H5T_NATIVE_INT);
+    CHECK(dtype, FAIL, "H5Tcopy");
+    if (dtype < 0)
+        goto out;
+
+    /* Check that the ID is correct */
+    type_ret = H5Iget_type(dtype);
+    VERIFY(type_ret, H5I_DATATYPE, "H5Iget_type");
+    if (type_ret == H5I_BADID)
+        goto out;
+
+    /* Check that the ID is correct */
+    type_ret = H5Iget_type(H5T_STRING);
+    VERIFY(type_ret, H5I_BADID, "H5Iget_type");
+    if (type_ret != H5I_BADID)
+        goto out;
+
+    /* Check that the ID is correct */
+    type_ret = H5Iget_type(-1);
+    VERIFY(type_ret, H5I_BADID, "H5Iget_type");
+    if (type_ret != H5I_BADID)
+        goto out;
+
+    H5Tclose(dtype);
+
+    return 0;
+
+out:
+    if(dtype != H5I_INVALID_HID)
+	H5Tclose(dtype);
+
+    return -1;
+}
 
 	/* Test boundary cases with lots of types */
 
@@ -498,6 +538,7 @@ void test_ids(void)
 	if (basic_id_test() < 0) TestErrPrintf("Basic ID test failed\n");
 	if (id_predefined_test() < 0) TestErrPrintf("Predefined ID type test failed\n");
 	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");
 
 }