Merge pull request #1386 in HDFFV/hdf5 from ~JHENDERSON/hdf5:develop to develop

* commit 'b4fe787bb9fe4bfc4709a124df59dd987c9a09d2':
  Add test for H5Arename NULL/empty attribute name fix
  align H5Arename behavior with H5Arename_by_name

2 files changed, 104 insertions, 3 deletions

diff --git a/src/H5A.c b/src/H5A.c
index 8ebd2b3..1b804d9 100644
--- a/src/H5A.c
+++ b/src/H5A.c
@@ -1201,10 +1201,16 @@ H5Arename(hid_t loc_id, const char *old_name, const char *new_name)
     H5TRACE3("e", "i*s*s", loc_id, old_name, new_name);
 
     /* check arguments */
-    if(!old_name || !new_name)
-        HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "name is nil")
     if(H5I_ATTR == H5I_get_type(loc_id))
         HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "location is not valid for an attribute")
+    if(!old_name)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "old attribute name cannot be NULL")
+    if(!*old_name)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "old attribute name cannot be an empty string")
+    if(!new_name)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "new attribute name cannot be NULL")
+    if(!*new_name)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "new attribute name cannot be an empty string")
 
     /* Avoid thrashing things if the names are the same */
     if(HDstrcmp(old_name, new_name)) {
diff --git a/test/tattr.c b/test/tattr.c
index 63a0580..9d80050 100644
--- a/test/tattr.c
+++ b/test/tattr.c
@@ -146,6 +146,10 @@ float attr_data5=-5.123F;        /* Test data for 5th attribute */
 /* Used by test_attr_info_null_info_pointer() */
 #define GET_INFO_NULL_POINTER_ATTR_NAME "NullInfoPointerAttr"
 
+/* Used by test_attr_rename_invalid_name() */
+#define INVALID_RENAME_TEST_ATTR_NAME "InvalidRenameTestAttr"
+#define INVALID_RENAME_TEST_NEW_ATTR_NAME "InvalidRenameTestNewAttr"
+
 
 /* Attribute iteration struct */
 typedef struct {
@@ -5894,7 +5898,7 @@ test_attr_info_null_info_pointer(hid_t fcpl, hid_t fapl)
     hid_t  attr;
     hid_t  sid;
 
-    /* Create dataspace for dataset & attributes */
+    /* Create dataspace for attribute */
     sid = H5Screate(H5S_SCALAR);
     CHECK(sid, FAIL, "H5Screate");
 
@@ -5938,6 +5942,95 @@ test_attr_info_null_info_pointer(hid_t fcpl, hid_t fapl)
 }
 
 
+/***************************************************************
+**
+**  test_attr_rename_invalid_name(): A test to ensure that
+**      passing a NULL or empty attribute name to
+**      H5Arename(_by_name) doesn't cause bad behavior.
+**
+****************************************************************/
+static void
+test_attr_rename_invalid_name(hid_t fcpl, hid_t fapl)
+{
+    herr_t err_ret = -1;
+    hid_t  fid;
+    hid_t  attr;
+    hid_t  sid;
+
+    /* Create dataspace for attribute */
+    sid = H5Screate(H5S_SCALAR);
+    CHECK(sid, FAIL, "H5Screate");
+
+    /* Create file */
+    fid = H5Fcreate(FILENAME, H5F_ACC_TRUNC, fcpl, fapl);
+    CHECK(fid, FAIL, "H5Fcreate");
+
+    /* Create attribute */
+    attr = H5Acreate2(fid, INVALID_RENAME_TEST_ATTR_NAME, H5T_NATIVE_UINT, sid, H5P_DEFAULT, H5P_DEFAULT);
+    CHECK(attr, FAIL, "H5Acreate2");
+
+    H5E_BEGIN_TRY {
+        err_ret = H5Arename(fid, NULL, INVALID_RENAME_TEST_NEW_ATTR_NAME);
+    } H5E_END_TRY;
+
+    CHECK(err_ret, SUCCEED, "H5Arename");
+
+    H5E_BEGIN_TRY {
+        err_ret = H5Arename(fid, "", INVALID_RENAME_TEST_NEW_ATTR_NAME);
+    } H5E_END_TRY;
+
+    CHECK(err_ret, SUCCEED, "H5Arename");
+
+    H5E_BEGIN_TRY {
+        err_ret = H5Arename(fid, INVALID_RENAME_TEST_ATTR_NAME, NULL);
+    } H5E_END_TRY;
+
+    CHECK(err_ret, SUCCEED, "H5Arename");
+
+    H5E_BEGIN_TRY {
+        err_ret = H5Arename(fid, INVALID_RENAME_TEST_ATTR_NAME, "");
+    } H5E_END_TRY;
+
+    CHECK(err_ret, SUCCEED, "H5Arename");
+
+    H5E_BEGIN_TRY {
+        err_ret = H5Arename_by_name(fid, ".", NULL, INVALID_RENAME_TEST_NEW_ATTR_NAME, H5P_DEFAULT);
+    } H5E_END_TRY;
+
+    CHECK(err_ret, SUCCEED, "H5Arename_by_name");
+
+    H5E_BEGIN_TRY {
+        err_ret = H5Arename_by_name(fid, ".", "", INVALID_RENAME_TEST_NEW_ATTR_NAME, H5P_DEFAULT);
+    } H5E_END_TRY;
+
+    CHECK(err_ret, SUCCEED, "H5Arename_by_name");
+
+    H5E_BEGIN_TRY {
+        err_ret = H5Arename_by_name(fid, ".", INVALID_RENAME_TEST_ATTR_NAME, NULL, H5P_DEFAULT);
+    } H5E_END_TRY;
+
+    CHECK(err_ret, SUCCEED, "H5Arename_by_name");
+
+    H5E_BEGIN_TRY {
+        err_ret = H5Arename_by_name(fid, ".", INVALID_RENAME_TEST_ATTR_NAME, "", H5P_DEFAULT);
+    } H5E_END_TRY;
+
+    CHECK(err_ret, SUCCEED, "H5Arename_by_name");
+
+    /* Close dataspace */
+    err_ret = H5Sclose(sid);
+    CHECK(err_ret, FAIL, "H5Sclose");
+
+    /* Close attribute */
+    err_ret = H5Aclose(attr);
+    CHECK(err_ret, FAIL, "H5Aclose");
+
+    /* Close file */
+    err_ret = H5Fclose(fid);
+    CHECK(err_ret, FAIL, "H5Fclose");
+}
+
+
 /****************************************************************
 **
 **  test_attr_delete_by_idx(): Test basic H5A (attribute) code.
@@ -10939,6 +11032,7 @@ test_attr(void)
                 test_attr_deprec(fcpl, my_fapl);                /* Test deprecated API routines */
                 test_attr_many(new_format, my_fcpl, my_fapl);               /* Test storing lots of attributes */
                 test_attr_info_null_info_pointer(my_fcpl, my_fapl); /* Test passing a NULL attribute info pointer to H5Aget_info(_by_name/_by_idx) */
+                test_attr_rename_invalid_name(my_fcpl, my_fapl); /* Test passing a NULL or empty attribute name to H5Arename(_by_name) */
 
                 /* Attribute creation order tests */
                 test_attr_corder_create_basic(my_fcpl, my_fapl);/* Test creating an object w/attribute creation order info */
@@ -10984,6 +11078,7 @@ test_attr(void)
             test_attr_deprec(fcpl, my_fapl);                    /* Test deprecated API routines */
             test_attr_many(new_format, fcpl, my_fapl);               /* Test storing lots of attributes */
             test_attr_info_null_info_pointer(fcpl, my_fapl); /* Test passing a NULL attribute info pointer to H5Aget_info(_by_name/_by_idx) */
+            test_attr_rename_invalid_name(fcpl, my_fapl); /* Test passing a NULL or empty attribute name to H5Arename(_by_name) */
 
             /* New attribute API routine tests, on old-format storage */
             test_attr_info_by_idx(new_format, fcpl, my_fapl);   /* Test querying attribute info by index */