about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2022-03-03 11:02:49 +0100
committerGitHub <noreply@github.com>2022-03-03 11:02:49 +0100
commitafd6f5c47808c7676014e88d0fda82e95c3a9da0 (patch)
tree5b241d5acad5589b2b914da331a189ec81ccff37
parent2f8d1a835b4e7feaf625f74d0d5cb9b84dbc845a (diff)
parent8253cfef7aa346c894259471e1e38a34df65cb58 (diff)
Rollup merge of #93562 - sunfishcode:sunfishcode/io-docs, r=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
-rw-r--r--library/std/src/os/fd/raw.rs42
-rw-r--r--library/std/src/os/windows/io/handle.rs38
-rw-r--r--library/std/src/os/windows/io/raw.rs96
-rw-r--r--library/std/src/os/windows/io/socket.rs7
4 files changed, 111 insertions, 72 deletions
diff --git a/library/std/src/os/fd/raw.rs b/library/std/src/os/fd/raw.rs
index f874cf0b42d..f9c883dd6bf 100644
--- a/library/std/src/os/fd/raw.rs
+++ b/library/std/src/os/fd/raw.rs
@@ -5,6 +5,8 @@
 use crate::fs;
 use crate::io;
 use crate::os::raw;
+#[cfg(doc)]
+use crate::os::unix::io::AsFd;
 #[cfg(unix)]
 use crate::os::unix::io::OwnedFd;
 #[cfg(target_os = "wasi")]
@@ -24,9 +26,14 @@ pub type RawFd = raw::c_int;
 pub trait AsRawFd {
     /// Extracts the raw file descriptor.
     ///
-    /// This method does **not** pass ownership of the raw file descriptor
-    /// to the caller. The descriptor is only guaranteed to be valid while
-    /// the original object has not yet been destroyed.
+    /// This function is typically used to **borrow** an owned file descriptor.
+    /// When used in this way, this method does **not** pass ownership of the
+    /// raw file descriptor to the caller, and the file descriptor is only
+    /// guaranteed to be valid while the original object has not yet been
+    /// destroyed.
+    ///
+    /// However, borrowing is not strictly required. See [`AsFd::as_fd`]
+    /// for an API which strictly borrows a file descriptor.
     ///
     /// # Example
     ///
@@ -55,15 +62,18 @@ pub trait FromRawFd {
     /// Constructs a new instance of `Self` from the given raw file
     /// descriptor.
     ///
-    /// This function **consumes ownership** of the specified file
-    /// descriptor. The returned object will take responsibility for closing
-    /// it when the object goes out of scope.
+    /// This function is typically used to **consume ownership** of the
+    /// specified file descriptor. When used in this way, the returned object
+    /// will take responsibility for closing it when the object goes out of
+    /// scope.
+    ///
+    /// However, consuming ownership is not strictly required. Use a
+    /// [`From<OwnedFd>::from`] implementation for an API which strictly
+    /// consumes ownership.
     ///
-    /// This function is also unsafe as the primitives currently returned
-    /// have the contract that they are the sole owner of the file
-    /// descriptor they are wrapping. Usage of this function could
-    /// accidentally allow violating this contract which can cause memory
-    /// unsafety in code that relies on it being true.
+    /// # Safety
+    ///
+    /// The `fd` passed in must be a valid an open file descriptor.
     ///
     /// # Example
     ///
@@ -94,9 +104,13 @@ pub trait FromRawFd {
 pub trait IntoRawFd {
     /// Consumes this object, returning the raw underlying file descriptor.
     ///
-    /// This function **transfers ownership** of the underlying file descriptor
-    /// to the caller. Callers are then the unique owners of the file descriptor
-    /// and must close the descriptor once it's no longer needed.
+    /// This function is typically used to **transfer ownership** of the underlying
+    /// file descriptor to the caller. When used in this way, callers are then the unique
+    /// owners of the file descriptor and must close it once it's no longer needed.
+    ///
+    /// However, transferring ownership is not strictly required. Use a
+    /// [`Into<OwnedFd>::into`] implementation for an API which strictly
+    /// transfers ownership.
     ///
     /// # Example
     ///
diff --git a/library/std/src/os/windows/io/handle.rs b/library/std/src/os/windows/io/handle.rs
index 842a15ae1bf..d378d591ba3 100644
--- a/library/std/src/os/windows/io/handle.rs
+++ b/library/std/src/os/windows/io/handle.rs
@@ -214,29 +214,13 @@ impl IntoRawHandle for OwnedHandle {
 }
 
 impl FromRawHandle for OwnedHandle {
-    /// Constructs a new instance of `Self` from the given raw handle.
-    ///
-    /// # Safety
-    ///
-    /// The resource pointed to by `handle` must be open and suitable for
-    /// assuming ownership. The resource must not require any cleanup other
-    /// than `CloseHandle`.
-    ///
-    /// In particular, it must not be used with handles to open registry
-    /// keys which need to be closed with [`RegCloseKey`] instead.
-    ///
-    /// Note that it *may* have the value `INVALID_HANDLE_VALUE` (-1), which is
-    /// sometimes a valid handle value. See [here] for the full story.
-    ///
-    /// [`RegCloseKey`]: https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regclosekey
-    /// [here]: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40443
     #[inline]
     unsafe fn from_raw_handle(handle: RawHandle) -> Self {
         Self { handle }
     }
 }
 
-impl FromRawHandle for HandleOrNull {
+impl HandleOrNull {
     /// Constructs a new instance of `Self` from the given `RawHandle` returned
     /// from a Windows API that uses null to indicate failure, such as
     /// `CreateThread`.
@@ -246,18 +230,18 @@ impl FromRawHandle for HandleOrNull {
     ///
     /// # Safety
     ///
-    /// The resource pointed to by `handle` must be either open and otherwise
-    /// unowned, or null. Note that not all Windows APIs use null for errors;
-    /// see [here] for the full story.
+    /// The passed `handle` value must either satisfy the safety requirements
+    /// of [`FromRawHandle::from_raw_handle`], or be null. Note that not all
+    /// Windows APIs use null for errors; see [here] for the full story.
     ///
     /// [here]: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40443
     #[inline]
-    unsafe fn from_raw_handle(handle: RawHandle) -> Self {
+    pub unsafe fn from_raw_handle(handle: RawHandle) -> Self {
         Self(OwnedHandle::from_raw_handle(handle))
     }
 }
 
-impl FromRawHandle for HandleOrInvalid {
+impl HandleOrInvalid {
     /// Constructs a new instance of `Self` from the given `RawHandle` returned
     /// from a Windows API that uses `INVALID_HANDLE_VALUE` to indicate
     /// failure, such as `CreateFileW`.
@@ -267,14 +251,14 @@ impl FromRawHandle for HandleOrInvalid {
     ///
     /// # Safety
     ///
-    /// The resource pointed to by `handle` must be either open and otherwise
-    /// unowned, null, or equal to `INVALID_HANDLE_VALUE` (-1). Note that not
-    /// all Windows APIs use `INVALID_HANDLE_VALUE` for errors; see [here] for
-    /// the full story.
+    /// The passed `handle` value must either satisfy the safety requirements
+    /// of [`FromRawHandle::from_raw_handle`], or be
+    /// `INVALID_HANDLE_VALUE` (-1). Note that not all Windows APIs use
+    /// `INVALID_HANDLE_VALUE` for errors; see [here] for the full story.
     ///
     /// [here]: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40443
     #[inline]
-    unsafe fn from_raw_handle(handle: RawHandle) -> Self {
+    pub unsafe fn from_raw_handle(handle: RawHandle) -> Self {
         Self(OwnedHandle::from_raw_handle(handle))
     }
 }
diff --git a/library/std/src/os/windows/io/raw.rs b/library/std/src/os/windows/io/raw.rs
index c7f122048a1..48c5fd358d9 100644
--- a/library/std/src/os/windows/io/raw.rs
+++ b/library/std/src/os/windows/io/raw.rs
@@ -5,6 +5,8 @@
 use crate::fs;
 use crate::io;
 use crate::net;
+#[cfg(doc)]
+use crate::os::windows::io::{AsHandle, AsSocket};
 use crate::os::windows::io::{OwnedHandle, OwnedSocket};
 use crate::os::windows::raw;
 use crate::sys;
@@ -22,7 +24,15 @@ pub type RawSocket = raw::SOCKET;
 /// Extracts raw handles.
 #[stable(feature = "rust1", since = "1.0.0")]
 pub trait AsRawHandle {
-    /// Extracts the raw handle, without taking any ownership.
+    /// Extracts the raw handle.
+    ///
+    /// This function is typically used to **borrow** an owned handle.
+    /// When used in this way, this method does **not** pass ownership of the
+    /// raw handle to the caller, and the handle is only guaranteed
+    /// to be valid while the original object has not yet been destroyed.
+    ///
+    /// However, borrowing is not strictly required. See [`AsHandle::as_handle`]
+    /// for an API which strictly borrows a handle.
     #[stable(feature = "rust1", since = "1.0.0")]
     fn as_raw_handle(&self) -> RawHandle;
 }
@@ -32,15 +42,28 @@ pub trait AsRawHandle {
 pub trait FromRawHandle {
     /// Constructs a new I/O object from the specified raw handle.
     ///
-    /// This function will **consume ownership** of the handle given,
-    /// passing responsibility for closing the handle to the returned
-    /// object.
+    /// This function is typically used to **consume ownership** of the handle
+    /// given, passing responsibility for closing the handle to the returned
+    /// object. When used in this way, the returned object
+    /// will take responsibility for closing it when the object goes out of
+    /// scope.
+    ///
+    /// However, consuming ownership is not strictly required. Use a
+    /// `From<OwnedHandle>::from` implementation for an API which strictly
+    /// consumes ownership.
+    ///
+    /// # Safety
     ///
-    /// This function is also unsafe as the primitives currently returned
-    /// have the contract that they are the sole owner of the file
-    /// descriptor they are wrapping. Usage of this function could
-    /// accidentally allow violating this contract which can cause memory
-    /// unsafety in code that relies on it being true.
+    /// The `handle` passed in must:
+    ///   - be a valid an open handle,
+    ///   - be a handle for a resource that may be freed via [`CloseHandle`]
+    ///     (as opposed to `RegCloseKey` or other close functions).
+    ///
+    /// Note that the handle *may* have the value `INVALID_HANDLE_VALUE` (-1),
+    /// which is sometimes a valid handle value. See [here] for the full story.
+    ///
+    /// [`CloseHandle`]: https://docs.microsoft.com/en-us/windows/win32/api/handleapi/nf-handleapi-closehandle
+    /// [here]: https://devblogs.microsoft.com/oldnewthing/20040302-00/?p=40443
     #[stable(feature = "from_raw_os", since = "1.1.0")]
     unsafe fn from_raw_handle(handle: RawHandle) -> Self;
 }
@@ -51,9 +74,13 @@ pub trait FromRawHandle {
 pub trait IntoRawHandle {
     /// Consumes this object, returning the raw underlying handle.
     ///
-    /// This function **transfers ownership** of the underlying handle to the
-    /// caller. Callers are then the unique owners of the handle and must close
-    /// it once it's no longer needed.
+    /// This function is typically used to **transfer ownership** of the underlying
+    /// handle to the caller. When used in this way, callers are then the unique
+    /// owners of the handle and must close it once it's no longer needed.
+    ///
+    /// However, transferring ownership is not strictly required. Use a
+    /// `Into<OwnedHandle>::into` implementation for an API which strictly
+    /// transfers ownership.
     #[stable(feature = "into_raw_os", since = "1.4.0")]
     fn into_raw_handle(self) -> RawHandle;
 }
@@ -130,7 +157,15 @@ impl IntoRawHandle for fs::File {
 /// Extracts raw sockets.
 #[stable(feature = "rust1", since = "1.0.0")]
 pub trait AsRawSocket {
-    /// Extracts the underlying raw socket from this object.
+    /// Extracts the raw socket.
+    ///
+    /// This function is typically used to **borrow** an owned socket.
+    /// When used in this way, this method does **not** pass ownership of the
+    /// raw socket to the caller, and the socket is only guaranteed
+    /// to be valid while the original object has not yet been destroyed.
+    ///
+    /// However, borrowing is not strictly required. See [`AsSocket::as_socket`]
+    /// for an API which strictly borrows a socket.
     #[stable(feature = "rust1", since = "1.0.0")]
     fn as_raw_socket(&self) -> RawSocket;
 }
@@ -138,16 +173,25 @@ pub trait AsRawSocket {
 /// Creates I/O objects from raw sockets.
 #[stable(feature = "from_raw_os", since = "1.1.0")]
 pub trait FromRawSocket {
-    /// Creates a new I/O object from the given raw socket.
+    /// Constructs a new I/O object from the specified raw socket.
+    ///
+    /// This function is typically used to **consume ownership** of the socket
+    /// given, passing responsibility for closing the socket to the returned
+    /// object. When used in this way, the returned object
+    /// will take responsibility for closing it when the object goes out of
+    /// scope.
     ///
-    /// This function will **consume ownership** of the socket provided and
-    /// it will be closed when the returned object goes out of scope.
+    /// However, consuming ownership is not strictly required. Use a
+    /// `From<OwnedSocket>::from` implementation for an API which strictly
+    /// consumes ownership.
     ///
-    /// This function is also unsafe as the primitives currently returned
-    /// have the contract that they are the sole owner of the file
-    /// descriptor they are wrapping. Usage of this function could
-    /// accidentally allow violating this contract which can cause memory
-    /// unsafety in code that relies on it being true.
+    /// # Safety
+    ///
+    /// The `socket` passed in must:
+    ///   - be a valid an open socket,
+    ///   - be a socket that may be freed via [`closesocket`].
+    ///
+    /// [`closesocket`]: https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-closesocket
     #[stable(feature = "from_raw_os", since = "1.1.0")]
     unsafe fn from_raw_socket(sock: RawSocket) -> Self;
 }
@@ -158,9 +202,13 @@ pub trait FromRawSocket {
 pub trait IntoRawSocket {
     /// Consumes this object, returning the raw underlying socket.
     ///
-    /// This function **transfers ownership** of the underlying socket to the
-    /// caller. Callers are then the unique owners of the socket and must close
-    /// it once it's no longer needed.
+    /// This function is typically used to **transfer ownership** of the underlying
+    /// socket to the caller. When used in this way, callers are then the unique
+    /// owners of the socket and must close it once it's no longer needed.
+    ///
+    /// However, transferring ownership is not strictly required. Use a
+    /// `Into<OwnedSocket>::into` implementation for an API which strictly
+    /// transfers ownership.
     #[stable(feature = "into_raw_os", since = "1.4.0")]
     fn into_raw_socket(self) -> RawSocket;
 }
diff --git a/library/std/src/os/windows/io/socket.rs b/library/std/src/os/windows/io/socket.rs
index a695a4106e8..a6b979cc22b 100644
--- a/library/std/src/os/windows/io/socket.rs
+++ b/library/std/src/os/windows/io/socket.rs
@@ -172,13 +172,6 @@ impl IntoRawSocket for OwnedSocket {
 }
 
 impl FromRawSocket for OwnedSocket {
-    /// Constructs a new instance of `Self` from the given raw socket.
-    ///
-    /// # Safety
-    ///
-    /// The resource pointed to by `socket` must be open and suitable for
-    /// assuming ownership. The resource must not require cleanup other than
-    /// `closesocket`.
     #[inline]
     unsafe fn from_raw_socket(socket: RawSocket) -> Self {
         debug_assert_ne!(socket, c::INVALID_SOCKET as RawSocket);