-
Notifications
You must be signed in to change notification settings - Fork 252
Do not use windows-sys types in public API #576
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -118,3 +118,28 @@ jobs: | |
- uses: dtolnay/rust-toolchain@stable | ||
- name: Run Clippy | ||
run: cargo clippy --all-targets --all-features -- -D warnings | ||
CheckExternalTypes: | ||
name: check-external-types (${{ matrix.os }}) | ||
runs-on: ${{ matrix.os }} | ||
strategy: | ||
matrix: | ||
os: | ||
- windows-latest | ||
- ubuntu-latest | ||
rust: | ||
# `check-external-types` requires a specific Rust nightly version. See | ||
# the README for details: https://github.com/awslabs/cargo-check-external-types | ||
- nightly-2024-06-30 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The readme mentions There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like the releases page has the recent version: https://github.com/awslabs/cargo-check-external-types/releases. |
||
steps: | ||
- uses: actions/checkout@v4 | ||
- name: Install Rust ${{ matrix.rust }} | ||
uses: dtolnay/rust-toolchain@stable | ||
with: | ||
toolchain: ${{ matrix.rust }} | ||
- name: Install cargo-check-external-types | ||
uses: taiki-e/cache-cargo-install-action@v1 | ||
with: | ||
tool: [email protected] | ||
locked: true | ||
- name: check-external-types | ||
run: cargo check-external-types --all-features |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,71 @@ | ||
use std::hash::Hash; | ||
use std::mem::{self, size_of, MaybeUninit}; | ||
use std::mem::{self, size_of}; | ||
use std::net::{SocketAddr, SocketAddrV4, SocketAddrV6}; | ||
use std::path::Path; | ||
use std::{fmt, io, ptr}; | ||
|
||
#[cfg(windows)] | ||
use windows_sys::Win32::Networking::WinSock::SOCKADDR_IN6_0; | ||
|
||
use crate::sys::{ | ||
c_int, sa_family_t, sockaddr, sockaddr_in, sockaddr_in6, sockaddr_storage, socklen_t, AF_INET, | ||
AF_INET6, AF_UNIX, | ||
}; | ||
use crate::sys::{c_int, sockaddr_in, sockaddr_in6, sockaddr_storage, AF_INET, AF_INET6, AF_UNIX}; | ||
use crate::Domain; | ||
|
||
/// The integer type used with `getsockname` on this platform. | ||
#[allow(non_camel_case_types)] | ||
pub type socklen_t = crate::sys::socklen_t; | ||
|
||
/// The integer type for the `ss_family` field on this platform. | ||
#[allow(non_camel_case_types)] | ||
pub type sa_family_t = crate::sys::sa_family_t; | ||
|
||
/// Rust version of the [`sockaddr_storage`] type. | ||
/// | ||
/// This type is intended to be used with with direct calls to the `getsockname` syscall. See the | ||
/// documentation of [`SockAddr::new`] for examples. | ||
/// | ||
/// This crate defines its own `sockaddr_storage` type to avoid semver concerns with upgrading | ||
/// `windows-sys`. | ||
#[repr(transparent)] | ||
pub struct SockAddrStorage { | ||
storage: sockaddr_storage, | ||
} | ||
|
||
impl SockAddrStorage { | ||
/// Construct a new storage containing all zeros. | ||
#[inline] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need these |
||
pub fn zeroed() -> Self { | ||
// SAFETY: All zeros is valid for this type. | ||
unsafe { mem::zeroed() } | ||
} | ||
|
||
/// Returns the size of this storage. | ||
#[inline] | ||
pub fn size_of(&self) -> socklen_t { | ||
size_of::<Self>() as socklen_t | ||
} | ||
|
||
/// View this type as another type. | ||
/// | ||
/// # Safety | ||
/// | ||
/// The type `T` must be one of the `sockaddr_*` types defined by this platform. | ||
#[inline] | ||
pub unsafe fn view_as<T>(&mut self) -> &mut T { | ||
assert!(size_of::<T>() <= size_of::<Self>()); | ||
// SAFETY: This type is repr(transparent) over `sockaddr_storage` and `T` is one of the | ||
// `sockaddr_*` types defined by this platform. | ||
&mut *(self as *mut Self as *mut T) | ||
} | ||
} | ||
|
||
impl std::fmt::Debug for SockAddrStorage { | ||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
f.debug_struct("sockaddr_storage") | ||
.field("ss_family", &self.storage.ss_family) | ||
.finish_non_exhaustive() | ||
} | ||
} | ||
|
||
/// The address of a socket. | ||
/// | ||
/// `SockAddr`s may be constructed directly to and from the standard library | ||
|
@@ -40,23 +93,22 @@ impl SockAddr { | |
/// # fn main() -> std::io::Result<()> { | ||
/// # #[cfg(unix)] { | ||
/// use std::io; | ||
/// use std::mem; | ||
/// use std::os::unix::io::AsRawFd; | ||
/// | ||
/// use socket2::{SockAddr, Socket, Domain, Type}; | ||
/// use socket2::{SockAddr, SockAddrStorage, Socket, Domain, Type}; | ||
/// | ||
/// let socket = Socket::new(Domain::IPV4, Type::STREAM, None)?; | ||
/// | ||
/// // Initialise a `SocketAddr` byte calling `getsockname(2)`. | ||
/// let mut addr_storage: libc::sockaddr_storage = unsafe { mem::zeroed() }; | ||
/// let mut len = mem::size_of_val(&addr_storage) as libc::socklen_t; | ||
/// let mut addr_storage = SockAddrStorage::zeroed(); | ||
/// let mut len = addr_storage.size_of(); | ||
/// | ||
/// // The `getsockname(2)` system call will intiliase `storage` for | ||
/// // us, setting `len` to the correct length. | ||
/// let res = unsafe { | ||
/// libc::getsockname( | ||
/// socket.as_raw_fd(), | ||
/// (&mut addr_storage as *mut libc::sockaddr_storage).cast(), | ||
/// addr_storage.view_as(), | ||
/// &mut len, | ||
/// ) | ||
/// }; | ||
|
@@ -70,8 +122,11 @@ impl SockAddr { | |
/// # Ok(()) | ||
/// # } | ||
/// ``` | ||
pub const unsafe fn new(storage: sockaddr_storage, len: socklen_t) -> SockAddr { | ||
SockAddr { storage, len } | ||
pub const unsafe fn new(storage: SockAddrStorage, len: socklen_t) -> SockAddr { | ||
SockAddr { | ||
storage: storage.storage, | ||
len: len as socklen_t, | ||
} | ||
} | ||
|
||
/// Initialise a `SockAddr` by calling the function `init`. | ||
|
@@ -121,25 +176,19 @@ impl SockAddr { | |
/// ``` | ||
pub unsafe fn try_init<F, T>(init: F) -> io::Result<(T, SockAddr)> | ||
where | ||
F: FnOnce(*mut sockaddr_storage, *mut socklen_t) -> io::Result<T>, | ||
F: FnOnce(*mut SockAddrStorage, *mut socklen_t) -> io::Result<T>, | ||
{ | ||
const STORAGE_SIZE: socklen_t = size_of::<sockaddr_storage>() as socklen_t; | ||
// NOTE: `SockAddr::unix` depends on the storage being zeroed before | ||
// calling `init`. | ||
// NOTE: calling `recvfrom` with an empty buffer also depends on the | ||
// storage being zeroed before calling `init` as the OS might not | ||
// initialise it. | ||
let mut storage = MaybeUninit::<sockaddr_storage>::zeroed(); | ||
let mut storage = SockAddrStorage::zeroed(); | ||
let mut len = STORAGE_SIZE; | ||
init(storage.as_mut_ptr(), &mut len).map(|res| { | ||
init(&mut storage, &mut len).map(|res| { | ||
debug_assert!(len <= STORAGE_SIZE, "overflown address storage"); | ||
let addr = SockAddr { | ||
// Safety: zeroed-out `sockaddr_storage` is valid, caller must | ||
// ensure at least `len` bytes are valid. | ||
storage: storage.assume_init(), | ||
len, | ||
}; | ||
(res, addr) | ||
(res, SockAddr::new(storage, len)) | ||
}) | ||
} | ||
|
||
|
@@ -179,13 +228,15 @@ impl SockAddr { | |
} | ||
|
||
/// Returns a raw pointer to the address. | ||
pub const fn as_ptr(&self) -> *const sockaddr { | ||
ptr::addr_of!(self.storage).cast() | ||
pub const fn as_ptr(&self) -> *const SockAddrStorage { | ||
&self.storage as *const sockaddr_storage as *const SockAddrStorage | ||
} | ||
|
||
/// Retuns the address as the storage. | ||
pub const fn as_storage(self) -> sockaddr_storage { | ||
self.storage | ||
pub const fn as_storage(self) -> SockAddrStorage { | ||
SockAddrStorage { | ||
storage: self.storage, | ||
} | ||
} | ||
|
||
/// Returns true if this address is in the `AF_INET` (IPv4) family, false otherwise. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to run it on different OS? Can we do cross-OS checking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cross-OS checking doesn't seem to work (well),
-Zbuild-std
can't be passed.