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

Update the documentation for {As,Into,From}Raw{Fd,Handle,Socket}. #93562

Merged
merged 10 commits into from
Mar 3, 2022

Conversation

sunfishcode
Copy link
Member

@sunfishcode sunfishcode commented Feb 1, 2022

This change weakens the descriptions of the
{as,into,from}_raw_{fd,handle,socket} descriptions from saying that
they do express ownership relations to say that they are typically used
in ways that express ownership relations. This is needed since, for
example, std's own RawFd implements {As,From,Into}Fd without any of
the ownership relationships.

This adds proper # Safety comments to from_raw_{fd,handle,socket},
adds the requirement that raw handles be not opened with the
FILE_FLAG_OVERLAPPED flag, and merges the OwnedHandle::from_raw_handle
comment into the main FromRawHandle::from_raw_handle comment.

And, this changes HandleOrNull and HandleOrInvalid to not implement
FromRawHandle, since they are intended for limited use in FFI situations,
and not for generic use, and they have constraints that are stronger than
the those of FromRawHandle.

This change weakens the descriptions of the
`{as,into,from}_raw_{fd,handle,socket}` descriptions from saying that
they *do* express ownership relations to say that they are *typically used*
in ways that express ownership relations. This needed needed since, for
example, std's own [`RawFd`] implements `{As,From,Into}Fd` without any of
the ownership relationships.

This adds proper `# Safety` comments to `from_raw_{fd,handle,socket}`,
adds the requirement that raw handles be not opened with the
`FILE_FLAG_OVERLAPPED` flag, and merges the `OwnedHandle::from_raw_handle`
comment into the main `FromRawHandle::from_raw_handle` comment.

And, this changes `HandleOrNull` and `HandleOrInvalid` to not implement
`FromRawHandle`, since they are intended for limited use in FFI situations,
and not for generic use, and they have constraints that are stronger than
the those of `FromRawHandle`.

[`RawFd`]: https://doc.rust-lang.org/stable/std/os/unix/io/type.RawFd.html
@rust-highfive
Copy link
Collaborator

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 1, 2022
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

This function is documented in more detail in the `FromRawSocket` trait.
sunfishcode added a commit to sunfishcode/io-lifetimes that referenced this pull request Feb 3, 2022
This ports the changes proposed in rust-lang/rust#93562 to io-lifetimes.

Specifically, remove the redundant comments on
`OwnedHandle::from_raw_handle` and `OwnedSocket::from_raw_socket`, and
remove the `FromRawHandle` impls for `HandleOrInvalid` and `HandleOrNull`.
@joshtriplett
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 2, 2022

📌 Commit ba6050f has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2022
sunfishcode added a commit to sunfishcode/io-lifetimes that referenced this pull request Mar 2, 2022
This ports the changes proposed in rust-lang/rust#93562 to io-lifetimes.

Specifically, remove the redundant comments on
`OwnedHandle::from_raw_handle` and `OwnedSocket::from_raw_socket`, and
remove the `FromRawHandle` impls for `HandleOrInvalid` and `HandleOrNull`.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…joshtriplett

Update the documentation for `{As,Into,From}Raw{Fd,Handle,Socket}`.

This change weakens the descriptions of the
`{as,into,from}_raw_{fd,handle,socket}` descriptions from saying that
they *do* express ownership relations to say that they are *typically used*
in ways that express ownership relations. This is needed since, for
example, std's own [`RawFd`] implements `{As,From,Into}Fd` without any of
the ownership relationships.

This adds proper `# Safety` comments to `from_raw_{fd,handle,socket}`,
adds the requirement that raw handles be not opened with the
`FILE_FLAG_OVERLAPPED` flag, and merges the `OwnedHandle::from_raw_handle`
comment into the main `FromRawHandle::from_raw_handle` comment.

And, this changes `HandleOrNull` and `HandleOrInvalid` to not implement
`FromRawHandle`, since they are intended for limited use in FFI situations,
and not for generic use, and they have constraints that are stronger than
the those of `FromRawHandle`.

[`RawFd`]: https://doc.rust-lang.org/stable/std/os/unix/io/type.RawFd.html
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…joshtriplett

Update the documentation for `{As,Into,From}Raw{Fd,Handle,Socket}`.

This change weakens the descriptions of the
`{as,into,from}_raw_{fd,handle,socket}` descriptions from saying that
they *do* express ownership relations to say that they are *typically used*
in ways that express ownership relations. This is needed since, for
example, std's own [`RawFd`] implements `{As,From,Into}Fd` without any of
the ownership relationships.

This adds proper `# Safety` comments to `from_raw_{fd,handle,socket}`,
adds the requirement that raw handles be not opened with the
`FILE_FLAG_OVERLAPPED` flag, and merges the `OwnedHandle::from_raw_handle`
comment into the main `FromRawHandle::from_raw_handle` comment.

And, this changes `HandleOrNull` and `HandleOrInvalid` to not implement
`FromRawHandle`, since they are intended for limited use in FFI situations,
and not for generic use, and they have constraints that are stronger than
the those of `FromRawHandle`.

[`RawFd`]: https://doc.rust-lang.org/stable/std/os/unix/io/type.RawFd.html
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 2, 2022
…joshtriplett

Update the documentation for `{As,Into,From}Raw{Fd,Handle,Socket}`.

This change weakens the descriptions of the
`{as,into,from}_raw_{fd,handle,socket}` descriptions from saying that
they *do* express ownership relations to say that they are *typically used*
in ways that express ownership relations. This is needed since, for
example, std's own [`RawFd`] implements `{As,From,Into}Fd` without any of
the ownership relationships.

This adds proper `# Safety` comments to `from_raw_{fd,handle,socket}`,
adds the requirement that raw handles be not opened with the
`FILE_FLAG_OVERLAPPED` flag, and merges the `OwnedHandle::from_raw_handle`
comment into the main `FromRawHandle::from_raw_handle` comment.

And, this changes `HandleOrNull` and `HandleOrInvalid` to not implement
`FromRawHandle`, since they are intended for limited use in FFI situations,
and not for generic use, and they have constraints that are stronger than
the those of `FromRawHandle`.

[`RawFd`]: https://doc.rust-lang.org/stable/std/os/unix/io/type.RawFd.html
@matthiaskrgr
Copy link
Member

Looks like this failed in a rollup: #94533 (comment)
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 2, 2022
@sunfishcode
Copy link
Member Author

I've now added a patch which fixes the broken doc link.

r? @joshtriplett

@sunfishcode sunfishcode added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 2, 2022
@jstarks
Copy link

jstarks commented Mar 2, 2022

Was there discussion somewhere about why FILE_FLAG_OVERLAPPED is prohibited?

There may eventually be something to say about `FILE_FLAG_OVERLAPPED` here,
however this appears to be independent of the other changes in this PR,
so remove them from this PR so that it can be discussed separately.
@joshtriplett
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Mar 3, 2022

‼️ Invalid head SHA found, retrying: 0000000000000000000000000000000000000000

@bors
Copy link
Contributor

bors commented Mar 3, 2022

📌 Commit 8253cfe has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2022
@sunfishcode
Copy link
Member Author

There was not; it came up from some private discussions and I had the impression it was a simple omission that would be nice to fix while I was making changes in this area.

Looking into it more now, I've now found #81357 on this topic, with non-trivial considerations. In light of that, I've now removed the mention of FILE_FLAG_OVERLAPPED from this PR, since the other changes don't depend on it.

bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2022
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#93562 (Update the documentation for `{As,Into,From}Raw{Fd,Handle,Socket}`.)
 - rust-lang#94101 (rustdoc: add test cases for hidden enum variants)
 - rust-lang#94484 (8 - Make more use of `let_chains`)
 - rust-lang#94522 (Remove out-of-context line at end of E0284 message)
 - rust-lang#94534 (Re-export (unstable) core::ffi types from std::ffi)
 - rust-lang#94536 (Move transmute_undefined_repr back to nursery again)
 - rust-lang#94537 (Use ? operator in one instance instead of manual match)
 - rust-lang#94544 (Add some examples to comments in mbe code)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit afd6f5c into rust-lang:master Mar 3, 2022
@rustbot rustbot added this to the 1.61.0 milestone Mar 3, 2022
@sunfishcode sunfishcode deleted the sunfishcode/io-docs branch March 3, 2022 13:40
@jstarks
Copy link

jstarks commented Mar 3, 2022

OK, great, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants