[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.

3 files changed, 38 insertions, 16 deletions

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);