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 mmap_anonymous function #2127

Merged
merged 4 commits into from
Nov 5, 2023
Merged

Add mmap_anonymous function #2127

merged 4 commits into from
Nov 5, 2023

Conversation

newpavlov
Copy link
Contributor

Closes #2126

@newpavlov newpavlov changed the title Add mmap_nofd function Add mmap_anonymous function Oct 2, 2023
src/sys/mman.rs Outdated Show resolved Hide resolved
src/sys/mman.rs Show resolved Hide resolved
@SteveLauC
Copy link
Member

CI failure seems to be unrelated to this PR


Probably two change logs need to be added:

### Changed
- `mmap()` no longer takes optional file descriptor as there is a dedicated interface `mmap_anonymous()` for that
   ([#2127](https://github.com/nix-rust/nix/pull/2127))

### Added
- A dedicated interface `mmap_anonymous()` for anonymous mmap has been added
   ([#2127](https://github.com/nix-rust/nix/pull/2127))

@SteveLauC SteveLauC self-assigned this Oct 2, 2023
@asomers
Copy link
Member

asomers commented Oct 2, 2023

The CI failure is fixed on master. You need to rebase.

@newpavlov newpavlov force-pushed the mmap_nofd branch 4 times, most recently from e32be67 to 8c605db Compare October 2, 2023 09:27
@SteveLauC
Copy link
Member

Code LGTM, still, change logs are needed

@SteveLauC
Copy link
Member

Friendly ping @newpavlov, could you please add change logs for this PR? We have changed the CHANGELOG mode in #2149, please see CONTRIBUTING.md on how to add change logs

@newpavlov
Copy link
Contributor Author

@SteveLauC
Done. I've reverted the mmap changes since they are breaking and additional changelog note (I am not sure if two changelog notes can be added for one PR). I think it's worth to eventually replace Option<F> with just F, but it can wait until next breaking release.

@SteveLauC
Copy link
Member

SteveLauC commented Nov 5, 2023

Done.

Thanks for doing this!

I've reverted the mmap changes since they are breaking

Breaking changes are allowed as we will bump our minor version number in the next release

and additional changelog note (I am not sure if two changelog notes can be added for one PR).

I just tested that we can have two change logs with the same PR number in the current mode, so go ahead:) I will document this in CONTRIBUTING.md tomorrow

@SteveLauC SteveLauC added this pull request to the merge queue Nov 5, 2023
Merged via the queue into nix-rust:master with commit a388118 Nov 5, 2023
35 checks passed
@SteveLauC SteveLauC mentioned this pull request Feb 15, 2024
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 this pull request may close these issues.

Introduce mmap_nofd to work around <F: AsFd>?
3 participants