Skip to content

Fix reading across page boundary #80

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
Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Fix reading across page boundary #80

wants to merge 4 commits into from

Conversation

@cphurley82
Copy link

@cphurley82 cphurley82 commented Nov 20, 2025 •
edited
Loading

Summary

  • Corrected memory::handle_operation to limit the second-page read to the remaining requested length, preventing buffer overwrite when crossing a page boundary.
  • Added a GoogleTest-based memory_boundary_test that reproduces and now validates the fixed boundary read.
  • Added unit test to CI.
  • Added TESTING.md.

Testing

@cphurley82 cphurley82 marked this pull request as ready for review November 21, 2025 19:02
@churley-qcom
update CI dependencies to enable act
d9944f4 
- act allows you to run GitHub Actions locally but requires minor changes to the workflow
@churley-qcom
.gitignore coan generated files
ad6167d 
- when running locally conan creates files that should be ignored by git
@churley-qcom
Add memory boundary test
c38c577 
- Added unit tests to CI workflow.
- Added Dockerfile for testing environment setup.
- Created `TESTING.md` for guidelines on running tests.
- Implemented memory boundary test in `tests/memory_boundary_test.cpp` with CMake configuration.
@churley-qcom
Fix memory handling for page boundary operations in handle_operation
482e736
@cphurley82 cphurley82 force-pushed the fix-reading-across-page-boundary branch from 66dd4f2 to 482e736 Compare December 3, 2025 01:02
@cphurley82
Copy link
Author

cphurley82 commented Dec 3, 2025

@eyck, can you please review this? The coan change tripped up my rebase so I'd like to get it in sooner rather than later.

@eyck
Copy link
Contributor

eyck commented Dec 3, 2025

I will have a look later this week...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cphurley82 @eyck @churley-qcom