Skip to content

Commit

Permalink
Merge pull request #17 from niklasmohrin/refactor-aliasing
Browse files Browse the repository at this point in the history
  • Loading branch information
niklasmohrin authored Oct 24, 2024
2 parents 777a83f + f5f4c63 commit 77d0604
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 38 deletions.
12 changes: 12 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,18 @@ jobs:
- run: cargo +nightly update -Zminimal-versions
- run: cargo +stable build --locked --all-features --all-targets

semver:
name: Check for semver breakage
strategy:
fail-fast: false
matrix:
os: [ubuntu-latest, windows-latest]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
- uses: dtolnay/rust-toolchain@stable
- uses: obi1kenobi/cargo-semver-checks-action@v2

rustfmt:
name: Check formatting
runs-on: ubuntu-latest
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "clircle"
version = "0.5.0"
version = "0.6.0"
authors = ["Niklas Mohrin <dev@niklasmohrin.de>"]
license = "MIT OR Apache-2.0"
rust-version = "1.69"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

[![CI](https://github.com/niklasmohrin/clircle/actions/workflows/ci.yml/badge.svg?branch=main&event=push)](https://github.com/niklasmohrin/clircle/actions/workflows/ci.yml)
[![crates.io version](https://img.shields.io/crates/v/clircle)](https://crates.io/crates/clircle)
[![MSRV](https://img.shields.io/badge/MSRV-1.63.0-blue)](https://blog.rust-lang.org/2022/08/11/Rust-1.63.0.html)
[![MSRV](https://img.shields.io/badge/MSRV-1.69.0-blue)](https://blog.rust-lang.org/2023/04/20/Rust-1.69.0.html)

Clircle provides a cross-platform API to detect read / write cycles from your
user-supplied arguments. You can get the important identifiers of a file (from
Expand Down
30 changes: 15 additions & 15 deletions src/clircle_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::{cmp, hash, ops};

/// Implementation of `Clircle` for Unix.
#[derive(Debug)]
pub struct UnixIdentifier {
pub(crate) struct Identifier {
device: u64,
inode: u64,
size: u64,
Expand All @@ -19,7 +19,7 @@ pub struct UnixIdentifier {
owns_fd: bool,
}

impl UnixIdentifier {
impl Identifier {
fn file(&self) -> &File {
self.file.as_ref().expect("Called file() on an identifier that has already been destroyed, this should never happen! Please file a bug!")
}
Expand All @@ -45,15 +45,15 @@ impl UnixIdentifier {
/// # Errors
///
/// The underlying call to `File::metadata` fails.
pub unsafe fn try_from_raw_fd(fd: RawFd, owns_fd: bool) -> io::Result<Self> {
unsafe fn try_from_raw_fd(fd: RawFd, owns_fd: bool) -> io::Result<Self> {
Self::try_from(File::from_raw_fd(fd)).map(|mut ident| {
ident.owns_fd = owns_fd;
ident
})
}
}

impl Clircle for UnixIdentifier {
impl Clircle for Identifier {
#[must_use]
fn into_inner(mut self) -> Option<File> {
if self.owns_fd {
Expand All @@ -73,7 +73,7 @@ impl Clircle for UnixIdentifier {
}
}

impl TryFrom<Stdio> for UnixIdentifier {
impl TryFrom<Stdio> for Identifier {
type Error = <Self as TryFrom<File>>::Error;

fn try_from(stdio: Stdio) -> Result<Self, Self::Error> {
Expand All @@ -88,15 +88,15 @@ impl TryFrom<Stdio> for UnixIdentifier {
}
}

impl ops::Drop for UnixIdentifier {
impl ops::Drop for Identifier {
fn drop(&mut self) {
if !self.owns_fd {
let _ = self.file.take().map(IntoRawFd::into_raw_fd);
}
}
}

impl TryFrom<File> for UnixIdentifier {
impl TryFrom<File> for Identifier {
type Error = io::Error;

fn try_from(file: File) -> Result<Self, Self::Error> {
Expand All @@ -111,16 +111,16 @@ impl TryFrom<File> for UnixIdentifier {
}
}

impl cmp::PartialEq for UnixIdentifier {
impl cmp::PartialEq for Identifier {
#[must_use]
fn eq(&self, other: &Self) -> bool {
self.device == other.device && self.inode == other.inode
}
}

impl Eq for UnixIdentifier {}
impl Eq for Identifier {}

impl hash::Hash for UnixIdentifier {
impl hash::Hash for Identifier {
fn hash<H: hash::Hasher>(&self, state: &mut H) {
self.device.hash(state);
self.inode.hash(state);
Expand All @@ -141,7 +141,7 @@ mod tests {
fn test_into_inner() {
let file = tempfile::tempfile().expect("failed to create tempfile");
file.metadata().expect("can stat file");
let ident = UnixIdentifier::try_from(file).expect("failed to create identifier");
let ident = Identifier::try_from(file).expect("failed to create identifier");
let mut file = ident
.into_inner()
.expect("failed to convert identifier to file");
Expand All @@ -153,7 +153,7 @@ mod tests {
fn test_borrowed_fd() {
let file = tempfile::tempfile().expect("failed to create tempfile");
let fd: OwnedFd = file.into();
let ident = unsafe { UnixIdentifier::try_from_raw_fd(fd.as_raw_fd(), false) }
let ident = unsafe { Identifier::try_from_raw_fd(fd.as_raw_fd(), false) }
.expect("failed to create identifier");
drop(ident);
let fd = fd.into_raw_fd();
Expand All @@ -166,7 +166,7 @@ mod tests {
fn test_owned_fd() {
let file = tempfile::tempfile().expect("failed to create tempfile");
let fd: OwnedFd = file.into();
let ident = unsafe { UnixIdentifier::try_from_raw_fd(fd.as_raw_fd(), true) }
let ident = unsafe { Identifier::try_from_raw_fd(fd.as_raw_fd(), true) }
.expect("failed to create identifier");
drop(ident);
#[cfg(feature = "test-close-again")]
Expand All @@ -181,14 +181,14 @@ mod tests {
slave: child,
} = openpty(None, None).expect("failed to open pty");

let parent_ident = unsafe { UnixIdentifier::try_from_raw_fd(parent.as_raw_fd(), false) }
let parent_ident = unsafe { Identifier::try_from_raw_fd(parent.as_raw_fd(), false) }
.expect("failed to create parent identifier");

assert_eq!(parent_ident, parent_ident);

assert!(!parent_ident.surely_conflicts_with(&parent_ident));

let child_ident = unsafe { UnixIdentifier::try_from_raw_fd(child.as_raw_fd(), false) }
let child_ident = unsafe { Identifier::try_from_raw_fd(child.as_raw_fd(), false) }
.expect("failed to create child identifier");

assert_ne!(parent_ident, child_ident);
Expand Down
25 changes: 11 additions & 14 deletions src/clircle_windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,32 +14,29 @@ use std::mem::MaybeUninit;
use std::os::windows::io::{FromRawHandle, IntoRawHandle, RawHandle};
use std::{cmp, hash, io, mem, ops};

/// Re-export of winapi
pub use winapi;

/// Implementation of `Clircle` for Windows.
#[derive(Debug)]
pub struct WindowsIdentifier {
pub(crate) struct Identifier {
volume_serial: u32,
file_index: u64,
handle: RawHandle,
owns_handle: bool,
}

impl WindowsIdentifier {
impl Identifier {
unsafe fn try_from_raw_handle(handle: RawHandle, owns_handle: bool) -> Result<Self, io::Error> {
if handle == INVALID_HANDLE_VALUE || handle == NULL {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"Tried to convert handle to WindowsIdentifier that was invalid or null.",
"Tried to convert handle to Identifier that was invalid or null.",
));
}
// SAFETY: This function can be called with any valid handle.
// https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfiletype
if GetFileType(handle) != FILE_TYPE_DISK {
return Err(io::Error::new(
io::ErrorKind::InvalidInput,
"Tried to convert handle to WindowsIdentifier that was not a file handle.",
"Tried to convert handle to Identifier that was not a file handle.",
));
}
let mut fi = MaybeUninit::<BY_HANDLE_FILE_INFORMATION>::uninit();
Expand Down Expand Up @@ -72,14 +69,14 @@ impl WindowsIdentifier {
}
}

impl Clircle for WindowsIdentifier {
impl Clircle for Identifier {
#[must_use]
fn into_inner(mut self) -> Option<File> {
Some(unsafe { File::from_raw_handle(self.take_handle()?) })
}
}

impl TryFrom<Stdio> for WindowsIdentifier {
impl TryFrom<Stdio> for Identifier {
type Error = io::Error;

fn try_from(stdio: Stdio) -> Result<Self, Self::Error> {
Expand All @@ -99,15 +96,15 @@ impl TryFrom<Stdio> for WindowsIdentifier {
unsafe { Self::try_from_raw_handle(handle, false) }
}
}
impl TryFrom<File> for WindowsIdentifier {
impl TryFrom<File> for Identifier {
type Error = io::Error;

fn try_from(file: File) -> Result<Self, Self::Error> {
unsafe { Self::try_from_raw_handle(file.into_raw_handle(), true) }
}
}

impl ops::Drop for WindowsIdentifier {
impl ops::Drop for Identifier {
fn drop(&mut self) {
unsafe {
if let Some(handle) = self.take_handle() {
Expand All @@ -117,15 +114,15 @@ impl ops::Drop for WindowsIdentifier {
}
}

impl cmp::PartialEq for WindowsIdentifier {
impl cmp::PartialEq for Identifier {
#[must_use]
fn eq(&self, other: &Self) -> bool {
self.volume_serial == other.volume_serial && self.file_index == other.file_index
}
}
impl Eq for WindowsIdentifier {}
impl Eq for Identifier {}

impl hash::Hash for WindowsIdentifier {
impl hash::Hash for Identifier {
fn hash<H: hash::Hasher>(&self, state: &mut H) {
self.volume_serial.hash(state);
self.file_index.hash(state);
Expand Down
42 changes: 35 additions & 7 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,10 @@
cfg_if::cfg_if! {
if #[cfg(unix)] {
mod clircle_unix;
pub use clircle_unix::UnixIdentifier;
/// Identifies a file. The type is aliased according to the target platform.
pub type Identifier = UnixIdentifier;
use clircle_unix as imp;
} else if #[cfg(windows)] {
mod clircle_windows;
pub use clircle_windows::{winapi, WindowsIdentifier};
/// Identifies a file. The type is aliased according to the target platform.
pub type Identifier = WindowsIdentifier;
use clircle_windows as imp;
} else {
compile_error!("Neither cfg(unix) nor cfg(windows) was true, aborting.");
}
Expand All @@ -64,6 +60,7 @@ cfg_if::cfg_if! {
use serde_derive::{Deserialize, Serialize};
use std::convert::TryFrom;
use std::fs::File;
use std::io;

/// The `Clircle` trait describes the public interface of the crate.
/// It contains all the platform-independent functionality.
Expand All @@ -76,7 +73,7 @@ pub trait Clircle: Eq + TryFrom<Stdio> + TryFrom<File> {
fn into_inner(self) -> Option<File>;

/// Checks whether the two values will without doubt conflict. By default, this always returns
/// `false`, but implementors can override this method. Currently, only `UnixIdentifier`
/// `false`, but implementors can override this method. Currently, only the Unix implementation
/// overrides `surely_conflicts_with`.
fn surely_conflicts_with(&self, _other: &Self) -> bool {
false
Expand Down Expand Up @@ -127,6 +124,37 @@ where
T::stdout().map_or(false, |stdout| inputs.contains(&stdout))
}

/// Identifies a file. The type forwards all methods to the platform implementation.
#[derive(Debug, PartialEq, Eq, Hash)]
pub struct Identifier(imp::Identifier);

impl Clircle for Identifier {
#[must_use]
fn into_inner(self) -> Option<File> {
self.0.into_inner()
}

fn surely_conflicts_with(&self, other: &Self) -> bool {
self.0.surely_conflicts_with(&other.0)
}
}

impl TryFrom<Stdio> for Identifier {
type Error = io::Error;

fn try_from(stdio: Stdio) -> Result<Self, Self::Error> {
imp::Identifier::try_from(stdio).map(Self)
}
}

impl TryFrom<File> for Identifier {
type Error = io::Error;

fn try_from(file: File) -> Result<Self, Self::Error> {
imp::Identifier::try_from(file).map(Self)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down

0 comments on commit 77d0604

Please sign in to comment.