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 sendfile(2) for DragonFly #1615

Merged
merged 1 commit into from Jan 4, 2022
Merged

Conversation

rtzoeller
Copy link
Collaborator

The code is copied from the Mac OS and FreeBSD implementations.

@rtzoeller rtzoeller marked this pull request as ready for review December 24, 2021 18:13
Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

The signatures in libc look the same. Is the only difference that none of the flags are supported on Dragonfly?

@rtzoeller
Copy link
Collaborator Author

The primary difference is the flags field (which FreeBSD actually packs flags and an int into). From the DragonFly man page:

The flags argument is currently undefined and should be specified as 0.

There are also some documentation differences, notably that FreeBSD supports reading from a shared memory object, while the DragonFly man page indicates it must be a regular file.

@asomers
Copy link
Member

asomers commented Dec 24, 2021

Just for consistency's sake, do you think it would make sense to define the API identically on FreeBSD and Dragonfly, but require that the flags field be zero on Dragonfly? That would make it easier to port programs from FreeBSD to Dragonfly. OTOH, it's not very hard with the current API either. I'm kind of on the fence about it.

@rtzoeller
Copy link
Collaborator Author

I considered it, but my reservation is DragonFly doesn't currently assign meaning to the flags parameter other than requiring that it must be zero. It could be repurposed to something other than a flags bitfield while maintaining ABI compatibility. Consolidating them would also require passing zero for the readahead parameter, which doesn't currently apply to DragonFly (and may never).

Given other targets like Linux have different signatures already, I think keeping it separate is probably (?) the best idea for now, while acknowledging they may be consolidated in the future.

We can defer merging this until after some of the other PRs are submitted, to avoid them needing to rebased yet again for changelog reasons. I'd rather rebase than make someone else need to, and it gives us time to change our minds on the implementation.

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

Sounds good.

@rtzoeller rtzoeller added this to the 0.24.0 milestone Dec 24, 2021
@rtzoeller rtzoeller force-pushed the dfly_sendfile branch 2 times, most recently from c6cc22e to 4d5c090 Compare January 4, 2022 02:22
@rtzoeller
Copy link
Collaborator Author

bors r+

@bors bors bot merged commit f6268d9 into nix-rust:master Jan 4, 2022
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.

None yet

2 participants