pq/libfreemkv
1
Fork 0
mirror of https://github.com/freemkv/libfreemkv.git synced 2026-10-07 18:13:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

io: remove unused open_for_mkv and path-based fs_type detection

Browse Source
This commit is contained in:
Matthew Jackson
2026-09-29 19:45:46 -07:00
parent 339a4b780a
commit 3feb0e0f8b
10 changed files with 27 additions and 304 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/io/flush.rs
+1 -1
View File
@@ -306,7 +306,7 @@ pub(crate) fn wait_failure(e: BoundedError, what: &str) -> io::Error {
}
}
#[cfg(unix)]
#[cfg(any(target_os = "linux", target_os = "macos"))]
fn rc(r: libc::c_int) -> io::Result<()> {
if r == 0 {
Ok(())
src/io/sink/mod.rs
-86
View File
@@ -34,67 +34,10 @@ pub trait SequentialSink: Write + Send {
/// random-access sink is always usable as a sequential sink.
pub trait RandomAccessSink: SequentialSink + Seek {}
// Picks the right RandomAccessSink impl for `dest` by filesystem type (Linux+NFS ->
// WritebackFile, else LocalFileSink). Not yet wired into mux::resolve.
#[allow(dead_code)]
pub(crate) fn open_for_mkv(
dest: &std::path::Path,
size_hint: Option<u64>,
) -> std::io::Result<Box<dyn RandomAccessSink>> {
#[cfg(target_os = "linux")]
{
use crate::platform::fs_type::FsType;
if classify_dest(dest) == FsType::Nfs {
let wf = match size_hint {
Some(n) => crate::io::WritebackFile::create_with_size_hint(dest, n)?,
None => crate::io::WritebackFile::create(dest)?,
};
return Ok(Box::new(wf));
}
}
// Only Linux differentiates the sink by filesystem type; other OSes always use
// `LocalFileSink`. Reference `classify_dest` as a value (no call/syscall) so it
// isn't flagged dead on non-Linux while avoiding a wasted probe.
#[cfg(not(target_os = "linux"))]
let _ = classify_dest;
let sink = match size_hint {
Some(n) => LocalFileSink::with_size_hint(dest, n)?,
None => LocalFileSink::create(dest)?,
};
Ok(Box::new(sink))
}
// Filesystem type of the output location. statfs needs an existing path and
// `dest` usually is not created yet, so probe its directory.
fn classify_dest(dest: &std::path::Path) -> crate::platform::fs_type::FsType {
crate::platform::fs_type::detect(probe_dir(dest))
}
fn probe_dir(dest: &std::path::Path) -> &std::path::Path {
match dest.parent() {
Some(p) if !p.as_os_str().is_empty() => p,
_ => std::path::Path::new("."),
}
}
#[cfg(test)]
mod tests {
use super::*;
// `dest` normally does not exist yet, so statfs on it fails (Unknown) and the
// NFS branch was unreachable. The classification must come from its directory.
#[test]
fn classify_dest_probes_the_directory_of_a_new_file() {
use std::path::Path;
assert_eq!(
probe_dir(Path::new("/mnt/nfs/out/Movie.mkv")),
Path::new("/mnt/nfs/out")
);
// A bare file name lives in the current directory.
assert_eq!(probe_dir(Path::new("bare.mkv")), Path::new("."));
}
// Type-level assertion: the concrete sinks satisfy the trait
// objects. These functions never run; they just have to type-check.
fn _assert_is_sequential(_: &mut dyn SequentialSink) {}
@@ -115,20 +58,6 @@ mod tests {
_assert_is_random_access(&mut wf);
}
#[test]
fn open_for_mkv_returns_a_random_access_sink() {
let dir = tempfile::tempdir().unwrap();
let p = dir.path().join("c.bin");
let mut sink = open_for_mkv(&p, Some(64 * 1024)).unwrap();
use std::io::{Seek, SeekFrom, Write};
sink.write_all(b"hello").unwrap();
sink.seek(SeekFrom::Start(0)).unwrap();
sink.finish().unwrap();
drop(sink);
let bytes = std::fs::read(&p).unwrap();
assert_eq!(&bytes[..5], b"hello");
}
// Regression test for the silent-no-op finish() bug.
#[test]
fn finish_through_trait_object_flushes_local_file() {
@@ -191,19 +120,4 @@ mod tests {
);
assert_eq!(bytes.load(Ordering::SeqCst), 3);
}
// Covers the `None` size_hint arm: must still be a random-access sink.
#[test]
fn open_for_mkv_without_size_hint_is_random_access() {
use std::io::{Seek, SeekFrom};
let dir = tempfile::tempdir().unwrap();
let p = dir.path().join("nohint.bin");
let mut sink = open_for_mkv(&p, None).unwrap();
sink.write_all(b"AAAABBBB").unwrap();
sink.seek(SeekFrom::Start(4)).unwrap();
sink.write_all(b"CCCC").unwrap();
sink.finish().unwrap();
drop(sink);
assert_eq!(std::fs::read(&p).unwrap(), b"AAAACCCC");
}
}
src/platform/fs_type/linux.rs
+4 -25
View File
@@ -1,13 +1,10 @@
//! Linux `statfs64`-based filesystem type detection.
//! Linux `fstatfs`-based filesystem type detection.
//!
//! Recognised local-FS magics: ext2/3/4, xfs, btrfs, tmpfs. NFS is the
//! one network FS this layer cares about (the buffering decision keys
//! off it). Anything else maps to [`FsType::Unknown`].
use std::ffi::CString;
use std::os::unix::ffi::OsStrExt;
use std::os::unix::io::RawFd;
use std::path::Path;
use super::FsType;
@@ -19,8 +16,7 @@ const XFS_SUPER_MAGIC: i64 = 0x5846_5342;
const BTRFS_SUPER_MAGIC: i64 = 0x9123_683E;
const TMPFS_MAGIC: i64 = 0x0102_1994;
// Classify an `f_type` magic from `statfs`/`fstatfs`; shared by the path-based and fd-based
// entry points.
// Classify an `f_type` magic from `fstatfs`.
#[allow(clippy::unnecessary_cast)]
fn classify_f_type(f_type: i64) -> FsType {
let nfs_magic = libc::NFS_SUPER_MAGIC as i64;
@@ -33,25 +29,8 @@ fn classify_f_type(f_type: i64) -> FsType {
}
}
pub(super) fn detect_impl(path: &Path) -> FsType {
let cpath = match CString::new(path.as_os_str().as_bytes()) {
Ok(c) => c,
Err(_) => return FsType::Unknown,
};
// `statfs64` is repr(C); zeroing is the documented init pattern for
// the kernel uapi struct.
let mut buf: libc::statfs64 = unsafe { std::mem::zeroed() };
let rc = unsafe { libc::statfs64(cpath.as_ptr(), &mut buf) };
if rc != 0 {
return FsType::Unknown;
}
#[allow(clippy::unnecessary_cast)]
classify_f_type(buf.f_type as i64)
}
/// fd-based variant of [`detect_impl`]. Same return semantics. Used
/// by the writeback pipeline, which knows the open `File` but not its
/// original path.
/// `fstatfs` classification of an open fd. Used by the writeback pipeline,
/// which knows the open `File` but not its original path.
pub(super) fn detect_fd_impl(fd: RawFd) -> FsType {
let mut buf: libc::statfs = unsafe { std::mem::zeroed() };
let rc = unsafe { libc::fstatfs(fd, &mut buf) };
src/platform/fs_type/macos.rs
-41
View File
@@ -1,41 +0,0 @@
//! macOS `statfs`-based filesystem type detection.
//!
//! macOS exposes a textual `f_fstypename` field (e.g. `"apfs"`, `"hfs"`,
//! `"nfs"`, `"smbfs"`) on its `statfs` struct, which is far more
//! reliable than chasing magic numbers. Anything starting with `"nfs"`
//! counts as NFS; anything else recognised maps to `Local`; otherwise
//! `Unknown`.
use std::ffi::CString;
use std::os::unix::ffi::OsStrExt;
use std::path::Path;
use super::FsType;
pub(super) fn detect_impl(path: &Path) -> FsType {
let cpath = match CString::new(path.as_os_str().as_bytes()) {
Ok(c) => c,
Err(_) => return FsType::Unknown,
};
let mut buf: libc::statfs = unsafe { std::mem::zeroed() };
let rc = unsafe { libc::statfs(cpath.as_ptr(), &mut buf) };
if rc != 0 {
return FsType::Unknown;
}
// `f_fstypename` is a NUL-terminated C string of length MFSTYPENAMELEN.
// SAFETY: libc guarantees the field is initialised to a NUL-terminated
// string by a successful statfs.
let name_ptr = buf.f_fstypename.as_ptr();
let cstr = unsafe { std::ffi::CStr::from_ptr(name_ptr) };
let name = cstr.to_bytes();
if name.starts_with(b"nfs") {
return FsType::Nfs;
}
// Recognised local types. SMB is not NFS but the buffering-policy
// outcome on macOS is the same as for any other local FS — there's
// no `WritebackFile` machinery to opt out of on this OS.
match name {
b"apfs" | b"hfs" | b"exfat" | b"msdos" | b"tmpfs" | b"smbfs" | b"webdav" => FsType::Local,
_ => FsType::Unknown,
}
}
src/platform/fs_type/mod.rs
+17 -106
View File
@@ -1,136 +1,47 @@
//! Filesystem-type detection.
//!
//! Picks the output sink for the Phase 2 buffering architecture (NFS gets `WritebackFile`,
//! local disks get `LocalFileSink`) and exposes the cross-platform enum and `detect` entry
//! point; the per-OS `statfs` call lives in the matching platform file.
//! Filesystem-type detection for an open file (Linux only): the writeback pipeline and the
//! flusher key their NFS policy off it. The `fstatfs` call lives in `linux.rs`.
use std::path::Path;
/// What kind of filesystem a path lives on, to the extent we can tell
/// What kind of filesystem a file lives on, to the extent we can tell
/// cheaply at construction time.
///
/// `Unknown` is the fail-open default: a misdetection here should not
/// be load-bearing for correctness, only for the choice of sink (and
/// hence buffering policy). Callers that need a binary local/non-local
/// answer should treat `Unknown` as "probably local".
/// be load-bearing for correctness, only for buffering policy. Callers
/// that need a binary local/non-local answer should treat `Unknown` as
/// "probably local".
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum FsType {
/// A local on-disk filesystem (ext4, xfs, btrfs, apfs, ntfs, …).
/// A local on-disk filesystem (ext4, xfs, btrfs, tmpfs).
Local,
/// A network filesystem with NFS-like semantics. The current
/// detector lumps SMB / UNC into this on Windows because the
/// buffering policy outcome is the same.
/// A network filesystem with NFS semantics.
Nfs,
/// `statfs` failed, the filesystem type is not on our recognised
/// list, or we are on an OS without a real implementation.
/// `fstatfs` failed or the filesystem type is not on our recognised list.
Unknown,
}
#[cfg(target_os = "linux")]
mod linux;
#[cfg(target_os = "macos")]
mod macos;
#[cfg(not(any(target_os = "linux", target_os = "macos", target_os = "windows")))]
mod other;
#[cfg(target_os = "windows")]
mod windows;
#[cfg(target_os = "linux")]
use linux::detect_impl;
#[cfg(target_os = "macos")]
use macos::detect_impl;
#[cfg(not(any(target_os = "linux", target_os = "macos", target_os = "windows")))]
use other::detect_impl;
#[cfg(target_os = "windows")]
use windows::detect_impl;
#[cfg(target_os = "linux")]
use linux::detect_fd_impl;
/// Best-effort classification of the filesystem under `path`.
/// Best-effort classification of the filesystem under the open `fd`.
///
/// Falls back to [`FsType::Unknown`] on any syscall error or unrecognised
/// filesystem signature. Never panics. Never blocks beyond the cost of
/// a single `statfs(2)` (Unix) or a string check (Windows).
pub fn detect(path: &Path) -> FsType {
detect_impl(path)
}
/// fd-based classification. Same return semantics as [`detect`], but
/// takes a `RawFd` so callers that only have an open file (notably
/// the writeback pipeline) don't have to
/// round-trip through the path.
///
/// Only implemented on Linux; other platforms return
/// [`FsType::Unknown`] (none of them have a writeback policy that
/// keys off this classification today).
#[cfg(target_os = "linux")]
/// filesystem signature. Never panics.
pub fn detect_fd(fd: std::os::unix::io::RawFd) -> FsType {
detect_fd_impl(fd)
}
/// Non-Linux stub for [`detect_fd`].
///
/// Always returns [`FsType::Unknown`]: only Linux keys its writeback
/// policy off this classification, so other platforms have nothing to
/// detect. The `fd` parameter is a bare `i32` rather than
/// `std::os::unix::io::RawFd` because this arm also compiles on Windows,
/// which has no `RawFd` — the universal integer keeps one signature across
/// all non-Linux targets. Unused on these targets (no caller invokes it),
/// hence `allow(dead_code)`.
#[cfg(not(target_os = "linux"))]
#[allow(dead_code)]
pub fn detect_fd(_fd: i32) -> FsType {
FsType::Unknown
linux::detect_fd_impl(fd)
}
#[cfg(test)]
mod tests {
use super::*;
use std::os::unix::io::AsRawFd;
#[test]
fn detect_does_not_panic_on_missing_path() {
// Non-existent path should fall through to Unknown, not panic.
let p = std::path::Path::new("/this/path/should/not/exist/freemkv-test");
let r = detect(p);
// We don't assert == Unknown because Windows' heuristic looks at
// the leading bytes and might still classify; just confirm the
// call returns rather than panicking.
let _ = r;
}
#[cfg(target_os = "macos")]
#[test]
fn macos_tmp_is_local() {
// `/tmp` on macOS dev rigs is APFS via the symlink to
// `/private/tmp`. Either way, never NFS.
let r = detect(std::path::Path::new("/tmp"));
assert!(
matches!(r, FsType::Local | FsType::Unknown),
"expected Local or Unknown for /tmp on macOS, got {r:?}",
);
}
#[cfg(target_os = "linux")]
#[test]
fn linux_tmp_is_local_or_unknown() {
fn detect_fd_tmp_is_local_or_unknown() {
// `/tmp` is tmpfs on most distros (which we recognise) but
// could be ext4 on others. NFS would be unusual.
let r = detect(std::path::Path::new("/tmp"));
let f = tempfile::tempfile().unwrap();
let r = detect_fd(f.as_raw_fd());
assert!(
matches!(r, FsType::Local | FsType::Unknown),
"expected Local or Unknown for /tmp on Linux, got {r:?}",
"expected Local or Unknown for a temp file on Linux, got {r:?}",
);
}
/// Real NFS exercise needs an actual NFS mount and isn't available
/// in CI. Kept here as a manual probe.
#[test]
#[ignore = "needs an NFS mount path (e.g. /mnt/nfs/...) to validate live"]
fn nfs_path_classified_as_nfs() {
// Operator passes the mount as FREEMKV_NFS_PROBE; gated behind
// `--ignored` because there's no portable NFS path.
let p = std::env::var("FREEMKV_NFS_PROBE").expect("set FREEMKV_NFS_PROBE");
assert_eq!(detect(std::path::Path::new(&p)), FsType::Nfs);
}
}
src/platform/fs_type/other.rs
-12
View File
@@ -1,12 +0,0 @@
//! Fallback fs-type detection for platforms without a specific impl.
//!
//! Always returns [`FsType::Unknown`]. Callers treat that as
//! "probably local" and pick `LocalFileSink`.
use std::path::Path;
use super::FsType;
pub(super) fn detect_impl(_path: &Path) -> FsType {
FsType::Unknown
}
src/platform/fs_type/windows.rs
-27
View File
@@ -1,27 +0,0 @@
//! Windows filesystem-type detection.
//!
//! Heuristic-only: any UNC path (`\\server\share\...`) is treated as a
//! network mount and bucketed into `Nfs`. Strictly, SMB is not NFS, but
//! the buffering-policy outcome is the same here — there is no platform
//! `WritebackFile` machinery on Windows yet, so both `Nfs` and `Local`
//! select `LocalFileSink`. A misclassification in either direction is
//! therefore harmless on Windows today: the sink choice does not change. A
//! proper `GetVolumeInformation` query is a Phase 4 concern.
use std::path::Path;
use super::FsType;
pub(super) fn detect_impl(path: &Path) -> FsType {
// `Path::starts_with("\\\\")` won't match because component-wise
// matching strips the prefix. Look at the raw OsStr instead.
let s = path.as_os_str();
// OsStr -> [u16] is the platform-correct way on Windows, but
// checking the leading bytes via `to_string_lossy` is good enough
// for a UNC prefix probe and works on any encoding.
let lossy = s.to_string_lossy();
if lossy.starts_with("\\\\") || lossy.starts_with("//") {
return FsType::Nfs;
}
FsType::Local
}
src/platform/mod.rs
+2 -1
View File
@@ -2,6 +2,7 @@
//!
//! Drive unlock no longer lives here — it moved out to the `freemkv-unlock`
//! crate (consumed via [`crate::unlock_bridge`]). This module now carries only
//! the filesystem-type detection used by the writeback / sink paths.
//! the Linux filesystem-type detection used by the writeback paths.
#[cfg(target_os = "linux")]
pub mod fs_type;
tests/lint_stop.rs
+1 -3
View File
@@ -535,7 +535,6 @@ const ALLOW_BASELINE: &[(&str, &str, usize)] = &[
r#"cfg_attr(not(target_os="linux"),allow(unused_variables))"#,
1,
),
("src/io/sink/mod.rs", r#"allow(dead_code)"#, 1),
(
"src/keys/arrival.rs",
r#"allow(clippy::too_many_arguments)"#,
@@ -564,9 +563,8 @@ const ALLOW_BASELINE: &[(&str, &str, usize)] = &[
(
"src/platform/fs_type/linux.rs",
r#"allow(clippy::unnecessary_cast)"#,
3,
2,
),
("src/platform/fs_type/mod.rs", r#"allow(dead_code)"#, 1),
("src/scsi/linux.rs", r#"allow(non_camel_case_types)"#, 1),
("src/scsi/mod.rs", r#"allow(dead_code)"#, 1),
(
tests/macos_platform_tests.rs
+2 -2
View File
@@ -1,7 +1,7 @@
//! macOS platform-integration tests that do NOT require an optical drive.
//!
//! These exercise the Mac-only code paths under `platform/fs_type/macos.rs`,
//! `io/platform_macos.rs`, and `io/writeback_file` end-to-end against the real
//! These exercise the Mac-only code paths under `io/platform_macos.rs`
//! and `io/writeback_file` end-to-end against the real
//! kernel. They run under `cargo test` without `--ignored`, so CI on macOS
//! (and the local `cargo test` a developer would run before pushing) covers
//! them automatically.