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 a few issues with error reporting during sec2 reads/writes #4794

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Fix a few issues with error reporting during sec2 reads/writes #4794

merged 2 commits into from
Sep 4, 2024

Conversation

jhendersonHDF
Copy link
Collaborator

No description provided.

@jhendersonHDF jhendersonHDF added Merge Use this label when a PR is for a downstream merge Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Component - C Library Core C library issues (usually in the src directory) Type - Bug Please report security issues to [email protected] instead of creating an issue on GitHub labels Aug 30, 2024
jhendersonHDF
jhendersonHDF commented Aug 30, 2024
View reviewed changes
src/H5FDsec2.c
@@ -700,15 +700,16 @@ H5FD__sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU
int myerrno = errno;
time_t mytime = time(NULL);

#ifndef H5_HAVE_PREADWRITE
offset = HDlseek(file->fd, 0, SEEK_CUR);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since pread doesn't change the file offset, this HDlseek call ends up resetting the file offset that gets printed out on errors.

jhendersonHDF
jhendersonHDF commented Aug 30, 2024
View reviewed changes
src/H5FDsec2.c
(unsigned long long)bytes_read, (unsigned long long)offset);
"error message = '%s', buf = %p, total read size = %z, bytes this sub-read = %llu, "
"offset = %llu",
ctime(&mytime), file->filename, file->fd, myerrno, strerror(myerrno), buf, size,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Switched to %zu for size rather than casting to unsigned long long.

jhendersonHDF
jhendersonHDF commented Aug 30, 2024
View reviewed changes
src/H5FDsec2.c
"bytes actually read = %llu, offset = %llu",
ctime(&mytime), file->filename, file->fd, myerrno, strerror(myerrno), buf,
(unsigned long long)size, (unsigned long long)bytes_in,
(unsigned long long)bytes_read, (unsigned long long)offset);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the printing out of bytes_read since it will always be -1 in this case, so casting it to unsigned long long and printing it out just gives misleading output.

@jhendersonHDF jhendersonHDF changed the title Fix a few issues with error reporting during sec2 reads Fix a few issues with error reporting during sec2 reads/writes Aug 30, 2024
@lrknox lrknox merged commit 308fc97 into HDFGroup:develop Sep 4, 2024
56 checks passed
lrknox pushed a commit to lrknox/hdf5 that referenced this pull request Sep 11, 2024
@jhendersonHDF @lrknox
Fix a few issues with error reporting during sec2 reads/writes (HDFGr…
6264832 
…oup#4794)
@jhendersonHDF jhendersonHDF deleted the sec2_read_error_info_fix branch September 12, 2024 20:31
lrknox added a commit that referenced this pull request Sep 13, 2024
@lrknox
Sync develop changes 8/27 - 9/11 to hdf5_1_14. (#4822)
4d135a0 
* Remove dummy comments that repeat function names. (#4775)

* Fix grammar in H5Odtype.c comment block (#4777)

* Add subfiling checks to the gcc action (#4776)

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

This PR switches H5I_object() to H5VL_vol_object() in H5O and H5T APIs.                 H5M is the last one and left out of this PR because it needs more work
in documentation and there is no test for the API functions.

Fixes GH-4730

* Add subfiling to CI more places where we test parallel (#4778)

* CMake: gcc,, AOCC
* Autotools: AOCC (gcc was added in a previous commit)

NVHPC generates a lot of tools errors for some reason

* Convert Collective Calls html file to doxygen (#4779)

* Fix grammar in H5Fint.c comment block (#4782)

* Improve the consistency of configure help messages (#4783)

Fix grammar in configure message

* Fixes Fortran parallel build race condition for tests (#4789)

* Update URL documentation links to support site (#4781)

* Fix grammar and simplify comment in H5Fint.c (#4790)

* Fix char-subscripts warnings in H5private.h (#4793)

* Bump the github-actions group with 3 updates (#4798)

Bumps the github-actions group with 3 updates: [actions/checkout](https://github.com/actions/checkout), [DoozyX/clang-format-lint-action](https://github.com/doozyx/clang-format-lint-action) and [github/codeql-action](https://github.com/github/codeql-action).


Updates `actions/checkout` from 4.1.1 to 4.1.7
- [Release notes](https://github.com/actions/checkout/releases)
- [Commits](actions/checkout@v4.1.1...v4.1.7)

Updates `DoozyX/clang-format-lint-action` from 0.17 to 0.18
- [Release notes](https://github.com/doozyx/clang-format-lint-action/releases)
- [Commits](DoozyX/clang-format-lint-action@v0.17...v0.18)

Updates `github/codeql-action` from 3.25.15 to 3.26.6
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@afb54ba...4dd1613)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: github-actions
- dependency-name: DoozyX/clang-format-lint-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: github-actions
- dependency-name: github/codeql-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
  dependency-group: github-actions
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Fix typo argueably in H5Cprivate.h (#4795)

* Fix typos in H5Cpkg.h (#4796)

* Add bounds checking to avoid Out-of-bounds Write for gif2h5 (#4786)

* Replace non-VOL calls with VOL calls - part 5 (#4788)

This PR switches H5I_object() and H5I_object_verify() to H5VL_vol_object() and          H5VL_vol_object_verify(), respectively, in the H5M APIs and H5Gdeprec (was left         out by mistake).  This completes the fixes of issue GH-4730.

* Correct the URL paths (#4802)

* Fix a few issues with error reporting during sec2 reads/writes (#4794)

* Update windows and apple signing process (#4806)

* Use latest clang format action (#4807)

* Correct path to document (#4808)

* Detect invalid ID to H5Gmove2 (#4765)

User's application segfaulted because the returned value H5I_BADID wasn't
detected when H5I_get_type() was called.  This PR adds checks for invalid
file/group identifiers passed into H5Gmove2.

This defect occurs in many other places, hence, issue GH-4764.

Fixes #4737

* Fix use of public API calls (#4809)

Switch public API calls to private ones. Root cause of #4672, which it fixes.

Also minor code cleanups

* Remove call to H5E_clear_stack() (#4810)

Remove call to H5E_clear_stack() in H5G_node_debug()

Add misc. minor cleanups

* Enable win Intel signing (#4812)

* Remove unneeded file name part (#4814)

* Update NVHPC optimization settings (#4815)

* Use -gopt in Autotools/CMake instead of -g
* Autotools uses -O3 for release, -O1 for debug
* Remove CMake optimization flag removal hack

* Add mirror VFD to serial -Werror CI action (#4753)

* Add mirror VFD to serial -Werror CI action

* NUL terminate mirror_vfd.c strings

* Add THREADS check to configuration file (#4746)

* Cleanup threads package checks

* Check first if package was found

* Remove unnecessary dependent checks

* Remove Unused options and fix names of option prefix

* Rework RFC url in aliases (#4813)

* Update config/cmake/hdf5-config.cmake.in

_ENABLE_THREADS doesn't belong in hdf5_1_14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qkoziol qkoziol qkoziol approved these changes

@lrknox lrknox lrknox approved these changes

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

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

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

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

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

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

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

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

Assignees
No one assigned
Labels
Component - C Library Core C library issues (usually in the src directory) Merge Use this label when a PR is for a downstream merge Priority - 3. Low 🔽 Code cleanup, small feature change requests, etc. Type - Bug Please report security issues to [email protected] instead of creating an issue on GitHub
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jhendersonHDF @qkoziol @lrknox