From 5d4294042e459b998d642c33d6233a6912f67c77 Mon Sep 17 00:00:00 2001 From: Raymond Lu Date: Wed, 1 Apr 2009 17:27:35 -0500 Subject: [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. --- src/H5I.c | 38 +++++++++++++++++++------------------- test/tid.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 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"); } -- cgit v0.12