Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix inconsistent documentation of get_name functions #4715

Merged
merged 5 commits into from
Aug 23, 2024
Merged

Fix inconsistent documentation of get_name functions #4715

merged 5 commits into from
Aug 23, 2024

Conversation

bmribler
Copy link
Contributor

Fixes GH-4704

bmribler and others added 2 commits August 15, 2024 14:48
@bmribler
Fix inconsistent documentation of get_name functions
b259967 
- Verified that the listed functions do not include null termintor in the returned length
- Improved some of the tests
- Corrected documentation

Fixes HDFGroupGH-4704
@github-actions
Committing clang-format changes
f2f1396
@bmribler bmribler added Merge - To 1.14 This needs to be merged to HDF5 1.14 Priority - 2. Medium ⏹ It would be nice to have this in the next release Component - Documentation Doxygen, markdown, etc. Component - Testing Code in test or testpar directories, GitHub workflows Type - Improvement Improvements that don't add a new feature or functionality labels Aug 16, 2024
derobins
derobins reviewed Aug 16, 2024
View reviewed changes
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 */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

len should probably be a size_t

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

H5Gget_comment returns int. I casted len to size_t for strcmp after checking for error.

mattjala
mattjala reviewed Aug 19, 2024
View reviewed changes
src/H5A.c Outdated
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be "Index (0-based) of attribute to retrieve"

jhendersonHDF
jhendersonHDF reviewed Aug 23, 2024
View reviewed changes
src/H5Gdeprec.c
* 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably go the other way as in "Number of characters in the comment, excluding the NUL terminator character."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jhendersonHDF I missed this, I'll put it in one of my next PRs.

@lrknox lrknox merged commit 379a5ba into HDFGroup:develop Aug 23, 2024
54 of 55 checks passed
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Aug 27, 2024
@bmribler @lrknox
Fix inconsistent documentation of get_name functions (HDFGroup#4715)
c3444df 
- 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
derobins added a commit that referenced this pull request Aug 27, 2024
@lrknox @hyoklee
@ajelenak @bmribler @derobins @jhendersonHDF @mattjala
Sync develop changes August 21 - 27 to hdf5_1_14 (#4774)
e77c72e 
* Use gfortran 14 for cmake-ctest.yml on mac (#4739)

* Use gfortran 14 for cmake-test on mac

* Remove notarization step

* Address @byrnHDF review

* Fix enum type mismatch warning (#4741)

* Fix macro redefined warnings (#4744)

Removes a duplicated HDopen macro from the performance testing programs

* Update nvhpc CI version to 24.7 (#4740)

* Return basic HTTP range GET logging to ROS3 (#4738)

* Add minimal amount of S3 request logging to ROS3

* Fix ROS3 logging ifdef conditions

* Replace non-VOL calls with VOL calls - part 1 (#4745)

This PR is part of the incremental switching H5I_object() and H5I_object_verify()
to their VOL counterpart, H5VL_object() and H5VL_vol_object_verify(), a newly addedinternal function.

Fixes GH-4730 partially.

* Fix inconsistent documentation of get_name functions (#4715)

- Verified that the listed functions do not include null terminator in the returned length
- Improved some of the tests
- Corrected documentation

Fixes GH-4704

* Casted a positive int to size_t

* Remove HDF-EOS5 CI action (#4750)

The code can't be downloaded due to changes that put it behind an
EarthData login. We'll disable this while we figure out a work-around.

* Replace non-VOL calls with VOL calls - part 2 (#4748)

This PR switches H5I_object_verify() to H5VL_vol_object_verify() in the H5F API
and fixes documentation of H5Fmount and H5Funmount.

* More on H5F API

* Restore rand_r in a few parallel tests (#4749)

The t_pmulti_dset and t_select_io_dset tests rely on the behavior
of the previous private rand_r-like implementation to get the
correct sequence of random numbers to pass. This has been restored
using a fully private rand_r-like implementation that doesn't
rely on rand_r and will work on Windows and other platforms
where rand_r doesn't exist.

* Don't run AOCC parallel tests with -j2 (#4752)

Don't run parallel tests in both Autotools and CMake with multiple
processes. ph5diff still runs with -j2 w/ Autotools since the test
script is in the tools/test/h5diff directory.

* Split off AOCC CMake parallel tests

* Remove unnecessary NPROCS env vars

* Put NPROCS back in serial tests

We run ph5diff tests there

* Replace non-VOL calls with VOL calls - part 3 (#4756)

This PR switches H5I_object_verify() to H5VL_vol_object_verify() in the H5G API
and removes unnecessary casts.

* Turn on parallel CI tests in Autotools & CMake (#4573)

* Fix typo in H5Centry.c (#4762)

* Set/Unset VOL wrapping context in H5VL_attr_close (#4759)

* Add missing C++ and Fortran to Intel oneAPI CI (#4761)

* Add Fortran and C++ to Autotools
* Add Fortran and C++ to Linux CMake
* Add C++ to Windows CMake
* Fix bad GitHub workspace variable

* Remove early test exit (#4757)

* Don't skip file tests

* Remove test with invalid flag for H5Fopen

* Verify that create/open of unseekable file fails

* Remove failure verification

* Restore Julia CI (#4763)

Fixes #4539

* Capitalize f in (#4766)

* Add testing to NVHPC CI actions (CMake & Autotools) (#4760)

Turns on testing, both serial and parallel, but skips:
* dt_arith and dtransform in CMake
* All main library tests in the Autotools
Due to dt_arith and dtransform segfaults when handling long doubles.

* Fix typo in H5T_order_t enum (#4773)

'bit endian' --> 'big endian'

* Correct julia workflows name for hdf5_1_14 branch.

---------

Co-authored-by: H. Joe Lee <hyoklee@hdfgroup.org>
Co-authored-by: Aleksandar Jelenak <ajelenak@users.noreply.github.com>
Co-authored-by: bmribler <39579120+bmribler@users.noreply.github.com>
Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com>
Co-authored-by: jhendersonHDF <jhenderson@hdfgroup.org>
Co-authored-by: mattjala <124107509+mattjala@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derobins derobins derobins left review comments

@jhendersonHDF jhendersonHDF jhendersonHDF left review comments

@lrknox lrknox lrknox approved these changes

@mattjala mattjala mattjala approved these changes

@byrnHDF byrnHDF Awaiting requested review from byrnHDF byrnHDF is a code owner

@fortnern fortnern Awaiting requested review from fortnern fortnern is a code owner

@qkoziol qkoziol Awaiting requested review from qkoziol qkoziol is a code owner

@vchoi-hdfgroup vchoi-hdfgroup Awaiting requested review from vchoi-hdfgroup vchoi-hdfgroup is a code owner

@glennsong09 glennsong09 Awaiting requested review from glennsong09 glennsong09 is a code owner

@brtnfld brtnfld Awaiting requested review from brtnfld brtnfld is a code owner

Assignees
No one assigned
Labels
Component - Documentation Doxygen, markdown, etc. Component - Testing Code in test or testpar directories, GitHub workflows Merge - To 1.14 This needs to be merged to HDF5 1.14 Priority - 2. Medium ⏹ It would be nice to have this in the next release Type - Improvement Improvements that don't add a new feature or functionality
Projects
Downstream HDF5 Merges
Status: Needs Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Inconsistent behavior of get_name functions in documentation
5 participants
@bmribler @derobins @lrknox @jhendersonHDF @mattjala