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

Make madvise_random compatible with non-Unix OS #221

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Make madvise_random compatible with non-Unix OS #221

merged 1 commit into from
Sep 5, 2023

Conversation

shamilsan
Copy link
Contributor

Fixes #220.

@arkpar arkpar merged commit f537334 into paritytech:master Sep 5, 2023
7 checks passed
@nazar-pc
Copy link
Contributor

@arkpar could you release this as a new version? Just hit this in Windows CI. Also would be great to add macOS and Windows to CI here.

@arkpar
Copy link
Member

arkpar commented Sep 13, 2023

published as v0.4.11

@nazar-pc
Copy link
Contributor

nazar-pc commented Sep 26, 2023 •
edited
Loading

Now I'm curious why not using https://docs.rs/memmap2/latest/memmap2/struct.Advice.html with .advise() methods on with memmap2? It does the same thing internally, but spared you from using unsafe libc::madvise (and takes care of alignment requirement for its argument).

@arkpar
Copy link
Member

arkpar commented Sep 26, 2023 •
edited
Loading

advise was not available in memmap2-rs back when this code was originally written. It makes sense to switch now indeed.

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

@arkpar arkpar arkpar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

madvise_random function in v0.4.10 has different signatures for Unix and non-Unix systems
3 participants
@shamilsan @nazar-pc @arkpar