nix
nix copied to clipboard
Make `FileStat` a wrapper type instead of directly exporting it from libc
Currently, all the fields of this struct are numeric types cause we are directly
exporting libc::stat as FileStat
pub use libc::stat as FileStat;
But we do have wrapper types for uid_t, gid_t, and timespec. So I would like
change these original numeric types to our wrapper type.
What we can do
-
Convert the type of
st_uidtonix::unistd::Uid -
Convert the type of
st_gidtonix::unistd::Gid -
Convert the type of
st_atime(st_atime_nsec)/st_mtime(st_mtime_nsec)/st_ctime(st_ctime_nsec)tonix::sys::time::TimeSpec
Implementation
-
Make
struct FileStata wrapper type, and implementstd::convert::From<&libc::stat>forFileStat -
Reimplement
fstat(),fstatat()andlstat()
/// Information about a file, based on `libc::stat`
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct FileStat {
pub st_dev: u64,
pub st_ino: u64,
pub st_nlink: u64,
pub st_mode: mode_t,
pub st_uid: Uid,
pub st_gid: Gid,
pub st_rdev: u64,
pub st_size: i64,
pub st_blksize: i64,
pub st_blocks: i64,
pub st_atime: TimeSpec,
pub st_mtime: TimeSpec,
pub st_ctime: TimeSpec,
#[cfg(any(
target_os = "freebsd",
target_os = "dragonfly",
target_os = "netbsd",
target_os = "openbsd",
target_os = "macos",
target_os = "ios"
))]
pub st_flag: FileFlag,
#[cfg(any(
target_os = "freebsd",
target_os = "dragonfly",
target_os = "netbsd",
target_os = "openbsd",
target_os = "macos",
target_os = "ios"
))]
pub st_gen: i32,
#[cfg(any(
target_os = "freebsd",
target_os = "netbsd",
target_os = "openbsd",
target_os = "macos",
target_os = "ios"
))]
pub st_birthtime: TimeSpec,
}
impl From<&libc::stat> for FileStat {
fn from(st: &libc::stat) -> Self {
Self {
st_dev: st.st_dev,
st_ino: st.st_ino,
st_nlink: st.st_nlink,
st_mode: st.st_mode,
st_uid: Uid::from_raw(st.st_uid),
st_gid: Gid::from_raw(st.st_gid),
st_rdev: st.st_rdev,
st_size: st.st_size,
st_blksize: st.st_blksize,
st_blocks: st.st_blocks,
st_atime: TimeSpec::from_timespec(timespec{ tv.sec: st.st_atime, tv_nsec: st.st_atime_nsec}),
st_mtime: TimeSpec::from_timespec(timespec{ tv.sec: st.st_mtime, tv_nsec: st.st_mtime_nsec}),
st_ctime: TimeSpec::from_timespec(timespec{ tv.sec: st.st_ctime, tv_nsec: st.st_ctime_nsec}),
#[cfg(any(
target_os = "freebsd",
target_os = "dragonfly",
target_os = "netbsd",
target_os = "openbsd",
target_os = "macos",
target_os = "ios"
))]
st_flag: FileFlag::from_bits_truncate(st.st_flags),
#[cfg(any(
target_os = "freebsd",
target_os = "dragonfly",
target_os = "netbsd",
target_os = "openbsd",
target_os = "macos",
target_os = "ios"
))]
st_gen: st.st_gen,
#[cfg(any(
target_os = "freebsd",
target_os = "netbsd",
target_os = "openbsd",
target_os = "macos",
target_os = "ios"
))]
st_birthtime: TimeSpec::from_timespec(timespec{ tv.sec: st.st_birthtime, tv_nsec: st.st_birthtimensec}),
}
}
}
The implementations of these 3 functions are easy, we can simply call FileStat::from
on the raw type:
pub fn stat<P: ?Sized + NixPath>(path: &P) -> Result<FileStat> {
let mut dst = mem::MaybeUninit::uninit();
let res = path.with_nix_path(|cstr| unsafe {
libc::stat(cstr.as_ptr(), dst.as_mut_ptr())
})?;
Errno::result(res)?;
Ok(FileStat::from(unsafe { &dst.assume_init() }))
}
pub fn lstat<P: ?Sized + NixPath>(path: &P) -> Result<FileStat> {
let mut dst = mem::MaybeUninit::uninit();
let res = path.with_nix_path(|cstr| unsafe {
libc::lstat(cstr.as_ptr(), dst.as_mut_ptr())
})?;
Errno::result(res)?;
Ok(FileStat::from(unsafe { &dst.assume_init() }))
}
pub fn fstat(fd: RawFd) -> Result<FileStat> {
let mut dst = mem::MaybeUninit::uninit();
let res = unsafe { libc::fstat(fd, dst.as_mut_ptr()) };
Errno::result(res)?;
Ok(FileStat::from(unsafe { &dst.assume_init() }))
}
Should I do this? I would like to hear some thoughts:)
BTW, we also have Mode and SFlag, what about making st_mode a wrapper type like this:
struct NewMode {
mode: Mode,
flag: SFlag,
}
so that:
/// Information about a file, based on `libc::stat`
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub struct FileStat {
...
pub st_mode: NewMode,
...
}
Kinda think we should also remove the st_ prefix in these fields if we create our own wrapper type, like nix::unistd::User did.
Yeah, I've noticed this too. It's annoying. I think they originally did it this way because this was one of the very first Nix functions ever written, back when Rust wasn't even 1.0 yet. Your proposed API looks like an improvement. But what about instead making FileStat a newtype, like this?
#[repr(transparent)]
struct FileStat(libc::stat);
And then giving it accessor methods to get the fields in the appropriate types, like:
impl FileStat {
pub fn user(&self) -> Uid {...}
pub fn atime(&self) -> &TimeSpec {...}
}
#[repr(transparent)] struct FileStat(libc::stat);
This looks good, as it adheres to CONVENTIONS: libc constants, functions and structs.
pub fn atime(&self) -> &TimeSpec {...}
Little bit confused about this signature, it returns a reference to a TimeSpec, this makes sense as the return value should be bound to our FileStat, but how can we implement this? FileStat does not has a TimeSpec inside it
// libc::stat
#[repr(C)]
pub struct stat {
...
pub st_atime: time_t,
pub st_atime_nsec: i64,
...
}
Ah, you're right. It'll have to return by value then.
Ok, then i will work on this issue:)