programs: ProgramFd is owned
#744
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hey @alessandrod, this pull request changes the Aya Public API and requires your review. |
mergify
bot
added
api/needs-review
tamird
force-pushed
the
programfd-borrowed
branch
from
bb6c27b
to
4350519
Compare
✅ Deploy Preview for aya-rs-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
tamird
force-pushed
the
programfd-borrowed
branch
from
4350519
to
277ee18
Compare
|
@tamird, this pull request is now in conflict and requires a rebase. |
tamird
force-pushed
the
programfd-borrowed
branch
from
277ee18
to
5748193
Compare
nrxus
reviewed
Aug 11, 2023
|
@tamird, this pull request is now in conflict and requires a rebase. |
tamird
force-pushed
the
programfd-borrowed
branch
from
5748193
to
8717bf1
Compare
dave-tucker
requested changes
Aug 11, 2023
tamird
force-pushed
the
programfd-borrowed
branch
2 times, most recently
from
35d85e5
to
db61441
Compare
tamird
force-pushed
the
programfd-borrowed
branch
from
db61441
to
5def006
Compare
tamird
changed the title
ProgramFd is borrowed from the programProgramFd is owned
tamird
force-pushed
the
programfd-borrowed
branch
from
5def006
to
ffb2e9c
Compare
nrxus
reviewed
Aug 12, 2023
tamird
force-pushed
the
programfd-borrowed
branch
2 times, most recently
from
69d2afc
to
edd307c
Compare
alessandrod
reviewed
Aug 14, 2023
| @@ -174,7 +176,9 @@ impl Xdp { | |||
| /// | |||
| /// Ownership of the link will transfer to this program. | |||
| pub fn attach_to_link(&mut self, link: XdpLink) -> Result<XdpLinkId, ProgramError> { | |||
| let prog_fd = self.data.fd_or_err()?; | |||
| let prog_fd = self.fd()?; | |||
There was a problem hiding this comment.
Here and everywhere else, why not do:
let prog_fd = self.fd()?.as_fd().as_raw_fd();
There was a problem hiding this comment.
Just personal preference - I prefer to avoid ? in the middle of an expression. This will also make the diff cleaner when we get around to removing the need for as_raw_fd; it'll be a pure deletion.
|
@tamird, this pull request is now in conflict and requires a rebase. |
alessandrod
approved these changes
Aug 14, 2023
tamird
force-pushed
the
programfd-borrowed
branch
from
edd307c
to
e3dccba
Compare
|
Will merge after @dave-tucker has a chance to approve. |
tamird
force-pushed
the
programfd-borrowed
branch
from
e3dccba
to
e6afab9
Compare
`ProgramData::fd` is now a `ProgramFd`. This means that `ProgramData` now closes the file descriptor on drop. In the future we might consider making `ProgramFd` hold a `BorrowedFd` but this requires API design work due to overlapping borrows. Since `ProgramFd` is no longer `Copy`, update methods to take it by reference to allow callers to use it multiple times as they are accustomed to doing. `ProgramFd` is now returned by reference and implements `try_clone` to allow callers to avoid file descriptor cloning when desired. This is an API breaking change. Updates #612.
tamird
force-pushed
the
programfd-borrowed
branch
from
e6afab9
to
504fd1d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ProgramData::fdis now also owned. This means thatProgramDatanowcloses the file descriptor on drop. In the future we might consider
making
ProgramFdhold aBorrowedFdbut this requires API design workdue to overlapping borrows.
Since
ProgramFdis no longerCopy, update methods to take it byreference to allow callers to use it multiple times as they are
accustomed to doing.
This is an API breaking change.
Updates #612.