Merge pull request #1985 in HDFFV/hdf5 from ~DEROBINS/hdf5_der:H5Gget_object_behavior_change to develop

* commit 'ff3278f1a0d57663d73caa6aaa7f209864e9a9bd':
  Fixed a bug (HDFFV-10899) where the behavior of the deprecated H5Gget_objinfo() call had changed and passing in a non-existing soft link with a statbuf of NULL passed instead of failed (as it should as per the RM). The HDF5 1.8 and 1.10 behavior was restored and H5Gget_objinfo() will now fail and return -1 as before.

2 files changed, 88 insertions, 80 deletions

diff --git a/src/H5Gdeprec.c b/src/H5Gdeprec.c
index 97a3ccf..fcb16d3 100644
--- a/src/H5Gdeprec.c
+++ b/src/H5Gdeprec.c
@@ -926,41 +926,38 @@ herr_t
 H5Gget_objinfo(hid_t loc_id, const char *name, hbool_t follow_link,
     H5G_stat_t *statbuf/*out*/)
 {
-    herr_t ret_value = SUCCEED;         /* Return value */
+    H5VL_object_t      *vol_obj = NULL;         /* Object token of loc_id */
+    H5VL_loc_params_t   loc_params;
+    herr_t              ret_value = SUCCEED;    /* Return value */
 
-    FUNC_ENTER_API(FAIL)
+    FUNC_ENTER_API(FAIL);
     H5TRACE4("e", "i*sbx", loc_id, name, follow_link, statbuf);
 
     /* Check arguments */
-    if(!name || !*name)
+    if (!name || !*name)
         HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "no name specified")
 
     /* Set up collective metadata if appropriate */
-    if(H5CX_set_loc(loc_id) < 0)
-        HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set collective metadata read info")
+    if (H5CX_set_loc(loc_id) < 0)
+        HGOTO_ERROR(H5E_SYM, H5E_CANTSET, FAIL, "can't set collective metadata read info");
 
-    /* Retrieve object info, if pointer to struct is given */
-    if(statbuf) {
-        H5VL_object_t      *vol_obj;        /* Object token of loc_id */
-        H5VL_loc_params_t   loc_params;
-
-        /* Fill out location struct */
-        loc_params.type                         = H5VL_OBJECT_BY_NAME;
-        loc_params.loc_data.loc_by_name.name    = name;
-        loc_params.loc_data.loc_by_name.lapl_id = H5P_LINK_ACCESS_DEFAULT;
-        loc_params.obj_type                     = H5I_get_type(loc_id);
+    /* Retrieve object info */
+    /* Fill out location struct */
+    loc_params.type                         = H5VL_OBJECT_BY_NAME;
+    loc_params.loc_data.loc_by_name.name    = name;
+    loc_params.loc_data.loc_by_name.lapl_id = H5P_LINK_ACCESS_DEFAULT;
+    loc_params.obj_type                     = H5I_get_type(loc_id);
 
-        /* Get the location object */
-        if(NULL == (vol_obj = H5VL_vol_object(loc_id)))
-            HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier")
+    /* Get the location object */
+    if (NULL == (vol_obj = H5VL_vol_object(loc_id)))
+        HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "invalid location identifier");
 
-        /* Retrieve the object's information */
-        if(H5VL_group_optional(vol_obj, H5P_DATASET_XFER_DEFAULT, H5_REQUEST_NULL, H5VL_NATIVE_GROUP_GET_OBJINFO, &loc_params, (unsigned)follow_link, statbuf) < 0)
-            HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "can't get info for object: '%s'", name)
-    } /* end if */
+    /* Retrieve the object's information */
+    if (H5VL_group_optional(vol_obj, H5P_DATASET_XFER_DEFAULT, H5_REQUEST_NULL, H5VL_NATIVE_GROUP_GET_OBJINFO, &loc_params, (unsigned)follow_link, statbuf) < 0)
+        HGOTO_ERROR(H5E_SYM, H5E_CANTGET, FAIL, "can't get info for object: '%s'", name);
 
 done:
-    FUNC_LEAVE_API(ret_value)
+    FUNC_LEAVE_API(ret_value);
 } /* end H5Gget_objinfo() */
 
 
@@ -982,60 +979,64 @@ H5G__get_objinfo_cb(H5G_loc_t H5_ATTR_UNUSED *grp_loc/*in*/, const char *name, c
     H5G_loc_t *obj_loc, void *_udata/*in,out*/, H5G_own_loc_t *own_loc/*out*/)
 {
     H5G_trav_goi_t *udata = (H5G_trav_goi_t *)_udata;   /* User data passed in */
-    H5G_stat_t *statbuf = udata->statbuf;   /* Convenience pointer for statbuf */
     herr_t ret_value = SUCCEED;         /* Return value */
 
-    FUNC_ENTER_STATIC
+    FUNC_ENTER_STATIC;
 
     /* Check if the name in this group resolved to a valid link */
-    if(lnk == NULL && obj_loc == NULL)
-        HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "'%s' doesn't exist", name)
-
-    /* Common code to retrieve the file's fileno */
-    if(H5F_get_fileno((obj_loc ? obj_loc : grp_loc)->oloc->file, &statbuf->fileno[0]) < 0)
-        HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "unable to read fileno")
-
-    /* Info for soft and UD links is gotten by H5L_get_info. If we have
-     *      a hard link, follow it and get info on the object
-     */
-    if(udata->follow_link || !lnk || (lnk->type == H5L_TYPE_HARD)) {
-        H5O_info_t oinfo;           /* Object information */
-
-        /* Go retrieve the object information */
-        /* (don't need index & heap info) */
-        HDassert(obj_loc);
-        if(H5O_get_info(obj_loc->oloc, &oinfo, H5O_INFO_BASIC|H5O_INFO_TIME|H5O_INFO_HDR) < 0)
-            HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to get object info")
-
-        /* Get mapped object type */
-        statbuf->type = H5G_map_obj_type(oinfo.type);
-
-        /* Get object number (i.e. address) for object */
-        statbuf->objno[0] = (unsigned long)(oinfo.addr);
+    if (lnk == NULL && obj_loc == NULL)
+        HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "'%s' doesn't exist", name);
+
+    /* Only modify user's buffer if it's available */                            
+    if (udata->statbuf) {                                                         
+        H5G_stat_t *statbuf = udata->statbuf;   /* Convenience pointer for statbuf */
+
+        /* Common code to retrieve the file's fileno */
+        if (H5F_get_fileno((obj_loc ? obj_loc : grp_loc)->oloc->file, &statbuf->fileno[0]) < 0)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, FAIL, "unable to read fileno");
+
+        /* Info for soft and UD links is gotten by H5L_get_info. If we have
+         *      a hard link, follow it and get info on the object
+         */
+        if (udata->follow_link || !lnk || (lnk->type == H5L_TYPE_HARD)) {
+            H5O_info_t oinfo;           /* Object information */
+
+            /* Go retrieve the object information */
+            /* (don't need index & heap info) */
+            HDassert(obj_loc);
+            if (H5O_get_info(obj_loc->oloc, &oinfo, H5O_INFO_BASIC|H5O_INFO_TIME|H5O_INFO_HDR) < 0)
+                HGOTO_ERROR(H5E_OHDR, H5E_CANTGET, FAIL, "unable to get object info");
+
+            /* Get mapped object type */
+            statbuf->type = H5G_map_obj_type(oinfo.type);
+
+            /* Get object number (i.e. address) for object */
+            statbuf->objno[0] = (unsigned long)(oinfo.addr);
 #if H5_SIZEOF_UINT64_T > H5_SIZEOF_LONG
-        statbuf->objno[1] = (unsigned long)(oinfo.addr >> 8 * sizeof(long));
+            statbuf->objno[1] = (unsigned long)(oinfo.addr >> 8 * sizeof(long));
 #else
-        statbuf->objno[1] = 0;
+            statbuf->objno[1] = 0;
 #endif
-        /* Get # of hard links pointing to object */
-        statbuf->nlink = oinfo.rc;
-
-        /* Get modification time for object */
-        statbuf->mtime = oinfo.ctime;
-
-        /* Retrieve the object header information */
-        statbuf->ohdr.size = oinfo.hdr.space.total;
-        statbuf->ohdr.free = oinfo.hdr.space.free;
-        statbuf->ohdr.nmesgs = oinfo.hdr.nmesgs;
-        statbuf->ohdr.nchunks = oinfo.hdr.nchunks;
-    } /* end if */
+            /* Get # of hard links pointing to object */
+            statbuf->nlink = oinfo.rc;
+
+            /* Get modification time for object */
+            statbuf->mtime = oinfo.ctime;
+
+            /* Retrieve the object header information */
+            statbuf->ohdr.size = oinfo.hdr.space.total;
+            statbuf->ohdr.free = oinfo.hdr.space.free;
+            statbuf->ohdr.nmesgs = oinfo.hdr.nmesgs;
+            statbuf->ohdr.nchunks = oinfo.hdr.nchunks;
+        }
+    }
 
 done:
     /* Indicate that this callback didn't take ownership of the group *
      * location for the object */
     *own_loc = H5G_OWN_NONE;
 
-    FUNC_LEAVE_NOAPI(ret_value)
+    FUNC_LEAVE_NOAPI(ret_value);
 } /* end H5G__get_objinfo_cb() */
 
 
@@ -1062,15 +1063,15 @@ H5G__get_objinfo(const H5G_loc_t *loc, const char *name, hbool_t follow_link,
     H5G_trav_goi_t udata;           /* User data for callback */
     herr_t      ret_value = SUCCEED;       /* Return value */
 
-    FUNC_ENTER_PACKAGE
+    FUNC_ENTER_PACKAGE;
 
     /* Sanity checks */
     HDassert(loc);
     HDassert(name && *name);
-    HDassert(statbuf);
 
     /* Reset stat buffer */
-    HDmemset(statbuf, 0, sizeof(H5G_stat_t));
+    if (statbuf)
+        HDmemset(statbuf, 0, sizeof(H5G_stat_t));
 
     /* Set up user data for retrieving information */
     udata.statbuf = statbuf;
@@ -1078,11 +1079,11 @@ H5G__get_objinfo(const H5G_loc_t *loc, const char *name, hbool_t follow_link,
     udata.loc_file = loc->oloc->file;
 
     /* Traverse the group hierarchy to locate the object to get info about */
-    if(H5G_traverse(loc, name, (unsigned)(follow_link ? H5G_TARGET_NORMAL : (H5G_TARGET_SLINK | H5G_TARGET_UDLINK)), H5G__get_objinfo_cb, &udata) < 0)
-        HGOTO_ERROR(H5E_SYM, H5E_EXISTS, FAIL, "name doesn't exist")
+    if (H5G_traverse(loc, name, (unsigned)(follow_link ? H5G_TARGET_NORMAL : (H5G_TARGET_SLINK | H5G_TARGET_UDLINK)), H5G__get_objinfo_cb, &udata) < 0)
+        HGOTO_ERROR(H5E_SYM, H5E_EXISTS, FAIL, "name doesn't exist");
 
     /* If we're pointing at a soft or UD link, get the real link length and type */
-    if(follow_link == 0) {
+    if (statbuf && follow_link == 0) {
         H5L_info_t linfo;           /* Link information buffer */
         herr_t ret;
 
@@ -1090,21 +1091,23 @@ H5G__get_objinfo(const H5G_loc_t *loc, const char *name, hbool_t follow_link,
          * because the object is ".", just treat the object as a hard link. */
         H5E_BEGIN_TRY {
             ret = H5L_get_info(loc, name, &linfo);
-        } H5E_END_TRY
+        } H5E_END_TRY;
 
-        if(ret >= 0 && linfo.type != H5L_TYPE_HARD) {
+        if (ret >= 0 && linfo.type != H5L_TYPE_HARD) {
             statbuf->linklen = linfo.u.val_size;
-            if(linfo.type == H5L_TYPE_SOFT)
+            if (linfo.type == H5L_TYPE_SOFT) {
                 statbuf->type = H5G_LINK;
-            else {  /* UD link. H5L_get_info checked for invalid link classes */
+            }
+            else {
+                /* UD link. H5L_get_info checked for invalid link classes */
                 HDassert(linfo.type >= H5L_TYPE_UD_MIN && linfo.type <= H5L_TYPE_MAX);
                 statbuf->type = H5G_UDLINK;
-            } /* end else */
-        } /* end if */
-    } /* end if */
+            }
+        }
+    }
 
 done:
-    FUNC_LEAVE_NOAPI(ret_value)
+    FUNC_LEAVE_NOAPI(ret_value);
 } /* end H5G__get_objinfo() */
 
 
diff --git a/test/links.c b/test/links.c
index 4eb4126..916ad88 100644
--- a/test/links.c
+++ b/test/links.c
@@ -1576,6 +1576,10 @@ test_deprec(hid_t fapl, hbool_t new_format)
     if(H5Gget_linkval(group2_id, "soft_link_to_group1", sb_soft1.linklen, tmpstr) < 0) FAIL_STACK_ERROR
     if(HDstrcmp("link_to_group1", tmpstr)) TEST_ERROR
 
+    /* Test non-existing links with H5Gget_objinfo */
+    H5E_BEGIN_TRY {
+        if(H5Gget_objinfo(file_id, "/group2/soft_link_no_exist", TRUE, NULL) >= 0) FAIL_STACK_ERROR
+    } H5E_END_TRY;
 
     /* Test the dangling soft link */
     if(H5Gget_objinfo(file_id, "/group2/dangle_soft_link", FALSE, &sb_soft2) < 0) FAIL_STACK_ERROR
@@ -14103,7 +14107,8 @@ main(void)
         nerrors += group_info_old(fapl) < 0 ? 1 : 0;
 
         if (minimize_dset_oh) {
-            if (H5Pclose(dcpl_g) < 0)  TEST_ERROR;
+            if (H5Pclose(dcpl_g) < 0)
+                TEST_ERROR;
             dcpl_g = -1;
         }
     } /* [un]minimized dataset object headers */
@@ -14141,5 +14146,5 @@ main(void)
 error:
     HDputs("*** TESTS FAILED ***");
     HDexit(EXIT_FAILURE);
-}
+} /* end main() */