From 96badb5752cb1524ef5d1f7faf88558a774829c3 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 17 Aug 2021 15:28:20 -0700 Subject: [PATCH] Use rustix instead of calling libc directly. Use rustix instead of calling libc directly, which factors out several unsafe blocks and simplifies error handling. There is still one `unsafe` block needed, to dereference raw file descriptors passed in from the user, since the crate still uses `AsRawFd`. Once I/O safety is stablized in std, this crate can switch to using `AsFd`, which will eliminate the last `unsafe` block. This splits out just the API-compatible parts of #14. It does not include the changes to use the `AsFd` trait in the public API, so it's not an API-breaking change. --- Cargo.toml | 4 +++- README.md | 6 ++--- src/sys/unix/mod.rs | 2 -- src/sys/unix/read_guard.rs | 6 ++--- src/sys/unix/rw_lock.rs | 36 +++++++++++++++++++----------- src/sys/unix/utils.rs | 9 -------- src/sys/unix/write_guard.rs | 6 ++--- src/sys/unsupported/read_guard.rs | 2 -- src/sys/unsupported/rw_lock.rs | 2 -- src/sys/unsupported/write_guard.rs | 2 -- 10 files changed, 33 insertions(+), 42 deletions(-) delete mode 100644 src/sys/unix/utils.rs diff --git a/Cargo.toml b/Cargo.toml index c6db2ed..dcdbcc2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,6 @@ readme = "README.md" edition = "2018" [dependencies] -libc = "0.2.58" cfg-if = "1.0.0" [target.'cfg(windows)'.dependencies.windows-sys] @@ -23,5 +22,8 @@ features = [ "Win32_System_IO", ] +[target.'cfg(unix)'.dependencies] +rustix = "0.31.3" + [dev-dependencies] tempfile = "3.0.8" diff --git a/README.md b/README.md index db00f1d..6f3019d 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ $ cargo add fd-lock ``` ## Safety -This crate uses `unsafe` to interface with `libc` and `winapi`. All invariants +This crate uses `unsafe` on Windows to interface with `winapi`. All invariants have been carefully checked, and are manually enforced. ## Contributing @@ -50,8 +50,8 @@ look at some of these issues: ## References - [LockFile function - WDC](https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-lockfile) -- [flock(2) - Linux Man Page](https://linux.die.net/man/2/flock) -- [`libc::flock`](https://docs.rs/libc/0.2.58/libc/fn.flock.html) +- [flock(2) - Linux Man Page](https://man7.org/linux/man-pages/man2/flock.2.html) +- [`rustix::fs::flock`](https://docs.rs/rustix/*/rustix/fs/fn.flock.html) - [`winapi::um::fileapi::LockFile`](https://docs.rs/winapi/0.3.7/x86_64-pc-windows-msvc/winapi/um/fileapi/fn.LockFile.html) ## License diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 9d7bd2c..023e691 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -2,8 +2,6 @@ mod read_guard; mod rw_lock; mod write_guard; -pub(crate) mod utils; - pub use read_guard::RwLockReadGuard; pub use rw_lock::RwLock; pub use write_guard::RwLockWriteGuard; diff --git a/src/sys/unix/read_guard.rs b/src/sys/unix/read_guard.rs index bf7490b..6d67187 100644 --- a/src/sys/unix/read_guard.rs +++ b/src/sys/unix/read_guard.rs @@ -1,8 +1,7 @@ -use libc::{flock, LOCK_UN}; +use rustix::fs::{flock, FlockOperation}; use std::ops; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::RwLock; #[derive(Debug)] @@ -28,7 +27,6 @@ impl ops::Deref for RwLockReadGuard<'_, T> { impl Drop for RwLockReadGuard<'_, T> { #[inline] fn drop(&mut self) { - let fd = self.lock.inner.as_raw_fd(); - let _ = syscall(unsafe { flock(fd, LOCK_UN) }); + let _ = flock(&self.lock.as_fd(), FlockOperation::Unlock).ok(); } } diff --git a/src/sys/unix/rw_lock.rs b/src/sys/unix/rw_lock.rs index 70c9860..9c6fc7b 100644 --- a/src/sys/unix/rw_lock.rs +++ b/src/sys/unix/rw_lock.rs @@ -1,8 +1,8 @@ -use libc::{flock, LOCK_EX, LOCK_NB, LOCK_SH}; +use rustix::fd::BorrowedFd; +use rustix::fs::{flock, FlockOperation}; use std::io::{self, Error, ErrorKind}; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::{RwLockReadGuard, RwLockWriteGuard}; #[derive(Debug)] @@ -18,34 +18,34 @@ impl RwLock { #[inline] pub fn write(&mut self) -> io::Result> { - let fd = self.inner.as_raw_fd(); - syscall(unsafe { flock(fd, LOCK_EX) })?; + flock(&self.as_fd(), FlockOperation::LockExclusive)?; Ok(RwLockWriteGuard::new(self)) } #[inline] pub fn try_write(&mut self) -> Result, Error> { - let fd = self.inner.as_raw_fd(); - syscall(unsafe { flock(fd, LOCK_EX | LOCK_NB) }).map_err(|err| match err.kind() { + flock(&self.as_fd(), FlockOperation::NonBlockingLockExclusive).map_err(|err| match err + .kind() + { ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(), - _ => err, + _ => Error::from(err), })?; Ok(RwLockWriteGuard::new(self)) } #[inline] pub fn read(&self) -> io::Result> { - let fd = self.inner.as_raw_fd(); - syscall(unsafe { flock(fd, LOCK_SH) })?; + flock(&self.as_fd(), FlockOperation::LockShared)?; Ok(RwLockReadGuard::new(self)) } #[inline] pub fn try_read(&self) -> Result, Error> { - let fd = self.inner.as_raw_fd(); - syscall(unsafe { flock(fd, LOCK_SH | LOCK_NB) }).map_err(|err| match err.kind() { - ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(), - _ => err, + flock(&self.as_fd(), FlockOperation::NonBlockingLockShared).map_err(|err| { + match err.kind() { + ErrorKind::AlreadyExists => ErrorKind::WouldBlock.into(), + _ => Error::from(err), + } })?; Ok(RwLockReadGuard::new(self)) } @@ -57,4 +57,14 @@ impl RwLock { { self.inner } + + #[inline] + pub(crate) fn as_fd(&self) -> BorrowedFd<'_> { + // Safety: We assume that `self.inner`'s file descriptor is valid for + // at least the lifetime of `self`. + // + // Once I/O safety is stablized in std, we can switch the public API to + // use `AsFd` instead of `AsRawFd` and eliminate this `unsafe` block. + unsafe { BorrowedFd::borrow_raw_fd(self.inner.as_raw_fd()) } + } } diff --git a/src/sys/unix/utils.rs b/src/sys/unix/utils.rs deleted file mode 100644 index 191dd0c..0000000 --- a/src/sys/unix/utils.rs +++ /dev/null @@ -1,9 +0,0 @@ -use std::io; -use std::os::raw::c_int; - -pub(crate) fn syscall(int: c_int) -> io::Result<()> { - match int { - 0 => Ok(()), - _ => Err(io::Error::last_os_error()), - } -} diff --git a/src/sys/unix/write_guard.rs b/src/sys/unix/write_guard.rs index 6060d60..a12bdf9 100644 --- a/src/sys/unix/write_guard.rs +++ b/src/sys/unix/write_guard.rs @@ -1,8 +1,7 @@ -use libc::{flock, LOCK_UN}; +use rustix::fs::{flock, FlockOperation}; use std::ops; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::RwLock; #[derive(Debug)] @@ -35,7 +34,6 @@ impl ops::DerefMut for RwLockWriteGuard<'_, T> { impl Drop for RwLockWriteGuard<'_, T> { #[inline] fn drop(&mut self) { - let fd = self.lock.inner.as_raw_fd(); - let _ = syscall(unsafe { flock(fd, LOCK_UN) }); + let _ = flock(&self.lock.as_fd(), FlockOperation::Unlock).ok(); } } diff --git a/src/sys/unsupported/read_guard.rs b/src/sys/unsupported/read_guard.rs index c0ea175..7d6b4c7 100644 --- a/src/sys/unsupported/read_guard.rs +++ b/src/sys/unsupported/read_guard.rs @@ -1,8 +1,6 @@ -use libc::{flock, LOCK_UN}; use std::ops; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::RwLock; #[derive(Debug)] diff --git a/src/sys/unsupported/rw_lock.rs b/src/sys/unsupported/rw_lock.rs index 856e5d7..fb11da8 100644 --- a/src/sys/unsupported/rw_lock.rs +++ b/src/sys/unsupported/rw_lock.rs @@ -1,8 +1,6 @@ -use libc::{flock, LOCK_EX, LOCK_NB, LOCK_SH}; use std::io::{self, Error, ErrorKind}; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::{RwLockReadGuard, RwLockWriteGuard}; #[derive(Debug)] diff --git a/src/sys/unsupported/write_guard.rs b/src/sys/unsupported/write_guard.rs index 28a6a64..19727c3 100644 --- a/src/sys/unsupported/write_guard.rs +++ b/src/sys/unsupported/write_guard.rs @@ -1,8 +1,6 @@ -use libc::{flock, LOCK_UN}; use std::ops; use std::os::unix::io::AsRawFd; -use super::utils::syscall; use super::RwLock; #[derive(Debug)]