[svn-r12831] Changed the API for "unpacking" an external link value to take the size of

the linkval buffer, per Elena and Frank's suggestions while revising
the documentation.  Added error checking using this size, as well as a
couple of tests.

Tested on juniper, kagiso, and sol.

8 files changed, 56 insertions, 25 deletions

diff --git a/examples/h5_elink_unix2win.c b/examples/h5_elink_unix2win.c
index b354661..7266347 100644
--- a/examples/h5_elink_unix2win.c
+++ b/examples/h5_elink_unix2win.c
@@ -54,7 +54,7 @@ static hid_t elink_unix2win_trav(const char *link_name, hid_t cur_group, void *
 
     printf("Converting Unix path to Windows path.\n");
 
-    if(H5Lunpack_elink_val(udata, &file_name, &obj_name) < 0)
+    if(H5Lunpack_elink_val(udata, udata_size, &file_name, &obj_name) < 0)
         goto error;
     fname_len = strlen(file_name);
 
diff --git a/src/H5L.c b/src/H5L.c
index 3cc46c7..24d5211 100644
--- a/src/H5L.c
+++ b/src/H5L.c
@@ -62,7 +62,7 @@ typedef struct {
 /* User data for path traversal routine for getting soft link value */
 typedef struct {
     size_t size;                              /* Size of user buffer */
-    char *buf;                                /* User buffer */
+    void *buf;                                /* User buffer */
 } H5L_trav_ud5_t;
 
 /* User data for path traversal routine for removing link (i.e. unlink) */
@@ -103,7 +103,7 @@ static herr_t H5L_linkval_cb(H5G_loc_t *grp_loc/*in*/, const char *name,
     const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/,
     H5G_own_loc_t *own_loc/*out*/);
 static herr_t H5L_linkval(H5G_loc_t *loc, const char *name, size_t size,
-    char *buf/*out*/, hid_t lapl_id, hid_t dxpl_id);
+    void *buf/*out*/, hid_t lapl_id, hid_t dxpl_id);
 static herr_t H5L_unlink_cb(H5G_loc_t *grp_loc/*in*/, const char *name,
     const H5O_link_t *lnk, H5G_loc_t *obj_loc, void *_udata/*in,out*/,
     H5G_own_loc_t *own_loc/*out*/);
@@ -780,7 +780,7 @@ done:
  *-------------------------------------------------------------------------
  */
 herr_t
-H5Lget_linkval(hid_t loc_id, const char *name, size_t size, char *buf/*out*/,
+H5Lget_linkval(hid_t loc_id, const char *name, size_t size, void *buf/*out*/,
                hid_t lapl_id)
 {
     H5G_loc_t	loc;
@@ -1496,13 +1496,15 @@ H5L_linkval_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const H
         if(udata->size > 0 && udata->buf) {
             HDstrncpy(udata->buf, lnk->u.soft.name, udata->size);
             if(HDstrlen(lnk->u.soft.name) >= udata->size)
-                udata->buf[udata->size - 1] = '\0';
+                ((char *) udata->buf)[udata->size - 1] = '\0';
         } /* end if */
     }
     else if(lnk->type >= H5L_TYPE_UD_MIN)
     {
-        /* Get the link class for this type of link.  It's okay if the class isn't registered, though--we
-        * just can't give any more information about it */
+        /* Get the link class for this type of link.  It's okay if the class 
+         * isn't registered, though--we just can't give any more information
+         * about it
+         */
         link_class = H5L_find_class(lnk->type);
 
         if(link_class != NULL && link_class->query_func != NULL) {
@@ -1510,7 +1512,7 @@ H5L_linkval_cb(H5G_loc_t UNUSED *grp_loc/*in*/, const char UNUSED *name, const H
               HGOTO_ERROR(H5E_LINK, H5E_CALLBACK, FAIL, "query callback returned failure")
         }
         else if(udata->buf && udata->size > 0)
-            udata->buf[0] = '\0';
+            ((char *)udata->buf)[0] = '\0';
     }
     else
         HGOTO_ERROR(H5E_LINK, H5E_BADTYPE, FAIL, "object is not a symbolic or user-defined link")
@@ -1544,7 +1546,7 @@ done:
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5L_linkval(H5G_loc_t *loc, const char *name, size_t size, char *buf/*out*/, hid_t lapl_id, hid_t dxpl_id)
+H5L_linkval(H5G_loc_t *loc, const char *name, size_t size, void *buf/*out*/, hid_t lapl_id, hid_t dxpl_id)
 {
     H5L_trav_ud5_t udata;           /* User data for callback */
     herr_t ret_value = SUCCEED;       /* Return value */
@@ -1919,7 +1921,6 @@ done:
             udata_out.lnk->u.soft.name = H5MM_xfree(udata_out.lnk->u.soft.name);
         else if(udata_out.lnk->type >= H5L_TYPE_UD_MIN && udata_out.lnk->u.ud.size > 0)
             udata_out.lnk->u.ud.udata = H5MM_xfree(udata_out.lnk->u.ud.udata);
- /* JAMES: the dest_cb already frees the link name.  Hmm. */
         H5MM_xfree(udata_out.lnk);
     }
 
diff --git a/src/H5Lexternal.c b/src/H5Lexternal.c
index 4ca5c83..ab2d7e6 100644
--- a/src/H5Lexternal.c
+++ b/src/H5Lexternal.c
@@ -292,7 +292,7 @@ done:
 
 
 /*-------------------------------------------------------------------------
- * Function: H5Lunpack_elink_path
+ * Function: H5Lunpack_elink_val
  *
  * Purpose: Given a buffer holding the "link value" from an external link,
  *              gets pointers to the filename and object path within the
@@ -315,7 +315,8 @@ done:
  *-------------------------------------------------------------------------
  */
 herr_t
-H5Lunpack_elink_val(char *ext_linkval, char **filename, char **obj_path)
+H5Lunpack_elink_val(char *ext_linkval, size_t link_size, char **filename,
+                    char **obj_path)
 {
     size_t      len;                /* Length of the filename in the linkval*/
     herr_t      ret_value=SUCCEED;  /* Return value */
@@ -325,15 +326,35 @@ H5Lunpack_elink_val(char *ext_linkval, char **filename, char **obj_path)
 
     if(ext_linkval == NULL )
         HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "not an external link linkval buffer")
+    if(link_size <= 1)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "not a valid external link buffer")
 
+    /* Try to do some error checking.  If the last character in the linkval
+     * (the last character of obj_path) isn't NULL, then something's wrong.
+     */
+    if(ext_linkval[link_size-1] != '\0')
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "linkval buffer is not NULL-terminated")
+
+    /* We're now guaranteed that HDstrlen won't segfault, since the buffer has
+     * at least one NULL in it.
+     */
+    len = HDstrlen(ext_linkval);
+
+    /* If the first NULL we found was at the very end of the buffer, then
+     * this external link value has no object name and is invalid.
+     */
+    if(len + 1>= link_size)
+        HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "linkval buffer doesn't contain an object path")
+
+    /* If we got here then the buffer contains (at least) two strings packed
+     * in the correct way.  Assume it's correct and return pointers to the
+     * filename and object path.
+     */
     if(filename != NULL)
         *filename = ext_linkval;
 
     if(obj_path != NULL)
-    {
-        len = HDstrlen(ext_linkval);
         *obj_path = ext_linkval + len + 1;  /* Add one for NULL terminator */
-    }
 
 done:
     FUNC_LEAVE_API(ret_value)
diff --git a/src/H5Lpublic.h b/src/H5Lpublic.h
index a351ab6..6462170 100644
--- a/src/H5Lpublic.h
+++ b/src/H5Lpublic.h
@@ -127,7 +127,7 @@ H5_DLL herr_t H5Lcreate_soft(const char *target_path, hid_t cur_loc,
                              const char *cur_name, hid_t lcpl_id, hid_t lapl_id);
 H5_DLL herr_t H5Lunlink(hid_t loc_id, const char *name, hid_t lapl_id);
 H5_DLL herr_t H5Lget_linkval(hid_t loc_id, const char *name, size_t size,
-			      char *buf/*out*/, hid_t lapl_id);
+			      void *buf/*out*/, hid_t lapl_id);
 H5_DLL herr_t H5Lget_linkinfo(hid_t loc_id, const char *name,
                               H5L_linkinfo_t *linkbuf /*out*/, hid_t lapl_id);
 
@@ -140,7 +140,7 @@ H5_DLL herr_t H5Lunregister(H5L_type_t id);
 H5_DLL htri_t H5Lis_registered(H5L_type_t id);
 
 /* External link functions */
-H5_DLL herr_t H5Lunpack_elink_val(char * ext_linkval/*in*/,
+H5_DLL herr_t H5Lunpack_elink_val(char * ext_linkval/*in*/, size_t link_size,
             char ** filename/*out*/, char** obj_path /*out*/);
 H5_DLL herr_t H5Lcreate_external(const char *file_name, const char *obj_name,
         hid_t link_loc_id, const char *link_name, hid_t lcpl_id, hid_t lapl_id);
diff --git a/test/links.c b/test/links.c
index 4c22b6e..e8b2e36 100644
--- a/test/links.c
+++ b/test/links.c
@@ -1602,7 +1602,7 @@ external_link_root(hid_t fapl, hbool_t new_format)
 	goto error;
     }
     if(H5Lget_linkval(fid, "ext_link", sizeof(objname), objname, H5P_DEFAULT) < 0) TEST_ERROR
-    if(H5Lunpack_elink_val(objname, &file, &path) < 0) TEST_ERROR
+    if(H5Lunpack_elink_val(objname, sb.linklen, &file, &path) < 0) TEST_ERROR
     if(HDstrcmp(file, filename1))
     {
 	H5_FAILED();
@@ -2628,7 +2628,7 @@ external_link_query(hid_t fapl, hbool_t new_format)
     if(H5Lget_linkval(fid, "src", (size_t)NAME_BUF_SIZE, query_buf, H5P_DEFAULT) < 0) TEST_ERROR
 
     /* Extract the file and object names from the buffer */
-    if(H5Lunpack_elink_val(query_buf, &file_name, &object_name) < 0) TEST_ERROR
+    if(H5Lunpack_elink_val(query_buf, li.u.link_size, &file_name, &object_name) < 0) TEST_ERROR
 
     /* Compare the file and object names */
     if(strcmp(file_name, filename2)) TEST_ERROR
@@ -2646,11 +2646,20 @@ external_link_query(hid_t fapl, hbool_t new_format)
     if(H5Fclose(fid) < 0) TEST_ERROR
 
     /* Make sure that passing in NULLs to H5Lunpack_elink_val works */
-    if(H5Lunpack_elink_val(query_buf, NULL, NULL) < 0) TEST_ERROR
+    if(H5Lunpack_elink_val(query_buf, li.u.link_size, NULL, NULL) < 0) TEST_ERROR
 
     /* Make sure that bogus cases trigger errors in H5Lunpack_elink_val */
     H5E_BEGIN_TRY {
-      if(H5Lunpack_elink_val(NULL, NULL, NULL) >= 0) TEST_ERROR
+      if(H5Lunpack_elink_val(query_buf, li.u.link_size - 1, NULL, NULL) >= 0) TEST_ERROR
+    } H5E_END_TRY
+    H5E_BEGIN_TRY {
+      if(H5Lunpack_elink_val(query_buf, 0, NULL, NULL) >= 0) TEST_ERROR
+    } H5E_END_TRY
+    H5E_BEGIN_TRY {
+      if(H5Lunpack_elink_val(NULL, 0, NULL, NULL) >= 0) TEST_ERROR
+    } H5E_END_TRY
+    H5E_BEGIN_TRY {
+      if(H5Lunpack_elink_val(NULL, 1000, NULL, NULL) >= 0) TEST_ERROR
     } H5E_END_TRY
 
     PASSED();
diff --git a/tools/h5dump/h5dump.c b/tools/h5dump/h5dump.c
index 3f51f81..384b765 100644
--- a/tools/h5dump/h5dump.c
+++ b/tools/h5dump/h5dump.c
@@ -1560,7 +1560,7 @@ dump_all(hid_t group, const char *name, void * op_data)
                     d_status = EXIT_FAILURE;
                     ret = FAIL;
                 } else {
-                    if(H5Lunpack_elink_val(targbuf, &filename, &targname) < 0) {
+                    if(H5Lunpack_elink_val(targbuf, statbuf.linklen, &filename, &targname) < 0) {
                       error_msg(progname, "unable to unpack external link value\n");
                       d_status = EXIT_FAILURE;
                       ret = FAIL;
@@ -3248,7 +3248,7 @@ handle_links(hid_t fid, char *links, void UNUSED * data)
               begin_obj(dump_header_format->extlinkbegin, links,
                         dump_header_format->extlinkblockbegin);
               if (H5Lget_linkval(fid, links, statbuf.linklen, buf, H5P_DEFAULT) >= 0) {
-                  if(H5Lunpack_elink_val(buf, &elink_file, &elink_path)>=0) {
+                  if(H5Lunpack_elink_val(buf, statbuf.linklen, &elink_file, &elink_path)>=0) {
                       indentation(COL);
                       printf("LINKCLASS %d\n", linfo.type);
                       indentation(COL);
diff --git a/tools/h5ls/h5ls.c b/tools/h5ls/h5ls.c
index 04a9371..d34cae0 100644
--- a/tools/h5ls/h5ls.c
+++ b/tools/h5ls/h5ls.c
@@ -1764,7 +1764,7 @@ udlink_open(hid_t location, const char *name)
         if ((buf = HDmalloc(linfo.u.link_size))==NULL) goto error;
         if (H5Lget_linkval (location, name, sizeof(buf), buf, H5P_DEFAULT)<0) goto error;
 
-        if(H5Lunpack_elink_val(buf, &filename, &path) < 0) goto error;
+        if(H5Lunpack_elink_val(buf, linfo.u.link_size,  &filename, &path) < 0) goto error;
         fputs("file: ", stdout);
         fputs(filename, stdout);
         fputs("    path: ", stdout);
diff --git a/tools/lib/h5trav.c b/tools/lib/h5trav.c
index 8a6173c..8480f57 100644
--- a/tools/lib/h5trav.c
+++ b/tools/lib/h5trav.c
@@ -521,7 +521,7 @@ static int traverse( hid_t loc_id,
       targbuf = HDmalloc(statbuf.linklen);
       assert(targbuf);
       H5Gget_linkval(loc_id,path,statbuf.linklen,targbuf);
-      H5Lunpack_elink_val(targbuf, NULL, &objname);
+      H5Lunpack_elink_val(targbuf, statbuf.linklen, NULL, &objname);
       if (print)
         printf(" %-10s %s -> %s %s\n", "ext link", path, targbuf, objname);
       free(targbuf);