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.

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

Add 64-bit offset support for std::io traits #333

Closed
ncpenke opened this issue Nov 5, 2022 · 4 comments · Fixed by #335
Closed

Add 64-bit offset support for std::io traits #333

ncpenke opened this issue Nov 5, 2022 · 4 comments · Fixed by #335

Comments

@ncpenke
Copy link
Contributor

ncpenke commented Nov 5, 2022

To make effective use of larger stable storage size, it would be useful to manipulate stable storage via std::io::Read and std::io::Write traits.

@ncpenke
Copy link
Contributor Author

ncpenke commented Nov 5, 2022

This commit fixes this by introducing a new StableIO structure that implements both std::io::Read, and std::io::Write. The existing logic for StableWriter and StableReader is moved under this structure, andStableWriter and StableReader are reimplemented as wrappers around StableIO.

The motivation for this approach was to allow backwards compatibility with existing API, while layering the new functionality/API on top.

This also fixes #334.

If you're interested, I can submit a PR.

@ncpenke
Copy link
Contributor Author

ncpenke commented Nov 5, 2022

A further iteration was made by my colleague dmidem@653892c to improve the readability by splitting logic into several files.

In general, happy to make the appropriate cleanup/changes if this functionality seems relevant to cdk-rs.

@lwshang
Copy link
Contributor

lwshang commented Nov 8, 2022

Thanks for the suggestion. Please file a PR.

@ncpenke
Copy link
Contributor Author

ncpenke commented Nov 8, 2022

Thanks @lwshang created a PR. Happy to incorporate any feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants