Skip to content

Commit

Permalink
Fix inconsistent documentation of get_name functions (HDFGroup#4715)
Browse files Browse the repository at this point in the history
- Verified that the listed functions do not include null terminator in the returned length
- Improved some of the tests
- Corrected documentation

Fixes HDFGroupGH-4704

* Casted a positive int to size_t
  • Loading branch information
bmribler authored and lrknox committed Aug 27, 2024
1 parent 2c89149 commit c3444df
Show file tree
Hide file tree
Showing 12 changed files with 68 additions and 52 deletions.
42 changes: 27 additions & 15 deletions src/H5A.c
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,7 @@ H5Aget_create_plist(hid_t attr_id)
DESCRIPTION
This function retrieves the name of an attribute for an attribute ID.
Up to 'buf_size' characters are stored in 'buf' followed by a '\0' string
Up to 'buf_size'-1 characters are stored in 'buf' followed by a '\0' string
terminator. If the name of the attribute is longer than 'buf_size'-1,
the string terminator is stored in the last position of the buffer to
properly terminate the string.
Expand Down Expand Up @@ -1258,20 +1258,32 @@ H5Aget_name(hid_t attr_id, size_t buf_size, char *buf /*out*/)
FUNC_LEAVE_API(ret_value)
} /* H5Aget_name() */

/*-------------------------------------------------------------------------
* Function: H5Aget_name_by_idx
*
* Purpose: Retrieve the name of an attribute, according to the
* order within an index.
*
* Same pattern of behavior as H5Iget_name.
*
* Return: Success: Non-negative length of name, with information
* in NAME buffer
* Failure: Negative
*
*-------------------------------------------------------------------------
*/
/*--------------------------------------------------------------------------
NAME
H5Aget_name_by_idx
PURPOSE
Retrieve the name of an attribute, according to the order within an index.
USAGE
ssize_t H5Aget_name_by_idx(loc_id, obj_name, idx_type, order, n, name, size, lapl_id)
hid_t loc_id; IN: Object that attribute is attached to
const char *obj_name; IN: Name of the object relative to location
H5_index_t idx_type; IN: Type of index to use
H5_iter_order_t order; IN: Order to iterate over index
hsize_t n; IN: Index (0-based) of attribute to retrieve
char *name; IN: Buffer to store the name in
size_t size; IN: The size of the buffer to store the name in.
hid_t lapl_id; IN: Link access property list
RETURNS
This function returns the length of the attribute's name (which may be
longer than 'buf_size') on success or negative for failure.
DESCRIPTION
This function retrieves the name of an attribute given its index. Up
to 'buf_size'-1 characters are stored in 'buf' followed by a '\0' string
terminator. If the name of the attribute is longer than 'buf_size'-1,
the string terminator is stored in the last position of the buffer to
properly terminate the string.
--------------------------------------------------------------------------*/
ssize_t
H5Aget_name_by_idx(hid_t loc_id, const char *obj_name, H5_index_t idx_type, H5_iter_order_t order, hsize_t n,
char *name /*out*/, size_t size, hid_t lapl_id)
Expand Down
15 changes: 3 additions & 12 deletions src/H5Apublic.h
Original file line number Diff line number Diff line change
Expand Up @@ -501,15 +501,9 @@ H5_DLL herr_t H5Aget_info_by_name(hid_t loc_id, const char *obj_name, const char
* value.
*
* \details H5Aget_name() retrieves the name of an attribute specified by
* the identifier, \p attr_id. Up to \p buf_size characters are
* stored in \p buf followed by a \0 string terminator. If the
* name of the attribute is longer than (\p buf_size -1), the
* string terminator is stored in the last position of the buffer
* to properly terminate the string.
* the identifier, \p attr_id.
*
* If the user only wants to retrieve the name length, the
* values 0 and NULL should be passed for the parameters
* \p bufsize and \p buf.
* \details_namelen{attribute,H5Aget_name}
*
* \since 1.0.0
*
Expand Down Expand Up @@ -544,10 +538,7 @@ H5_DLL ssize_t H5Aget_name(hid_t attr_id, size_t buf_size, char *buf);
* traversal order, and a position in the index, \p idx_type,
* \p order and \p n, respectively.
*
* If the attribute name's size is unknown, the values 0 and NULL
* can be passed in for the parameters \p size and \p name. The
* function's return value will provide the correct value for
* \p size.
* \details_namelen{attribute,H5Aget_name_by_idx}
*
* The link access property list, \p lapl_id, may provide
* information regarding the properties of links required to access
Expand Down
8 changes: 5 additions & 3 deletions src/H5Epublic.h
Original file line number Diff line number Diff line change
Expand Up @@ -420,9 +420,11 @@ H5_DLL herr_t H5Eclose_stack(hid_t stack_id);
* by the class identifier. If a non-NULL pointer is passed in for \p
* name and \p size is greater than zero, the class name of \p size
* long is returned. The length of the error class name is also
* returned. If NULL is passed in as \p name, only the length of class
* name is returned. If zero is returned, it means no name. The user is
* responsible for allocating sufficient buffer space for the name.
* returned.
*
* \details_namelen{error class,H5Eget_class_name}
*
* If zero is returned, it means the error class has no name.
*
* \since 1.8.0
*/
Expand Down
7 changes: 3 additions & 4 deletions src/H5Gdeprec.c
Original file line number Diff line number Diff line change
Expand Up @@ -719,11 +719,10 @@ H5Gset_comment(hid_t loc_id, const char *name, const char *comment)
*
* Note: Deprecated in favor of H5Oget_comment/H5Oget_comment_by_name
*
* Return: Success: Number of characters in the comment counting
* the null terminator. The value returned may
* be larger than the BUFSIZE argument.
* Return: Success: Number of characters in the comment. The value
* returned may be larger than the BUFSIZE argument.
*
* Failure: Negative
* Failure: Negative
*
*-------------------------------------------------------------------------
*/
Expand Down
7 changes: 1 addition & 6 deletions src/H5Gpublic.h
Original file line number Diff line number Diff line change
Expand Up @@ -960,12 +960,7 @@ H5_DLL herr_t H5Gset_comment(hid_t loc_id, const char *name, const char *comment
* root group
* \li A dot (\c .), if \p loc_id fully specifies the object
*
* At most bufsize characters, including a null-terminator, are
* returned in \p buf. The returned value is not null-terminated if the
* comment is longer than the supplied buffer. If the size of the
* comment is unknown, a preliminary \p H5Gget_comment() call will
* return the size of the comment, including space for the
* null-terminator.
* \details_namelen{comment,H5Gget_comment}
*
* If an object does not have a comment, the empty string is returned
* in comment.
Expand Down
11 changes: 4 additions & 7 deletions src/H5Lpublic.h
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ H5_DLL herr_t H5Lget_info2(hid_t loc_id, const char *name, H5L_info2_t *linfo, h
*
* \return \herr_t
*
* \details H5get_info_by_idx2() returns the metadata for a link in a group
* \details H5Lget_info_by_idx2() returns the metadata for a link in a group
* according to a specified field or index and a specified order. The
* link for which information is to be returned is specified by \p
* idx_type, \p order, and \p n as follows:
Expand Down Expand Up @@ -819,7 +819,7 @@ H5_DLL herr_t H5Lget_info_by_idx2(hid_t loc_id, const char *group_name, H5_index
* \return Returns the size of the link name if successful; otherwise returns a
* negative value.
*
* \details H5get_name_by_idx() retrieves the name of the \Emph{n}-th link in a
* \details H5Lget_name_by_idx() retrieves the name of the \Emph{n}-th link in a
* group, according to the specified order, \p order, within a specified
* field or index, \p idx_type.
*
Expand All @@ -835,10 +835,7 @@ H5_DLL herr_t H5Lget_info_by_idx2(hid_t loc_id, const char *group_name, H5_index
* If \p loc_id specifies the group in which the link resides,
* \p group_name can be a dot (\c .).
*
* The size in bytes of name is specified in \p size. If \p size is
* unknown, it can be determined via an initial H5Lget_name_by_idx()
* call with name set to NULL; the function's return value will be the
* size of the name.
* \details_namelen{link,H5Lget_name_by_idx}
*
* \note Please note that in order for the specified index to correspond to the
* creation order index, \p order must be set to #H5_ITER_INC or
Expand Down Expand Up @@ -1578,7 +1575,7 @@ H5_DLL herr_t H5Lget_info1(hid_t loc_id, const char *name, H5L_info1_t *linfo /*
* the function H5Lget_info_by_idx2() and the macro
* H5Lget_info_by_idx().
*
* \details H5get_info_by_idx1() returns the metadata for a link in a group
* \details H5Lget_info_by_idx1() returns the metadata for a link in a group
* according to a specified field or index and a specified order.
*
* The link for which information is to be returned is specified by \p
Expand Down
6 changes: 4 additions & 2 deletions test/error_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,13 @@ test_error(hid_t file)
static herr_t
init_error(void)
{
ssize_t cls_size = (ssize_t)strlen(ERR_CLS_NAME) + 1;
ssize_t cls_size = (ssize_t)strlen(ERR_CLS_NAME);
ssize_t msg_size = (ssize_t)strlen(ERR_MIN_SUBROUTINE_MSG) + 1;
char *cls_name = NULL;
char *msg = NULL;
H5E_type_t msg_type;

/* Account for null terminator */
if (NULL == (cls_name = (char *)malloc(strlen(ERR_CLS_NAME) + 1)))
TEST_ERROR;
if (NULL == (msg = (char *)malloc(strlen(ERR_MIN_SUBROUTINE_MSG) + 1)))
Expand All @@ -189,7 +190,8 @@ init_error(void)
if ((ERR_CLS = H5Eregister_class(ERR_CLS_NAME, PROG_NAME, PROG_VERS)) < 0)
TEST_ERROR;

if (cls_size != H5Eget_class_name(ERR_CLS, cls_name, (size_t)cls_size) + 1)
/* Account for null terminator */
if (cls_size != H5Eget_class_name(ERR_CLS, cls_name, (size_t)cls_size + 1))
TEST_ERROR;
if (strcmp(ERR_CLS_NAME, cls_name) != 0)
TEST_ERROR;
Expand Down
14 changes: 12 additions & 2 deletions test/links.c
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,7 @@ test_deprec(hid_t fapl, bool new_format)
hsize_t num_objs; /* Number of objects in a group */
char filename[1024];
char tmpstr[1024];
int len = 0; /* Length of comment */

if (new_format)
TESTING("backwards compatibility (w/new group format)");
Expand All @@ -1968,12 +1969,21 @@ test_deprec(hid_t fapl, bool new_format)
FAIL_STACK_ERROR;

/* Test H5Gset and get comment */

if (H5Gset_comment(file_id, "group1", "comment") < 0)
FAIL_STACK_ERROR;
if (H5Gget_comment(file_id, "group1", sizeof(tmpstr), tmpstr) < 0)
if ((len = H5Gget_comment(file_id, "group1", 0, NULL)) < 0)
FAIL_STACK_ERROR;

/* Returned length should be the same as strlen of the comment */
if ((size_t)len != strlen("comment"))
FAIL_STACK_ERROR;

/* Get and verify the comment */
if (H5Gget_comment(file_id, "group1", (size_t)len + 1, tmpstr) < 0)
FAIL_STACK_ERROR;
if (strcmp(tmpstr, "comment") != 0)
TEST_ERROR;
FAIL_STACK_ERROR;

/* Create links using H5Glink and H5Glink2 */
if (H5Glink(file_id, H5G_LINK_HARD, "group2", "group1/link_to_group2") < 0)
Expand Down
3 changes: 2 additions & 1 deletion test/tfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -2420,10 +2420,11 @@ test_file_getname(void)
file_id = H5Fcreate(FILE1, H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT);
CHECK(file_id, FAIL, "H5Fcreate");

/* Get and verify file name */
/* Get and verify file name and its length */
name_len = H5Fget_name(file_id, name, (size_t)TESTA_NAME_BUF_SIZE);
CHECK(name_len, FAIL, "H5Fget_name");
VERIFY_STR(name, FILE1, "H5Fget_name");
VERIFY(name_len, strlen(FILE1), "H5Fget_name");

/* Create a group in the root group */
group_id = H5Gcreate2(file_id, TESTA_GROUPNAME, H5P_DEFAULT, H5P_DEFAULT, H5P_DEFAULT);
Expand Down
5 changes: 5 additions & 0 deletions test/th5o.c
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,7 @@ test_h5o_comment(void)
/* Getting the comment on the file and verify it */
comment_len = H5Oget_comment(fid, NULL, (size_t)0);
CHECK(comment_len, FAIL, "H5Oget_comment");
VERIFY(comment_len, strlen(file_comment), "H5Oget_comment");

len = H5Oget_comment(fid, check_comment, (size_t)comment_len + 1);
CHECK(len, FAIL, "H5Oget_comment");
Expand All @@ -1237,6 +1238,7 @@ test_h5o_comment(void)

len = H5Oget_comment(grp, check_comment, (size_t)comment_len + 1);
CHECK(len, FAIL, "H5Oget_comment");
VERIFY(len, strlen(grp_comment), "H5Oget_comment");

ret_value = strcmp(grp_comment, check_comment);
VERIFY(ret_value, 0, "H5Oget_comment");
Expand All @@ -1248,6 +1250,7 @@ test_h5o_comment(void)
/* Getting the comment on the datatype and verify it */
comment_len = H5Oget_comment(dtype, NULL, (size_t)0);
CHECK(comment_len, FAIL, "H5Oget_comment");
VERIFY(comment_len, strlen(dtype_comment), "H5Oget_comment");

len = H5Oget_comment(dtype, check_comment, (size_t)comment_len + 1);
CHECK(len, FAIL, "H5Oget_comment");
Expand All @@ -1265,6 +1268,7 @@ test_h5o_comment(void)

len = H5Oget_comment(dset, check_comment, (size_t)comment_len + 1);
CHECK(ret, len, "H5Oget_comment");
VERIFY(len, strlen(dset_comment), "H5Oget_comment");

ret_value = strcmp(dset_comment, check_comment);
VERIFY(ret_value, 0, "H5Oget_comment");
Expand Down Expand Up @@ -1406,6 +1410,7 @@ test_h5o_comment_by_name(void)

len = H5Oget_comment_by_name(fid, ".", check_comment, (size_t)comment_len + 1, H5P_DEFAULT);
CHECK(len, FAIL, "H5Oget_comment_by_name");
VERIFY(len, strlen(file_comment), "H5Oget_comment");

ret_value = strcmp(file_comment, check_comment);
VERIFY(ret_value, 0, "H5Oget_comment_by_name");
Expand Down
1 change: 1 addition & 0 deletions test/tmisc.c
Original file line number Diff line number Diff line change
Expand Up @@ -4296,6 +4296,7 @@ test_misc23(void)
namelen = H5Iget_name(tmp_id, objname, (size_t)MISC23_NAME_BUF_SIZE);
CHECK(namelen, FAIL, "H5Iget_name");
VERIFY_STR(objname, "/A/B01/grp", "H5Iget_name");
VERIFY(namelen, strlen("/A/B01/grp"), "H5Iget_name");

status = H5Gclose(tmp_id);
CHECK(status, FAIL, "H5Gclose");
Expand Down
1 change: 1 addition & 0 deletions test/trefer.c
Original file line number Diff line number Diff line change
Expand Up @@ -2417,6 +2417,7 @@ test_reference_group(void)
H5P_DEFAULT);
CHECK(size, (-1), "H5Lget_name_by_idx");
VERIFY_STR(objname, DSETNAME2, "H5Lget_name_by_idx");
VERIFY(size, strlen(DSETNAME2), "H5Lget_name_by_idx");

ret = H5Oget_info_by_idx3(gid, ".", H5_INDEX_NAME, H5_ITER_INC, (hsize_t)0, &oinfo, H5O_INFO_BASIC,
H5P_DEFAULT);
Expand Down

0 comments on commit c3444df

Please sign in to comment.