From b2599678d7682d95a917713833e978c7ba2f84fb Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Thu, 15 Aug 2024 14:48:51 -0400 Subject: [PATCH 1/4] Fix inconsistent documentation of get_name functions - Verified that the listed functions do not include null termintor in the returned length - Improved some of the tests - Corrected documentation Fixes GH-4704 --- src/H5A.c | 42 +++++++++++++++++++++++++++--------------- src/H5Apublic.h | 15 +++------------ src/H5Epublic.h | 8 +++++--- src/H5Gdeprec.c | 7 +++---- src/H5Gpublic.h | 7 +------ src/H5Lpublic.h | 11 ++++------- test/error_test.c | 6 ++++-- test/links.c | 12 +++++++++++- test/tfile.c | 3 ++- test/th5o.c | 5 +++++ test/tmisc.c | 1 + test/trefer.c | 1 + 12 files changed, 67 insertions(+), 51 deletions(-) diff --git a/src/H5A.c b/src/H5A.c index 651ed13c256..d078ed5f623 100644 --- a/src/H5A.c +++ b/src/H5A.c @@ -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. @@ -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) attribute + 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) diff --git a/src/H5Apublic.h b/src/H5Apublic.h index 256d19cf355..7c28c0a24a2 100644 --- a/src/H5Apublic.h +++ b/src/H5Apublic.h @@ -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 * @@ -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 diff --git a/src/H5Epublic.h b/src/H5Epublic.h index 49628efb9be..9263c3c96bf 100644 --- a/src/H5Epublic.h +++ b/src/H5Epublic.h @@ -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 */ diff --git a/src/H5Gdeprec.c b/src/H5Gdeprec.c index 433748e4389..e86bc82c738 100644 --- a/src/H5Gdeprec.c +++ b/src/H5Gdeprec.c @@ -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 * *------------------------------------------------------------------------- */ diff --git a/src/H5Gpublic.h b/src/H5Gpublic.h index 318098b9081..3f1268bc276 100644 --- a/src/H5Gpublic.h +++ b/src/H5Gpublic.h @@ -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. diff --git a/src/H5Lpublic.h b/src/H5Lpublic.h index 2bf3c53b83e..5a418b8bd58 100644 --- a/src/H5Lpublic.h +++ b/src/H5Lpublic.h @@ -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: @@ -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. * @@ -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 @@ -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 diff --git a/test/error_test.c b/test/error_test.c index fe27300f431..54a6f8031ef 100644 --- a/test/error_test.c +++ b/test/error_test.c @@ -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))) @@ -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; diff --git a/test/links.c b/test/links.c index 222b3b66039..6f2212a7c9e 100644 --- a/test/links.c +++ b/test/links.c @@ -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)"); @@ -1968,9 +1969,18 @@ 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 (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; diff --git a/test/tfile.c b/test/tfile.c index 62881264441..823165709e8 100644 --- a/test/tfile.c +++ b/test/tfile.c @@ -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); diff --git a/test/th5o.c b/test/th5o.c index 801091f6b9f..09eb94d1be4 100644 --- a/test/th5o.c +++ b/test/th5o.c @@ -1215,6 +1215,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"); @@ -1232,6 +1233,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"); @@ -1243,6 +1245,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"); @@ -1260,6 +1263,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"); @@ -1401,6 +1405,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"); diff --git a/test/tmisc.c b/test/tmisc.c index a9d94a5ec97..63bf5d8edcb 100644 --- a/test/tmisc.c +++ b/test/tmisc.c @@ -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"); diff --git a/test/trefer.c b/test/trefer.c index fcd0a21484a..8d16de4b0dd 100644 --- a/test/trefer.c +++ b/test/trefer.c @@ -2416,6 +2416,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); From f2f1396de9653194ab20b04af43c6880ed50e70f Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 15 Aug 2024 18:52:55 +0000 Subject: [PATCH 2/4] Committing clang-format changes --- test/error_test.c | 2 +- test/links.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/error_test.c b/test/error_test.c index 54a6f8031ef..204055097b8 100644 --- a/test/error_test.c +++ b/test/error_test.c @@ -191,7 +191,7 @@ init_error(void) TEST_ERROR; /* Account for null terminator */ - if (cls_size != H5Eget_class_name(ERR_CLS, cls_name, (size_t)cls_size+1)) + 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; diff --git a/test/links.c b/test/links.c index 6f2212a7c9e..229b753048a 100644 --- a/test/links.c +++ b/test/links.c @@ -1949,7 +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 */ + int len = 0; /* Length of comment */ if (new_format) TESTING("backwards compatibility (w/new group format)"); @@ -1972,7 +1972,7 @@ test_deprec(hid_t fapl, bool new_format) if (H5Gset_comment(file_id, "group1", "comment") < 0) FAIL_STACK_ERROR; - if ((len=H5Gget_comment(file_id, "group1", 0, NULL)) < 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 */ @@ -1980,7 +1980,7 @@ test_deprec(hid_t fapl, bool new_format) FAIL_STACK_ERROR; /* Get and verify the comment */ - if (H5Gget_comment(file_id, "group1", (size_t)len+1, tmpstr) < 0) + if (H5Gget_comment(file_id, "group1", (size_t)len + 1, tmpstr) < 0) FAIL_STACK_ERROR; if (strcmp(tmpstr, "comment") != 0) TEST_ERROR; From 55e1a894d39d6eff599717e7477c014b0e5d296b Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Mon, 19 Aug 2024 10:42:31 -0400 Subject: [PATCH 3/4] Casted a positive int to size_t --- test/links.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/links.c b/test/links.c index 6f2212a7c9e..755a41d4ca6 100644 --- a/test/links.c +++ b/test/links.c @@ -1976,14 +1976,14 @@ test_deprec(hid_t fapl, bool new_format) FAIL_STACK_ERROR; /* Returned length should be the same as strlen of the comment */ - if (len != strlen("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) From 42c6500dd8d3b753f79557faafc1cf6ab6147e25 Mon Sep 17 00:00:00 2001 From: Binh-Minh Date: Thu, 22 Aug 2024 12:16:04 -0400 Subject: [PATCH 4/4] Fix comment --- src/H5A.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/H5A.c b/src/H5A.c index d078ed5f623..69a771ec6cb 100644 --- a/src/H5A.c +++ b/src/H5A.c @@ -1269,7 +1269,7 @@ H5Aget_name(hid_t attr_id, size_t buf_size, char *buf /*out*/) 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) attribute + 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