From 74895a863600abfcac0033f902d1eff3889f1029 Mon Sep 17 00:00:00 2001 From: Vincent Thomas Date: Fri, 24 Apr 2026 01:09:42 +0200 Subject: [PATCH] remove unsafe here and there --- lio/src/backend/impls/iocp.rs | 7 +-- lio/src/backend/impls/pollingv2/mod.rs | 70 +++++++++++---------- lio/src/backend/impls/pollingv2/os/epoll.rs | 5 +- lio/src/ffi.rs | 16 +++-- lio/src/fs/file.rs | 12 ++-- lio/src/macros.rs | 25 +++----- lio/src/process.rs | 17 ++--- 7 files changed, 78 insertions(+), 74 deletions(-) diff --git a/lio/src/backend/impls/iocp.rs b/lio/src/backend/impls/iocp.rs index a539500d..b69c8ca7 100644 --- a/lio/src/backend/impls/iocp.rs +++ b/lio/src/backend/impls/iocp.rs @@ -71,7 +71,7 @@ struct IocpOpState { impl IocpOpState { fn new(op_id: u64) -> Box { - Box::new(Self { overlapped: unsafe { std::mem::zeroed() }, op_id }) + Box::new(Self { overlapped: OVERLAPPED::default(), op_id }) } /// Get the OVERLAPPED pointer for this state. @@ -156,7 +156,7 @@ impl Iocp { /// Ensure WSA is initialized (for socket operations). fn ensure_wsa(&mut self) -> io::Result<()> { if !self.wsa_initialized { - let mut wsa_data: WSADATA = unsafe { std::mem::zeroed() }; + let mut wsa_data = WSADATA::default(); let result = unsafe { WSAStartup(0x0202, &mut wsa_data) }; if result != 0 { return Err(io::Error::from_raw_os_error(result)); @@ -362,8 +362,7 @@ impl Iocp { let handle = fd.as_raw_handle() as HANDLE; // Lock the entire file (0 to u64::MAX) - // SAFETY: Creating a zeroed OVERLAPPED is valid for LockFileEx - let mut overlapped: OVERLAPPED = unsafe { std::mem::zeroed() }; + let mut overlapped = OVERLAPPED::default(); if *operation & lock::LOCK_UN != 0 { // Unlock diff --git a/lio/src/backend/impls/pollingv2/mod.rs b/lio/src/backend/impls/pollingv2/mod.rs index 613dab06..37abbf66 100644 --- a/lio/src/backend/impls/pollingv2/mod.rs +++ b/lio/src/backend/impls/pollingv2/mod.rs @@ -246,19 +246,13 @@ impl Poller { ) -> io::Result { let mut created_here = false; let dir = if opaque.is_null() { - // SAFETY: `fd` is a live directory file descriptor supplied by the caller. - let dup_fd = unsafe { libc::dup(fd) }; - if dup_fd < 0 { - return Err(io::Error::last_os_error()); - } + let dup_fd = syscall!(dup(fd))?; + // SAFETY: `dup_fd` is a valid duplicated directory descriptor. let dir = unsafe { libc::fdopendir(dup_fd) }; if dir.is_null() { let err = io::Error::last_os_error(); - // SAFETY: `dup_fd` is still locally owned on this error path. - unsafe { - libc::close(dup_fd); - } + let _ = syscall!(close(dup_fd)); return Err(err); } *opaque = dir.cast(); @@ -341,23 +335,19 @@ impl Poller { match result { Ok(entries) => { if entries.eof { - // SAFETY: `dir` is an owned directory stream being closed exactly once. - let close_result = unsafe { libc::closedir(dir) }; + let close_result = syscall!(closedir(dir)); *opaque = std::ptr::null_mut(); *opaque_drop = None; - if close_result < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(entries) - } + + let _ = close_result?; + Ok(entries) } else { Ok(entries) } } Err(err) => { if !created_here || !dir.is_null() { - // SAFETY: `dir` is an owned directory stream being closed on error. - let _ = unsafe { libc::closedir(dir) }; + let _ = syscall!(raw closedir(dir)); *opaque = std::ptr::null_mut(); *opaque_drop = None; } @@ -430,8 +420,15 @@ impl Poller { None }; - // SAFETY: `msghdr` is POD and may be zero-initialized before field assignment. - let mut hdr: libc::msghdr = unsafe { std::mem::zeroed() }; + let mut hdr = libc::msghdr { + msg_name: std::ptr::null_mut(), + msg_namelen: 0, + msg_iov: std::ptr::null_mut(), + msg_iovlen: 0, + msg_control: std::ptr::null_mut(), + msg_controllen: 0, + msg_flags: 0, + }; hdr.msg_iov = iovecs.as_mut_ptr(); hdr.msg_iovlen = bufs.len() as _; if let Some((storage, len)) = addr.as_mut() { @@ -457,8 +454,15 @@ impl Poller { } let mut addr = msg.to.map(crate::backend::op::socket_addr_to_storage); - // SAFETY: `msghdr` is POD and may be zero-initialized before field assignment. - let mut hdr: libc::msghdr = unsafe { std::mem::zeroed() }; + let mut hdr = libc::msghdr { + msg_name: std::ptr::null_mut(), + msg_namelen: 0, + msg_iov: std::ptr::null_mut(), + msg_iovlen: 0, + msg_control: std::ptr::null_mut(), + msg_controllen: 0, + msg_flags: 0, + }; hdr.msg_iov = iovecs.as_mut_ptr(); hdr.msg_iovlen = bufs.len() as _; if let Some((storage, len)) = addr.as_mut() { @@ -557,11 +561,11 @@ impl Poller { // Emulate RWF_APPEND: write at end of file let write_offset = if has_append { - // SAFETY: `libc::stat` is POD and may be zero-initialized. - let mut stat: libc::stat = unsafe { std::mem::zeroed() }; - let result = syscall!(raw fstat(fd, &mut stat)?); + let mut stat = std::mem::MaybeUninit::::uninit(); + let result = syscall!(raw fstat(fd, stat.as_mut_ptr())?); assert_eq!(result, 0); - stat.st_size as i64 + // SAFETY: successful `fstat` initialized `stat`. + unsafe { stat.assume_init() }.st_size } else { offset }; @@ -651,11 +655,12 @@ impl Poller { _ => return false, }; - // SAFETY: `libc::stat` is POD and may be zero-initialized. - let mut stat: libc::stat = unsafe { std::mem::zeroed() }; - if syscall!(raw fstat(fd, &mut stat)) < 0 { + let mut stat = std::mem::MaybeUninit::::uninit(); + if syscall!(raw fstat(fd, stat.as_mut_ptr())) < 0 { return false; } + // SAFETY: successful `fstat` initialized `stat`. + let stat = unsafe { stat.assume_init() }; (stat.st_mode & libc::S_IFMT) == libc::S_IFREG } @@ -803,17 +808,18 @@ impl Poller { *mode, )), Op::Stat { dir_fd, path, follow_symlinks, out } => { - // SAFETY: `libc::stat` is POD and may be zero-initialized. - let mut stat: libc::stat = unsafe { std::mem::zeroed() }; + let mut stat = std::mem::MaybeUninit::::uninit(); let flags = if *follow_symlinks { 0 } else { libc::AT_SYMLINK_NOFOLLOW }; let result = syscall!(raw fstatat( dir_fd.as_raw_fd(), path.as_ptr(), - &mut stat, + stat.as_mut_ptr(), flags, )); if result >= 0 { + // SAFETY: successful `fstatat` initialized `stat`. + let stat = unsafe { stat.assume_init() }; // SAFETY: `out` points to caller-provided writable result storage. unsafe { *out.as_ptr() = crate::backend::op::file_stat_from_raw(&stat); diff --git a/lio/src/backend/impls/pollingv2/os/epoll.rs b/lio/src/backend/impls/pollingv2/os/epoll.rs index af2670e2..280eef6e 100644 --- a/lio/src/backend/impls/pollingv2/os/epoll.rs +++ b/lio/src/backend/impls/pollingv2/os/epoll.rs @@ -235,10 +235,7 @@ impl ReadinessPoll for OsPoller { ) -> io::Result { /// `timespec` value that equals zero. #[cfg(not(target_os = "redox"))] - // SAFETY: All-zeros is a valid representation of timespec (all integer fields) - const TS_ZERO: libc::timespec = unsafe { - std::mem::transmute([0u8; std::mem::size_of::()]) - }; + const TS_ZERO: libc::timespec = libc::timespec { tv_sec: 0, tv_nsec: 0 }; #[cfg(not(target_os = "redox"))] if let Some(ref timer_fd) = self.timer_fd { // Configure the timeout using timerfd. diff --git a/lio/src/ffi.rs b/lio/src/ffi.rs index 5ed2c72c..dea9d042 100644 --- a/lio/src/ffi.rs +++ b/lio/src/ffi.rs @@ -333,7 +333,9 @@ pub unsafe extern "C" fn lio_read_at( offset: i64, callback: extern "C" fn(libc::c_int, *mut u8, usize), ) { - // SAFETY: C caller transfers malloc ownership of buf with size buf_len + // SAFETY: caller transfers ownership of a buffer allocated by + // `lio_buf_alloc(buf_len)`, which matches the `Vec` layout reconstructed + // here. let vec = unsafe { Vec::from_raw_parts(buf, buf_len, buf_len) }; // SAFETY: caller guarantees fd is valid per fn contract let resource = unsafe { fd_to_borrowed_resource(fd) }; @@ -501,8 +503,8 @@ pub unsafe extern "C" fn lio_accept( /// - `callback(result, buf, len)`: bytes sent (or negative errno), buffer /// /// # Safety -/// `lio` must be valid; `buf` must be at least `buf_len` bytes allocated with -/// `malloc`. +/// `lio` must be valid; `buf` must point to at least `buf_len` bytes allocated +/// with `malloc`. #[unsafe(no_mangle)] pub unsafe extern "C" fn lio_send( lio: *mut lio_handle_t, @@ -539,8 +541,8 @@ pub unsafe extern "C" fn lio_send( /// - `callback(result, buf, len)`: bytes received (or negative errno), buffer /// /// # Safety -/// `lio` must be valid; `buf` must be at least `buf_len` bytes allocated with -/// `malloc`. +/// `lio` must be valid; `buf` must be a live buffer previously allocated by +/// [`lio_buf_alloc(buf_len)`]. #[unsafe(no_mangle)] pub unsafe extern "C" fn lio_recv( lio: *mut lio_handle_t, @@ -550,7 +552,9 @@ pub unsafe extern "C" fn lio_recv( flags: libc::c_int, callback: extern "C" fn(libc::c_int, *mut u8, usize), ) { - // SAFETY: C caller transfers malloc ownership of buf with size buf_len + // SAFETY: caller transfers ownership of a buffer allocated by + // `lio_buf_alloc(buf_len)`, which matches the `Vec` layout reconstructed + // here. let vec = unsafe { Vec::from_raw_parts(buf, buf_len, buf_len) }; // SAFETY: caller guarantees fd is valid per fn contract let resource = unsafe { fd_to_borrowed_resource(fd) }; diff --git a/lio/src/fs/file.rs b/lio/src/fs/file.rs index c30cf47f..681adafa 100644 --- a/lio/src/fs/file.rs +++ b/lio/src/fs/file.rs @@ -347,20 +347,22 @@ impl File { /// ``` #[cfg(unix)] pub fn metadata(&self) -> io::Result { + use std::mem::MaybeUninit; use std::os::fd::AsRawFd; let fd = self.0.as_raw_fd(); - // SAFETY: stat struct can be safely zero-initialized - let mut stat: libc::stat = unsafe { std::mem::zeroed() }; + let mut stat = MaybeUninit::::uninit(); - // SAFETY: fd is valid, stat is a valid mutable pointer - let ret = unsafe { libc::fstat(fd, &mut stat) }; + // SAFETY: fd is valid and `stat` points to writable output storage for the + // duration of the call. + let ret = unsafe { libc::fstat(fd, stat.as_mut_ptr()) }; if ret < 0 { return Err(io::Error::last_os_error()); } - Ok(Metadata { stat }) + // SAFETY: successful `fstat` initialized `stat`. + Ok(Metadata { stat: unsafe { stat.assume_init() } }) } } diff --git a/lio/src/macros.rs b/lio/src/macros.rs index 6cfbfe8c..9a1aa516 100644 --- a/lio/src/macros.rs +++ b/lio/src/macros.rs @@ -172,27 +172,22 @@ macro_rules! syscall { } else { // Return negative errno - this will cause early return from function + // #[cfg(target_os = "linux")] - { - // SAFETY: This exists on the platform in cfg. - let errno = unsafe { *libc::__errno_location() }; - -(errno as isize) - } + // SAFETY: This exists on the platform in cfg. + let err = unsafe { *libc::__errno_location() }; #[cfg(any(target_os = "macos", target_os = "freebsd"))] - { - // SAFETY: This exists on the platform in cfg. - let errno = unsafe { *libc::__error() }; - -(errno as isize) - } + // SAFETY: This exists on the platform in cfg. + let err = unsafe { *libc::__error() }; #[cfg(windows)] - { - // SAFETY: This exists on the platform in cfg. - let last_error = unsafe { windows_sys::Win32::Foundation::GetLastError() }; - -(last_error as isize) - } + // SAFETY: This exists on the platform in cfg. + let err = unsafe { windows_sys::Win32::Foundation::GetLastError() }; + + -(err as isize) } } }}; + ($fn: ident ( $($arg: expr),* $(,)* ) ) => {{ let result = syscall!(raw $fn($($arg),*)); if result >= 0 { diff --git a/lio/src/process.rs b/lio/src/process.rs index 18cb2bbf..7bdef12c 100644 --- a/lio/src/process.rs +++ b/lio/src/process.rs @@ -300,7 +300,7 @@ impl ChildStdout { resource: self.inner.clone(), accumulated: Vec::new(), buffer: vec![0u8; 8192], - iovec: unsafe { std::mem::zeroed() }, + iovec: libc::iovec { iov_base: std::ptr::null_mut(), iov_len: 0 }, done: false, }) } @@ -381,7 +381,7 @@ impl ChildStderr { resource: self.inner.clone(), accumulated: Vec::new(), buffer: vec![0u8; 8192], - iovec: unsafe { std::mem::zeroed() }, + iovec: libc::iovec { iov_base: std::ptr::null_mut(), iov_len: 0 }, done: false, }) } @@ -709,15 +709,16 @@ impl SpawnChild { return Ok(()); } - // Initialize file actions - // SAFETY: posix_spawn_file_actions_t is safe to zero-initialize - let mut file_actions: libc::posix_spawn_file_actions_t = - unsafe { std::mem::zeroed() }; - // SAFETY: file_actions is a valid pointer to uninitialized posix_spawn_file_actions_t - let ret = unsafe { libc::posix_spawn_file_actions_init(&mut file_actions) }; + let mut file_actions = + std::mem::MaybeUninit::::uninit(); + // SAFETY: `file_actions` points to writable storage that + // `posix_spawn_file_actions_init` initializes on success. + let ret = unsafe { libc::posix_spawn_file_actions_init(file_actions.as_mut_ptr()) }; if ret != 0 { return Err(io::Error::from_raw_os_error(ret)); } + // SAFETY: successful `posix_spawn_file_actions_init` initialized the value. + let mut file_actions = unsafe { file_actions.assume_init() }; // Open /dev/null if needed let needs_null = self.stdin_cfg == Stdio::Null