From 2be51a3e93ee9c284533e29d8dc7de05f210bc1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Sat, 17 Apr 2021 21:06:34 +0200 Subject: [PATCH 01/28] add accessors for the underlying stat data This anticipates the statx() interface I'll add soon. Breaking change: Adds a 'SimpleType::Unknown' which can be returned when the filetype can't be determinded. Code matching on SimpleType may add a clause for this when there is no catch-all. --- src/filetype.rs | 2 ++ src/metadata.rs | 79 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/src/filetype.rs b/src/filetype.rs index efaedbe..4ee98e4 100644 --- a/src/filetype.rs +++ b/src/filetype.rs @@ -7,6 +7,8 @@ use std::fs::Metadata; /// this simplified enum that works for many appalications. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SimpleType { + /// Entry is of unknown type + Unknown, /// Entry is a symlink Symlink, /// Entry is a directory diff --git a/src/metadata.rs b/src/metadata.rs index de7cc22..f7259c0 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -2,6 +2,7 @@ use std::fs::Permissions; use std::os::unix::fs::PermissionsExt; use libc; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use crate::SimpleType; @@ -45,12 +46,90 @@ impl Metadata { pub fn len(&self) -> u64 { self.stat.st_size as u64 } + /// Return low level file type, if available + pub fn file_type(&self) -> Option { + Some(self.stat.st_mode & libc::S_IFMT) + } + /// Return device node, if available + pub fn ino(&self) -> Option { + Some(self.stat.st_ino) + } + /// Return device node major of the file, if available + pub fn dev_major(&self) -> Option { + Some(unsafe { libc::major(self.stat.st_dev) } as u64) + } + /// Return device node minor of the file, if available + pub fn dev_minor(&self) -> Option { + Some(unsafe { libc::minor(self.stat.st_dev) } as u64) + } + /// Return device node major of an device descriptor, if available + pub fn rdev_major(&self) -> Option { + match self.file_type()? { + libc::S_IFBLK | libc::S_IFCHR => Some(unsafe { libc::major(self.stat.st_rdev) } as u64), + _ => None, + } + } + /// Return device node minor of an device descriptor, if available + pub fn rdev_minor(&self) -> Option { + match self.file_type()? { + libc::S_IFBLK | libc::S_IFCHR => Some(unsafe { libc::minor(self.stat.st_rdev) } as u64), + _ => None, + } + } + /// Return preferered I/O Blocksize, if available + pub fn blksize(&self) -> Option { + Some(self.stat.st_blksize as u32) + } + /// Return the number of 512 bytes blocks, if available + pub fn blocks(&self) -> Option { + Some(self.stat.st_blocks as u64) + } + /// Returns file size (same as len() but Option), if available + pub fn size(&self) -> Option { + Some(self.stat.st_size as u64) + } + /// Returns number of hard-links, if available + pub fn nlink(&self) -> Option { + Some(self.stat.st_nlink as u32) + } + /// Returns user id, if available + pub fn uid(&self) -> Option { + Some(self.stat.st_uid as u32) + } + /// Returns group id, if available + pub fn gid(&self) -> Option { + Some(self.stat.st_gid as u32) + } + /// Returns mode bits, if available + pub fn file_mode(&self) -> Option { + Some(self.stat.st_mode & 0o7777) + } + /// Returns last access time, if available + pub fn atime(&self) -> Option { + Some(unix_systemtime(self.stat.st_atime, self.stat.st_atime_nsec)) + } + /// Returns creation, if available + pub fn btime(&self) -> Option { + None + } + /// Returns last status change time, if available + pub fn ctime(&self) -> Option { + Some(unix_systemtime(self.stat.st_ctime, self.stat.st_ctime_nsec)) + } + /// Returns last modification time, if available + pub fn mtime(&self) -> Option { + Some(unix_systemtime(self.stat.st_mtime, self.stat.st_mtime_nsec)) + } } pub fn new(stat: libc::stat) -> Metadata { Metadata { stat: stat } } +fn unix_systemtime(sec: libc::time_t, nsec: i64) -> SystemTime { + UNIX_EPOCH + Duration::from_secs(sec as u64) + Duration::from_nanos(nsec as u64) +} + #[cfg(test)] mod test { use super::*; From bba06871f3751d1b01359ed14c4a6cc9242ba4b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Sun, 18 Apr 2021 02:36:41 +0200 Subject: [PATCH 02/28] FIX: make use of SimpleType::Unknown in simple_type() --- src/metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index f7259c0..e71683e 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -18,11 +18,11 @@ pub struct Metadata { impl Metadata { /// Returns simplified type of the directory entry pub fn simple_type(&self) -> SimpleType { - let typ = self.stat.st_mode & libc::S_IFMT; - match typ { + match self.file_type().unwrap_or(0) { libc::S_IFREG => SimpleType::File, libc::S_IFDIR => SimpleType::Dir, libc::S_IFLNK => SimpleType::Symlink, + 0 => SimpleType::Unknown, _ => SimpleType::Other, } } From c502cc24da12c97fd872e20353f19ffafe0f05ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Sun, 18 Apr 2021 02:38:24 +0200 Subject: [PATCH 03/28] FIX: use u32 for the 'dev' values --- src/metadata.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index e71683e..f4c2abc 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -55,24 +55,24 @@ impl Metadata { Some(self.stat.st_ino) } /// Return device node major of the file, if available - pub fn dev_major(&self) -> Option { - Some(unsafe { libc::major(self.stat.st_dev) } as u64) + pub fn dev_major(&self) -> Option { + Some(unsafe { libc::major(self.stat.st_dev) }) } /// Return device node minor of the file, if available - pub fn dev_minor(&self) -> Option { - Some(unsafe { libc::minor(self.stat.st_dev) } as u64) + pub fn dev_minor(&self) -> Option { + Some(unsafe { libc::minor(self.stat.st_dev) }) } /// Return device node major of an device descriptor, if available - pub fn rdev_major(&self) -> Option { + pub fn rdev_major(&self) -> Option { match self.file_type()? { - libc::S_IFBLK | libc::S_IFCHR => Some(unsafe { libc::major(self.stat.st_rdev) } as u64), + libc::S_IFBLK | libc::S_IFCHR => Some(unsafe { libc::major(self.stat.st_rdev) }), _ => None, } } /// Return device node minor of an device descriptor, if available - pub fn rdev_minor(&self) -> Option { + pub fn rdev_minor(&self) -> Option { match self.file_type()? { - libc::S_IFBLK | libc::S_IFCHR => Some(unsafe { libc::minor(self.stat.st_rdev) } as u64), + libc::S_IFBLK | libc::S_IFCHR => Some(unsafe { libc::minor(self.stat.st_rdev) }), _ => None, } } From c8f364dc981bb7d6a009bdf5abebf9a75f495c38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Sun, 18 Apr 2021 03:04:46 +0200 Subject: [PATCH 04/28] Changes to make macos happy --- src/metadata.rs | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index f4c2abc..a7a9e9c 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -18,7 +18,7 @@ pub struct Metadata { impl Metadata { /// Returns simplified type of the directory entry pub fn simple_type(&self) -> SimpleType { - match self.file_type().unwrap_or(0) { + match self.file_type().unwrap_or(0) as libc::mode_t { libc::S_IFREG => SimpleType::File, libc::S_IFDIR => SimpleType::Dir, libc::S_IFLNK => SimpleType::Symlink, @@ -48,7 +48,7 @@ impl Metadata { } /// Return low level file type, if available pub fn file_type(&self) -> Option { - Some(self.stat.st_mode & libc::S_IFMT) + Some(self.stat.st_mode as u32 & libc::S_IFMT as u32) } /// Return device node, if available pub fn ino(&self) -> Option { @@ -56,23 +56,23 @@ impl Metadata { } /// Return device node major of the file, if available pub fn dev_major(&self) -> Option { - Some(unsafe { libc::major(self.stat.st_dev) }) + Some(major(self.stat.st_dev)) } /// Return device node minor of the file, if available pub fn dev_minor(&self) -> Option { - Some(unsafe { libc::minor(self.stat.st_dev) }) + Some(minor(self.stat.st_dev)) } /// Return device node major of an device descriptor, if available pub fn rdev_major(&self) -> Option { - match self.file_type()? { - libc::S_IFBLK | libc::S_IFCHR => Some(unsafe { libc::major(self.stat.st_rdev) }), + match self.file_type()? as libc::mode_t { + libc::S_IFBLK | libc::S_IFCHR => Some(major(self.stat.st_rdev)), _ => None, } } /// Return device node minor of an device descriptor, if available pub fn rdev_minor(&self) -> Option { - match self.file_type()? { - libc::S_IFBLK | libc::S_IFCHR => Some(unsafe { libc::minor(self.stat.st_rdev) }), + match self.file_type()? as libc::mode_t { + libc::S_IFBLK | libc::S_IFCHR => Some(minor(self.stat.st_rdev)), _ => None, } } @@ -102,7 +102,7 @@ impl Metadata { } /// Returns mode bits, if available pub fn file_mode(&self) -> Option { - Some(self.stat.st_mode & 0o7777) + Some(self.stat.st_mode as u32 & 0o7777) } /// Returns last access time, if available pub fn atime(&self) -> Option { @@ -130,6 +130,28 @@ fn unix_systemtime(sec: libc::time_t, nsec: i64) -> SystemTime { UNIX_EPOCH + Duration::from_secs(sec as u64) + Duration::from_nanos(nsec as u64) } +#[cfg(not(target_os = "macos"))] +pub fn major(dev: libc::dev_t) -> u32 { + unsafe { libc::major(dev) } +} + +#[cfg(not(target_os = "macos"))] +pub fn minor(dev: libc::dev_t) -> u32 { + unsafe { libc::minor(dev) } +} + +// major/minor are not in rust's darwin libc (why) +// see https://github.com/apple/darwin-xnu/blob/main/bsd/sys/types.h +#[cfg(target_os = "macos")] +pub fn major(dev: libc::dev_t) -> u32 { + (dev as u32 >> 24) & 0xff +} + +#[cfg(target_os = "macos")] +pub fn minor(dev: libc::dev_t) -> u32 { + dev as u32 & 0xffffff +} + #[cfg(test)] mod test { use super::*; From 39fc8039e19b27a7032374a4c4e8b0500b0c3c24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Mon, 19 Apr 2021 08:38:11 +0200 Subject: [PATCH 05/28] FIX: replace ino_t with u64 to comply with statx --- src/metadata.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index a7a9e9c..275cdb4 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -51,8 +51,8 @@ impl Metadata { Some(self.stat.st_mode as u32 & libc::S_IFMT as u32) } /// Return device node, if available - pub fn ino(&self) -> Option { - Some(self.stat.st_ino) + pub fn ino(&self) -> Option { + Some(self.stat.st_ino as u64) } /// Return device node major of the file, if available pub fn dev_major(&self) -> Option { From 815357e37fc7b982573e17d3fd18fa398a83fb5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Mon, 19 Apr 2021 22:00:56 +0200 Subject: [PATCH 06/28] Add/Refactor Dir::list() -> DirIter ctor dir.list() consumes the dir and creates a DirIter. The underlying 'dup()' operation now becomes explicit, one may need to call try_clone() first. the list_self() and list_dir() using this now but are merely convinience wrappers and may (or may not) deprecated in future. Rationale: this simplifies the code a bit (list::open_dir() removed) and is closer to the low level semantics. --- src/dir.rs | 15 ++++++++++----- src/list.rs | 14 +------------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index 3bed451..add0382 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -7,8 +7,8 @@ use std::os::unix::ffi::{OsStringExt}; use std::path::{PathBuf}; use libc; +use crate::list::{open_dirfd, DirIter}; use crate::metadata::{self, Metadata}; -use crate::list::{DirIter, open_dir, open_dirfd}; use crate::{Dir, AsPath}; @@ -53,15 +53,20 @@ impl Dir { /// List subdirectory of this dir /// /// You can list directory itself with `list_self`. + // TODO(cehteh): may be deprecated in favor of list() pub fn list_dir(&self, path: P) -> io::Result { - open_dir(self, to_cstr(path)?.as_ref()) + self.sub_dir(path)?.list() } /// List this dir + // TODO(cehteh): may be deprecated in favor of list() pub fn list_self(&self) -> io::Result { - unsafe { - open_dirfd(libc::dup(self.0)) - } + self.try_clone()?.list() + } + + /// Create a DirIter from a Dir + pub fn list(self) -> io::Result { + open_dirfd(self.0) } /// Open subdirectory diff --git a/src/list.rs b/src/list.rs index 5b4d3cd..3d4889c 100644 --- a/src/list.rs +++ b/src/list.rs @@ -5,8 +5,7 @@ use std::os::unix::ffi::OsStrExt; use libc; -use crate::{Dir, Entry, SimpleType}; - +use crate::{Entry, SimpleType}; // We have such weird constants because C types are ugly const DOT: [libc::c_char; 2] = [b'.' as libc::c_char, 0]; @@ -104,17 +103,6 @@ pub fn open_dirfd(fd: libc::c_int) -> io::Result { } } -pub fn open_dir(dir: &Dir, path: &CStr) -> io::Result { - let dir_fd = unsafe { - libc::openat(dir.0, path.as_ptr(), libc::O_DIRECTORY|libc::O_CLOEXEC) - }; - if dir_fd < 0 { - Err(io::Error::last_os_error()) - } else { - open_dirfd(dir_fd) - } -} - impl Iterator for DirIter { type Item = io::Result; fn next(&mut self) -> Option { From 8647f9383927f2f52a8a0d65c15ff148fe2b36ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Tue, 20 Apr 2021 02:09:33 +0200 Subject: [PATCH 07/28] FIX: fixes previous commit, adds special O_PATH funcs * Forgot to forget the Dir handle, thus it would been dropped. * On linux O_PATH was passed when Dir descriptors where opened. This is generally a good thing but broke the refactored list(). This also shown that O_PATH has different enough semantics to be problematic vs. opening handles without (as on other OS'es). Changed this to Dir::open() and Dir::sub_dir() default to opening without O_PATH (but O_DIRECTORY see remarks). * added Dir::open_path() and Dir::sub_dir_path() with O_PATH (urgh, better idea for naming? open_protected() or something like that?) which offer the O_PATH feature on linux. --- src/dir.rs | 102 ++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 78 insertions(+), 24 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index add0382..2490bf0 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -12,11 +12,12 @@ use crate::metadata::{self, Metadata}; use crate::{Dir, AsPath}; -#[cfg(target_os="linux")] -const BASE_OPEN_FLAGS: libc::c_int = libc::O_PATH|libc::O_CLOEXEC; -#[cfg(target_os="freebsd")] -const BASE_OPEN_FLAGS: libc::c_int = libc::O_DIRECTORY|libc::O_CLOEXEC; -#[cfg(not(any(target_os="linux", target_os="freebsd")))] +// NOTE(cehteh): removed O_PATH since it is linux only and highly unportable (semantics can't be emulated) +// but see open_path() below. +#[cfg(any(target_os = "linux", target_os = "freebsd"))] +const BASE_OPEN_FLAGS: libc::c_int = libc::O_DIRECTORY | libc::O_CLOEXEC; +// NOTE(cehteh): O_DIRECTORY is defined in posix, what is the reason for not using it? +#[cfg(not(any(target_os = "linux", target_os = "freebsd")))] const BASE_OPEN_FLAGS: libc::c_int = libc::O_CLOEXEC; impl Dir { @@ -36,13 +37,18 @@ impl Dir { /// Open a directory descriptor at specified path // TODO(tailhook) maybe accept only absolute paths? pub fn open(path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref()) + Dir::_open(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS) } - fn _open(path: &CStr) -> io::Result { - let fd = unsafe { - libc::open(path.as_ptr(), BASE_OPEN_FLAGS) - }; + /// Open a directory descriptor at specified path with O_PATH (linux only) + /// A descriptor obtained with this flag is restricted to do only certain operations (see: man 2 open) + #[cfg(any(target_os = "linux"))] + pub fn open_path(path: P) -> io::Result { + Dir::_open(to_cstr(path)?.as_ref(), libc::O_PATH | BASE_OPEN_FLAGS) + } + + fn _open(path: &CStr, flags: libc::c_int) -> io::Result { + let fd = unsafe { libc::open(path.as_ptr(), flags) }; if fd < 0 { Err(io::Error::last_os_error()) } else { @@ -66,7 +72,9 @@ impl Dir { /// Create a DirIter from a Dir pub fn list(self) -> io::Result { - open_dirfd(self.0) + let fd = self.0; + std::mem::forget(self); + open_dirfd(fd) } /// Open subdirectory @@ -76,15 +84,23 @@ impl Dir { /// /// [`read_link`]: #method.read_link pub fn sub_dir(&self, path: P) -> io::Result { - self._sub_dir(to_cstr(path)?.as_ref()) + self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW) } - fn _sub_dir(&self, path: &CStr) -> io::Result { - let fd = unsafe { - libc::openat(self.0, - path.as_ptr(), - BASE_OPEN_FLAGS|libc::O_NOFOLLOW) - }; + /// Open subdirectory with O_PATH (linux only) + /// + /// Note that this method does not resolve symlinks by default, so you may have to call + /// A descriptor obtained with this flag is restricted to do only certain operations (see: man 2 open) + /// [`read_link`] to resolve the real path first. + /// + /// [`read_link`]: #method.read_link + #[cfg(any(target_os = "linux"))] + pub fn sub_dir_path(&self, path: P) -> io::Result { + self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW | libc::O_PATH) + } + + fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { + let fd = unsafe { libc::openat(self.0, path.as_ptr(), flags) }; if fd < 0 { Err(io::Error::last_os_error()) } else { @@ -634,7 +650,8 @@ mod test { } #[test] - #[cfg_attr(target_os="freebsd", should_panic(expected="Not a directory"))] + #[cfg_attr(any(target_os = "freebsd", target_os = "linux"), should_panic(expected = "Not a directory"))] + // NOTE(cehteh): should fail in all cases! see O_DIRECTORY at the top fn test_open_file() { Dir::open("src/lib.rs").unwrap(); } @@ -666,13 +683,50 @@ mod test { #[test] fn test_list() { + let dir = Dir::open("src").unwrap(); + let me = dir.list().unwrap(); + assert!(me + .collect::, _>>() + .unwrap() + .iter() + .find(|x| { x.file_name() == Path::new("lib.rs").as_os_str() }) + .is_some()); + } + + #[test] + fn test_list_self() { + let dir = Dir::open("src").unwrap(); + let me = dir.list_self().unwrap(); + assert!(me + .collect::, _>>() + .unwrap() + .iter() + .find(|x| { x.file_name() == Path::new("lib.rs").as_os_str() }) + .is_some()); + } + + #[test] + fn test_list_dot() { let dir = Dir::open("src").unwrap(); let me = dir.list_dir(".").unwrap(); - assert!(me.collect::, _>>().unwrap() - .iter().find(|x| { - x.file_name() == Path::new("lib.rs").as_os_str() - }) - .is_some()); + assert!(me + .collect::, _>>() + .unwrap() + .iter() + .find(|x| { x.file_name() == Path::new("lib.rs").as_os_str() }) + .is_some()); + } + + #[test] + fn test_list_dir() { + let dir = Dir::open(".").unwrap(); + let me = dir.list_dir("src").unwrap(); + assert!(me + .collect::, _>>() + .unwrap() + .iter() + .find(|x| { x.file_name() == Path::new("lib.rs").as_os_str() }) + .is_some()); } #[test] From d567fb1aafee57a5880a7a8f9dd7a89c186278fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 21 Apr 2021 14:29:45 +0200 Subject: [PATCH 08/28] rename *_path to *_lite, improve docs, provide impl for non linux 'Lite' file descriptors are possibly a better naming of what O_PATH does. This introduces then and implements them portable. On systems which do not support O_PATH normal file descriptors are used. --- src/dir.rs | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index 2490bf0..894e4f1 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -40,13 +40,22 @@ impl Dir { Dir::_open(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS) } - /// Open a directory descriptor at specified path with O_PATH (linux only) - /// A descriptor obtained with this flag is restricted to do only certain operations (see: man 2 open) + /// Open a 'lite' directory descriptor at specified path + /// A descriptor obtained with this flag is restricted to do only certain operations: + /// - It may be used as anchor for opening sub-objects + /// - One can query metadata of this directory + /// Using this descriptor for iterating over the content is unspecified. + /// Uses O_PATH on Linux #[cfg(any(target_os = "linux"))] - pub fn open_path(path: P) -> io::Result { + pub fn open_lite(path: P) -> io::Result { Dir::_open(to_cstr(path)?.as_ref(), libc::O_PATH | BASE_OPEN_FLAGS) } + #[cfg(not(target_os = "linux"))] + pub fn open_lite(path: P) -> io::Result { + Dir::_open(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS) + } + fn _open(path: &CStr, flags: libc::c_int) -> io::Result { let fd = unsafe { libc::open(path.as_ptr(), flags) }; if fd < 0 { @@ -87,18 +96,28 @@ impl Dir { self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW) } - /// Open subdirectory with O_PATH (linux only) + /// Open subdirectory with a 'lite' descriptor at specified path + /// A descriptor obtained with this flag is restricted to do only certain operations: + /// - It may be used as anchor for opening sub-objects + /// - One can query metadata of this directory + /// Using this descriptor for iterating over the content is unspecified. + /// Uses O_PATH on Linux /// /// Note that this method does not resolve symlinks by default, so you may have to call - /// A descriptor obtained with this flag is restricted to do only certain operations (see: man 2 open) + /// /// [`read_link`] to resolve the real path first. /// /// [`read_link`]: #method.read_link #[cfg(any(target_os = "linux"))] - pub fn sub_dir_path(&self, path: P) -> io::Result { + pub fn sub_dir_lite(&self, path: P) -> io::Result { self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW | libc::O_PATH) } + #[cfg(not(target_os = "linux"))] + pub fn sub_dir_lite(&self, path: P) -> io::Result { + self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW) + } + fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { let fd = unsafe { libc::openat(self.0, path.as_ptr(), flags) }; if fd < 0 { From e5e9c5b7747d14f52184f19bd5fd8301f0fbbbb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 21 Apr 2021 15:06:58 +0200 Subject: [PATCH 09/28] New: clone_dirfd() may fix #34, ADD: FdType machinery, libc_ok() * With FdType/fd_type() one can determine the kind of an underlying file descriptor. Lite descriptors are implemented only in Linux (for now). When O_DIRECTORY is supported it uses fcntl() in favo over stat() * clone_dirfd() tries to do 'the right thing' for duplicating FD's * libc_ok() is a simple wraper for libc calls that return -1 on error, I will refactor the code in the next commits to make use of that more. Please test this! So far it works for me on Linux. --- src/dir.rs | 88 ++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 75 insertions(+), 13 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index 894e4f1..54270df 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -497,29 +497,91 @@ impl Dir { /// descriptor. The returned `Dir` will take responsibility for /// closing it when it goes out of scope. pub unsafe fn from_raw_fd_checked(fd: RawFd) -> io::Result { - let mut stat = mem::zeroed(); - let res = libc::fstat(fd, &mut stat); - if res < 0 { - Err(io::Error::last_os_error()) - } else { - match stat.st_mode & libc::S_IFMT { - libc::S_IFDIR => Ok(Dir(fd)), - _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)) - } + match fd_type(fd)? { + FdType::NormalDir | FdType::LiteDir => Ok(Dir(fd)), + _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } } /// Creates a new independently owned handle to the underlying directory. pub fn try_clone(&self) -> io::Result { - let fd = unsafe { libc::dup(self.0) }; - if fd == -1 { - Err(io::Error::last_os_error()) + Ok(Dir(clone_dirfd(self.0)?)) + } + + //TODO(cehteh): clone/conversion full<->lite +} + +const CURRENT_DIRECTORY: [libc::c_char; 2] = [b'.' as libc::c_char, 0]; + +fn clone_dirfd(fd: libc::c_int) -> io::Result { + unsafe { + match fd_type(fd)? { + FdType::NormalDir => libc_ok(libc::openat( + fd, + &CURRENT_DIRECTORY as *const libc::c_char, + BASE_OPEN_FLAGS, + )), + #[cfg(target_os = "linux")] + FdType::LiteDir => libc_ok(libc::dup(fd)), + _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), + } + } +} + +enum FdType { + NormalDir, + LiteDir, + Other, +} + +// OSes with O_DIRECTORY can use fcntl() +// Linux hash O_PATH +#[cfg(target_os = "linux")] +fn fd_type(fd: libc::c_int) -> io::Result { + let flags = unsafe { libc_ok(libc::fcntl(fd, libc::F_GETFL))? }; + if flags & libc::O_DIRECTORY != 0 { + if flags & libc::O_PATH != 0 { + Ok(FdType::LiteDir) } else { - unsafe { Self::from_raw_fd_checked(fd) } + Ok(FdType::NormalDir) + } + } else { + Ok(FdType::Other) + } +} + +#[cfg(target_os = "freebsd")] +fn fd_type(fd: libc::c_int) -> io::Result { + let flags = unsafe { libc_ok(libc::fcntl(fd, libc::F_GETFL))? }; + if flags & libc::O_DIRECTORY != 0 { + Ok(FdType::NormalDir) + } else { + Ok(FdType::Other) + } +} + +// OSes without O_DIRECTORY use stat() +#[cfg(not(any(target_os = "linux", target_os = "freebsd")))] +fn fd_type(fd: libc::c_int) -> io::Result { + unsafe { + let mut stat = mem::zeroed(); // TODO(cehteh): uninit + libc_ok(libc::fstat(fd, &mut stat))?; + match stat.st_mode & libc::S_IFMT { + libc::S_IFDIR => Ok(FdType::NormalDir), + _ => Ok(FdType::Other), } } } +#[inline] +fn libc_ok(ret: libc::c_int) -> io::Result { + if ret != -1 { + Ok(ret) + } else { + Err(io::Error::last_os_error()) + } +} + /// Rename (move) a file between directories /// /// Files must be on a single filesystem anyway. This funtion does **not** From 9d48913c3aa4bf49d8764d3e81f9b67cffb9e0e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 21 Apr 2021 16:00:14 +0200 Subject: [PATCH 10/28] cosmetics: use libc_ok() where applicable This simplifies the code a bit, in 'release' the same output is generated. debug builds may not inline the libc_ok() and be slightly larger. --- src/dir.rs | 100 +++++++++++++++++------------------------------------ 1 file changed, 32 insertions(+), 68 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index 54270df..c5dd427 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -57,12 +57,8 @@ impl Dir { } fn _open(path: &CStr, flags: libc::c_int) -> io::Result { - let fd = unsafe { libc::open(path.as_ptr(), flags) }; - if fd < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(Dir(fd)) - } + let fd = unsafe { libc_ok(libc::open(path.as_ptr(), flags))? }; + Ok(Dir(fd)) } /// List subdirectory of this dir @@ -119,12 +115,7 @@ impl Dir { } fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { - let fd = unsafe { libc::openat(self.0, path.as_ptr(), flags) }; - if fd < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(Dir(fd)) - } + Ok(Dir(unsafe { libc_ok(libc::openat(self.0, path.as_ptr(), flags))? })) } /// Read link in this directory @@ -344,14 +335,12 @@ impl Dir { // variadic in the signature. Since integers are not implicitly // promoted as they are in C this would break on Freebsd where // *mode_t* is an alias for `uint16_t`. - let res = libc::openat(self.0, path.as_ptr(), - flags|libc::O_CLOEXEC|libc::O_NOFOLLOW, - mode as libc::c_uint); - if res < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(File::from_raw_fd(res)) - } + let res = libc_ok( + libc::openat(self.0, path.as_ptr(), + flags|libc::O_CLOEXEC|libc::O_NOFOLLOW, + mode as libc::c_uint) + )?; + Ok(File::from_raw_fd(res)) } } @@ -383,13 +372,9 @@ impl Dir { } fn _create_dir(&self, path: &CStr, mode: libc::mode_t) -> io::Result<()> { unsafe { - let res = libc::mkdirat(self.0, path.as_ptr(), mode); - if res < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + libc_ok(libc::mkdirat(self.0, path.as_ptr(), mode))?; } + Ok(()) } /// Rename a file in this directory to another name (keeping same dir) @@ -432,19 +417,16 @@ impl Dir { } fn _unlink(&self, path: &CStr, flags: libc::c_int) -> io::Result<()> { unsafe { - let res = libc::unlinkat(self.0, path.as_ptr(), flags); - if res < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + libc_ok(libc::unlinkat(self.0, path.as_ptr(), flags))?; } + Ok(()) } /// Get the path of this directory (if possible) /// /// This uses symlinks in `/proc/self`, they sometimes may not be /// available so use with care. + // FIXME(cehteh): proc stuff isn't portable pub fn recover_path(&self) -> io::Result { let fd = self.0; if fd != libc::AT_FDCWD { @@ -466,27 +448,18 @@ impl Dir { } fn _stat(&self, path: &CStr, flags: libc::c_int) -> io::Result { unsafe { - let mut stat = mem::zeroed(); - let res = libc::fstatat(self.0, path.as_ptr(), - &mut stat, flags); - if res < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(metadata::new(stat)) - } + let mut stat = mem::zeroed(); // TODO(cehteh): uninit + libc_ok(libc::fstatat(self.0, path.as_ptr(), &mut stat, flags))?; + Ok(metadata::new(stat)) } } /// Returns the metadata of the directory itself. pub fn self_metadata(&self) -> io::Result { unsafe { - let mut stat = mem::zeroed(); - let res = libc::fstat(self.0, &mut stat); - if res < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(metadata::new(stat)) - } + let mut stat = mem::zeroed(); // TODO(cehteh): uninit + libc_ok(libc::fstat(self.0, &mut stat))?; + Ok(metadata::new(stat)) } } @@ -593,18 +566,11 @@ pub fn rename(old_dir: &Dir, old: P, new_dir: &Dir, new: R) _rename(old_dir, to_cstr(old)?.as_ref(), new_dir, to_cstr(new)?.as_ref()) } -fn _rename(old_dir: &Dir, old: &CStr, new_dir: &Dir, new: &CStr) - -> io::Result<()> -{ +fn _rename(old_dir: &Dir, old: &CStr, new_dir: &Dir, new: &CStr) -> io::Result<()> { unsafe { - let res = libc::renameat(old_dir.0, old.as_ptr(), - new_dir.0, new.as_ptr()); - if res < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + libc_ok(libc::renameat(old_dir.0, old.as_ptr(), new_dir.0, new.as_ptr()))?; } + Ok(()) } /// Create a hardlink to a file @@ -624,19 +590,17 @@ pub fn hardlink(old_dir: &Dir, old: P, new_dir: &Dir, new: R) 0) } -fn _hardlink(old_dir: &Dir, old: &CStr, new_dir: &Dir, new: &CStr, - flags: libc::c_int) - -> io::Result<()> -{ +fn _hardlink( + old_dir: &Dir, + old: &CStr, + new_dir: &Dir, + new: &CStr, + flags: libc::c_int, +) -> io::Result<()> { unsafe { - let res = libc::linkat(old_dir.0, old.as_ptr(), - new_dir.0, new.as_ptr(), flags); - if res < 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } + libc_ok(libc::linkat(old_dir.0, old.as_ptr(), new_dir.0, new.as_ptr(), flags))?; } + Ok(()) } /// Rename (move) a file between directories with flags From fcf92fa7ec0529d6499ef37c1f4bd7c144098b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 21 Apr 2021 16:31:18 +0200 Subject: [PATCH 11/28] add clone_upgrade()/clone_downgrade(), fix list_self() This up/downgrade cloning converts into normal/lite handles which was missing before. I hope this fixes #34 finally. --- src/dir.rs | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index c5dd427..66bf82d 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -17,6 +17,7 @@ use crate::{Dir, AsPath}; #[cfg(any(target_os = "linux", target_os = "freebsd"))] const BASE_OPEN_FLAGS: libc::c_int = libc::O_DIRECTORY | libc::O_CLOEXEC; // NOTE(cehteh): O_DIRECTORY is defined in posix, what is the reason for not using it? +// TODO(cehteh): on systems that do not support O_DIRECTORY a runtime stat-check is required #[cfg(not(any(target_os = "linux", target_os = "freebsd")))] const BASE_OPEN_FLAGS: libc::c_int = libc::O_CLOEXEC; @@ -72,10 +73,11 @@ impl Dir { /// List this dir // TODO(cehteh): may be deprecated in favor of list() pub fn list_self(&self) -> io::Result { - self.try_clone()?.list() + self.clone_upgrade()?.list() } /// Create a DirIter from a Dir + /// Dir must not be a 'Lite' handle pub fn list(self) -> io::Result { let fd = self.0; std::mem::forget(self); @@ -477,11 +479,21 @@ impl Dir { } /// Creates a new independently owned handle to the underlying directory. + /// The new handle has the same (Normal/Lite) semantics as the original handle. pub fn try_clone(&self) -> io::Result { Ok(Dir(clone_dirfd(self.0)?)) } - //TODO(cehteh): clone/conversion full<->lite + /// Creates a new 'Normal' independently owned handle to the underlying directory. + pub fn clone_upgrade(&self) -> io::Result { + Ok(Dir(clone_dirfd_upgrade(self.0)?)) + } + + /// Creates a new 'Lite' independently owned handle to the underlying directory. + pub fn clone_downgrade(&self) -> io::Result { + Ok(Dir(clone_dirfd_downgrade(self.0)?)) + } + } const CURRENT_DIRECTORY: [libc::c_char; 2] = [b'.' as libc::c_char, 0]; @@ -501,6 +513,41 @@ fn clone_dirfd(fd: libc::c_int) -> io::Result { } } +fn clone_dirfd_upgrade(fd: libc::c_int) -> io::Result { + unsafe { + match fd_type(fd)? { + FdType::NormalDir | FdType::LiteDir => libc_ok(libc::openat( + fd, + &CURRENT_DIRECTORY as *const libc::c_char, + BASE_OPEN_FLAGS, + )), + _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), + } + } +} + +fn clone_dirfd_downgrade(fd: libc::c_int) -> io::Result { + unsafe { + match fd_type(fd)? { + #[cfg(target_os = "linux")] + FdType::NormalDir => libc_ok(libc::openat( + fd, + &CURRENT_DIRECTORY as *const libc::c_char, + libc::O_PATH | BASE_OPEN_FLAGS, + )), + #[cfg(not(target_os = "linux"))] + FdType::NormalDir => libc_ok(libc::openat( + fd, + &CURRENT_DIRECTORY as *const libc::c_char, + BASE_OPEN_FLAGS, + )), + #[cfg(target_os = "linux")] + FdType::LiteDir => libc_ok(libc::dup(fd)), + _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), + } + } +} + enum FdType { NormalDir, LiteDir, From abef3ecc22303b720771acb115fd0cc49600cc17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Mon, 19 Apr 2021 08:32:45 +0200 Subject: [PATCH 12/28] Store inode in Entry, add accessor the d_ino field is mandatory in POSIX and clients (me) really need it when passing data around. Keeping it in the Entry saves an extra stat()/metadata query. --- src/lib.rs | 1 + src/list.rs | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index a1a2feb..4bd3e3a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -73,6 +73,7 @@ pub struct Dir(RawFd); pub struct Entry { name: CString, file_type: Option, + ino: libc::ino_t, } #[cfg(test)] diff --git a/src/list.rs b/src/list.rs index 5b4d3cd..3577fb2 100644 --- a/src/list.rs +++ b/src/list.rs @@ -37,6 +37,10 @@ impl Entry { pub fn simple_type(&self) -> Option { self.file_type } + /// Returns the inode number of this entry + pub fn inode(&self) -> libc::ino_t { + self.ino + } } #[cfg(any(target_os="linux", target_os="fuchsia"))] @@ -136,6 +140,7 @@ impl Iterator for DirIter { libc::DT_LNK => Some(SimpleType::Symlink), _ => Some(SimpleType::Other), }, + ino: e.d_ino, })); } } From ffe5d3fbff770e463123b48346807611ec7170a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 21 Apr 2021 20:00:36 +0200 Subject: [PATCH 13/28] use types from 'struct stat' instead 'struct statx' --- src/metadata.rs | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/metadata.rs b/src/metadata.rs index 275cdb4..7b5efb5 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -47,12 +47,12 @@ impl Metadata { self.stat.st_size as u64 } /// Return low level file type, if available - pub fn file_type(&self) -> Option { - Some(self.stat.st_mode as u32 & libc::S_IFMT as u32) + pub fn file_type(&self) -> Option { + Some(self.stat.st_mode & libc::S_IFMT) } /// Return device node, if available - pub fn ino(&self) -> Option { - Some(self.stat.st_ino as u64) + pub fn ino(&self) -> Option { + Some(self.stat.st_ino) } /// Return device node major of the file, if available pub fn dev_major(&self) -> Option { @@ -77,32 +77,32 @@ impl Metadata { } } /// Return preferered I/O Blocksize, if available - pub fn blksize(&self) -> Option { - Some(self.stat.st_blksize as u32) + pub fn blksize(&self) -> Option { + Some(self.stat.st_blksize) } /// Return the number of 512 bytes blocks, if available - pub fn blocks(&self) -> Option { - Some(self.stat.st_blocks as u64) + pub fn blocks(&self) -> Option { + Some(self.stat.st_blocks) } /// Returns file size (same as len() but Option), if available - pub fn size(&self) -> Option { - Some(self.stat.st_size as u64) + pub fn size(&self) -> Option { + Some(self.stat.st_size) } /// Returns number of hard-links, if available - pub fn nlink(&self) -> Option { - Some(self.stat.st_nlink as u32) + pub fn nlink(&self) -> Option { + Some(self.stat.st_nlink) } /// Returns user id, if available - pub fn uid(&self) -> Option { - Some(self.stat.st_uid as u32) + pub fn uid(&self) -> Option { + Some(self.stat.st_uid) } /// Returns group id, if available - pub fn gid(&self) -> Option { - Some(self.stat.st_gid as u32) + pub fn gid(&self) -> Option { + Some(self.stat.st_gid) } /// Returns mode bits, if available - pub fn file_mode(&self) -> Option { - Some(self.stat.st_mode as u32 & 0o7777) + pub fn file_mode(&self) -> Option { + Some(self.stat.st_mode & 0o7777) } /// Returns last access time, if available pub fn atime(&self) -> Option { From 6b4d359a8a56209378e4bc5b0288a4d5b583979a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Thu, 22 Apr 2021 11:31:43 +0200 Subject: [PATCH 14/28] start abstracting OS capabilities into features Instead compiling conditionally on the target_os this defines features for every aspect that can be different. Tested only on linux so far. Eventually the presence/absence of these features should be determined by a build.rs script. Meanwhile there are OS specific lists (only linux so far, please contribute more) which can be activated when building with '--feature osname'. --- Cargo.toml | 17 +++++++++ examples/exchange.rs | 4 +- src/dir.rs | 89 ++++++++++++++++++++++++-------------------- tests/tmpfile.rs | 2 +- 4 files changed, 70 insertions(+), 42 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index a5b5b3f..8d9fac2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,23 @@ version = "0.1.21" authors = ["paul@colomiets.name"] edition = "2018" +[features] +default = [] +#default = ["linux"] +linux = ["o_path", "o_directory", "o_tmpfile", "statx", "proc_self_fd", "link_file_at", "rename_exchange", "renameat_flags"] +#freebsd = ["o_directory", "o_search"] +#NOTE(cehteh): does freebsd have /proc/self/fd like linux? +#NOTE(cehteh): complete this for other OS'es until there is some build.rs autodetection +o_path = [] +o_directory = [] +o_tmpfile = [] +o_search = [] +link_file_at = [] +proc_self_fd = [] +renameat_flags = [] +rename_exchange = [] +statx = [] + [dependencies] libc = "0.2.34" diff --git a/examples/exchange.rs b/examples/exchange.rs index 21fdf0a..fdd651d 100644 --- a/examples/exchange.rs +++ b/examples/exchange.rs @@ -32,8 +32,10 @@ fn main() { } let parent = path1.parent().expect("path must have parent directory"); let dir = Dir::open(parent).expect("can open directory"); + #[cfg(feature = "rename_exchange")] dir.local_exchange( path1.file_name().expect("path1 must have filename"), path2.file_name().expect("path2 must have filename"), - ).expect("can rename"); + ) + .expect("can rename"); } diff --git a/src/dir.rs b/src/dir.rs index 66bf82d..e5483d4 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -13,13 +13,25 @@ use crate::metadata::{self, Metadata}; use crate::{Dir, AsPath}; // NOTE(cehteh): removed O_PATH since it is linux only and highly unportable (semantics can't be emulated) -// but see open_path() below. -#[cfg(any(target_os = "linux", target_os = "freebsd"))] -const BASE_OPEN_FLAGS: libc::c_int = libc::O_DIRECTORY | libc::O_CLOEXEC; -// NOTE(cehteh): O_DIRECTORY is defined in posix, what is the reason for not using it? -// TODO(cehteh): on systems that do not support O_DIRECTORY a runtime stat-check is required -#[cfg(not(any(target_os = "linux", target_os = "freebsd")))] -const BASE_OPEN_FLAGS: libc::c_int = libc::O_CLOEXEC; +// but see open_lite() below. + + +#[cfg(feature = "o_directory")] +const O_DIRECTORY_FLAG: libc::c_int = libc::O_DIRECTORY; +#[cfg(not(feature = "o_directory"))] +const O_DIRECTORY_FLAG: libc::c_int = 0; + +#[cfg(feature = "o_path")] +const O_PATH_FLAG: libc::c_int = libc::O_PATH; +#[cfg(not(feature = "o_path"))] +const O_PATH_FLAG: libc::c_int = 0; + +#[cfg(feature = "o_search")] +const O_SEARCH_FLAG: libc::c_int = libc::O_SEARCH; +#[cfg(not(feature = "o_search"))] +const O_SEARCH_FLAG: libc::c_int = 0; + +const BASE_OPEN_FLAGS: libc::c_int = O_DIRECTORY_FLAG | libc::O_CLOEXEC; impl Dir { /// Creates a directory descriptor that resolves paths relative to current @@ -47,14 +59,8 @@ impl Dir { /// - One can query metadata of this directory /// Using this descriptor for iterating over the content is unspecified. /// Uses O_PATH on Linux - #[cfg(any(target_os = "linux"))] - pub fn open_lite(path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref(), libc::O_PATH | BASE_OPEN_FLAGS) - } - - #[cfg(not(target_os = "linux"))] pub fn open_lite(path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS) + Dir::_open(to_cstr(path)?.as_ref(), O_PATH_FLAG | BASE_OPEN_FLAGS) } fn _open(path: &CStr, flags: libc::c_int) -> io::Result { @@ -62,17 +68,21 @@ impl Dir { Ok(Dir(fd)) } + //PLANNED(cehteh): add fn is_dir(&self) using fd_type() below, for the cases one *must* + // check that the opened Dir is really a directory. This will be more lightweight than a + // stat() when O_DIRECTORY is supported. + /// List subdirectory of this dir /// /// You can list directory itself with `list_self`. - // TODO(cehteh): may be deprecated in favor of list() pub fn list_dir(&self, path: P) -> io::Result { + //TODO(cehteh): with(O_SEARCH_FLAG) self.sub_dir(path)?.list() } /// List this dir - // TODO(cehteh): may be deprecated in favor of list() pub fn list_self(&self) -> io::Result { + //TODO(cehteh): with(O_SEARCH_FLAG) self.clone_upgrade()?.list() } @@ -106,14 +116,8 @@ impl Dir { /// [`read_link`] to resolve the real path first. /// /// [`read_link`]: #method.read_link - #[cfg(any(target_os = "linux"))] pub fn sub_dir_lite(&self, path: P) -> io::Result { - self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW | libc::O_PATH) - } - - #[cfg(not(target_os = "linux"))] - pub fn sub_dir_lite(&self, path: P) -> io::Result { - self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW) + self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW | O_PATH_FLAG) } fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { @@ -223,7 +227,7 @@ impl Dir { /// Currently, we recommend to fallback on any error if this operation /// can't be accomplished rather than relying on specific error codes, /// because semantics of errors are very ugly. - #[cfg(target_os="linux")] + #[cfg(feature = "o_tmpfile")] pub fn new_unnamed_file(&self, mode: libc::mode_t) -> io::Result { @@ -252,10 +256,13 @@ impl Dir { /// Currently, we recommend to fallback on any error if this operation /// can't be accomplished rather than relying on specific error codes, /// because semantics of errors are very ugly. - #[cfg(not(target_os="linux"))] + #[cfg(not(feature = "o_tmpfile"))] pub fn new_unnamed_file(&self, _mode: libc::mode_t) -> io::Result { + //NOTE(cehteh): tempfiles can be obtained by creating a random named file and + // immediately unlink them. This is portable so far, still link_file_at() wont work on + // those. Err(io::Error::new(io::ErrorKind::Other, "creating unnamed tmpfiles is only supported on linux")) } @@ -271,7 +278,7 @@ impl Dir { /// fails. But in obscure scenarios where `/proc` is not mounted this /// method may fail even on linux. So your code should be able to fallback /// to a named file if this method fails too. - #[cfg(target_os="linux")] + #[cfg(feature = "link_file_at")] pub fn link_file_at(&self, file: &F, path: P) -> io::Result<()> { @@ -292,7 +299,10 @@ impl Dir { /// fails. But in obscure scenarios where `/proc` is not mounted this /// method may fail even on linux. So your code should be able to fallback /// to a named file if this method fails too. - #[cfg(not(target_os="linux"))] + //NOTE(cehteh): would it make sense to remove this function (for non linux), this will + // generate a compile time error rather than a runtime error, which most likely is + // favorable since the semantic cant easily emulated. + #[cfg(not(feature = "link_file_at"))] pub fn link_file_at(&self, _file: F, _path: P) -> io::Result<()> { @@ -389,7 +399,7 @@ impl Dir { /// Similar to `local_rename` but atomically swaps both paths /// /// Only supported on Linux. - #[cfg(target_os="linux")] + #[cfg(feature = "rename_exchange")] pub fn local_exchange(&self, old: P, new: R) -> io::Result<()> { @@ -428,7 +438,7 @@ impl Dir { /// /// This uses symlinks in `/proc/self`, they sometimes may not be /// available so use with care. - // FIXME(cehteh): proc stuff isn't portable + #[cfg(feature = "proc_self_fd")] pub fn recover_path(&self) -> io::Result { let fd = self.0; if fd != libc::AT_FDCWD { @@ -506,7 +516,7 @@ fn clone_dirfd(fd: libc::c_int) -> io::Result { &CURRENT_DIRECTORY as *const libc::c_char, BASE_OPEN_FLAGS, )), - #[cfg(target_os = "linux")] + #[cfg(feature = "o_path")] FdType::LiteDir => libc_ok(libc::dup(fd)), _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } @@ -529,19 +539,19 @@ fn clone_dirfd_upgrade(fd: libc::c_int) -> io::Result { fn clone_dirfd_downgrade(fd: libc::c_int) -> io::Result { unsafe { match fd_type(fd)? { - #[cfg(target_os = "linux")] + #[cfg(feature = "o_path")] FdType::NormalDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, libc::O_PATH | BASE_OPEN_FLAGS, )), - #[cfg(not(target_os = "linux"))] + #[cfg(not(feature = "o_path"))] FdType::NormalDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, BASE_OPEN_FLAGS, )), - #[cfg(target_os = "linux")] + #[cfg(feature = "o_path")] FdType::LiteDir => libc_ok(libc::dup(fd)), _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } @@ -556,7 +566,7 @@ enum FdType { // OSes with O_DIRECTORY can use fcntl() // Linux hash O_PATH -#[cfg(target_os = "linux")] +#[cfg(all(feature = "o_path", feature = "o_directory"))] fn fd_type(fd: libc::c_int) -> io::Result { let flags = unsafe { libc_ok(libc::fcntl(fd, libc::F_GETFL))? }; if flags & libc::O_DIRECTORY != 0 { @@ -570,7 +580,7 @@ fn fd_type(fd: libc::c_int) -> io::Result { } } -#[cfg(target_os = "freebsd")] +#[cfg(all(not(feature = "o_path"), feature = "o_directory"))] fn fd_type(fd: libc::c_int) -> io::Result { let flags = unsafe { libc_ok(libc::fcntl(fd, libc::F_GETFL))? }; if flags & libc::O_DIRECTORY != 0 { @@ -581,7 +591,7 @@ fn fd_type(fd: libc::c_int) -> io::Result { } // OSes without O_DIRECTORY use stat() -#[cfg(not(any(target_os = "linux", target_os = "freebsd")))] +#[cfg(not(feature = "o_directory"))] fn fd_type(fd: libc::c_int) -> io::Result { unsafe { let mut stat = mem::zeroed(); // TODO(cehteh): uninit @@ -656,7 +666,7 @@ fn _hardlink( /// fallback to copying if needed. /// /// Only supported on Linux. -#[cfg(target_os="linux")] +#[cfg(feature = "renameat_flags")] pub fn rename_flags(old_dir: &Dir, old: P, new_dir: &Dir, new: R, flags: libc::c_int) -> io::Result<()> @@ -667,7 +677,7 @@ pub fn rename_flags(old_dir: &Dir, old: P, new_dir: &Dir, new: R, flags) } -#[cfg(target_os="linux")] +#[cfg(feature = "renameat_flags")] fn _rename_flags(old_dir: &Dir, old: &CStr, new_dir: &Dir, new: &CStr, flags: libc::c_int) -> io::Result<()> @@ -742,8 +752,7 @@ mod test { } #[test] - #[cfg_attr(any(target_os = "freebsd", target_os = "linux"), should_panic(expected = "Not a directory"))] - // NOTE(cehteh): should fail in all cases! see O_DIRECTORY at the top + #[cfg_attr(feature = "o_directory", should_panic(expected = "Not a directory"))] fn test_open_file() { Dir::open("src/lib.rs").unwrap(); } diff --git a/tests/tmpfile.rs b/tests/tmpfile.rs index 4fa0f0d..6dd6e0a 100644 --- a/tests/tmpfile.rs +++ b/tests/tmpfile.rs @@ -6,7 +6,7 @@ use std::os::unix::fs::PermissionsExt; use openat::Dir; #[test] -#[cfg(target_os="linux")] +#[cfg(feature = "link_file_at")] fn unnamed_tmp_file_link() -> Result<(), io::Error> { let tmp = tempfile::tempdir()?; let dir = Dir::open(tmp.path())?; From 076ba5c6b372a4ce873fd671a6963510ddb2859f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Tue, 27 Apr 2021 18:38:24 +0200 Subject: [PATCH 15/28] WIP: Add new 'conf_test' for build time configuration, o_path check --- Cargo.toml | 11 ++++++----- build.rs | 5 +++++ conf_tests/o_path.rs | 8 ++++++++ 3 files changed, 19 insertions(+), 5 deletions(-) create mode 100644 build.rs create mode 100644 conf_tests/o_path.rs diff --git a/Cargo.toml b/Cargo.toml index 8d9fac2..872af05 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,17 +16,14 @@ edition = "2018" [features] default = [] -#default = ["linux"] linux = ["o_path", "o_directory", "o_tmpfile", "statx", "proc_self_fd", "link_file_at", "rename_exchange", "renameat_flags"] -#freebsd = ["o_directory", "o_search"] -#NOTE(cehteh): does freebsd have /proc/self/fd like linux? -#NOTE(cehteh): complete this for other OS'es until there is some build.rs autodetection +#NOTE(cehteh): eventually provide some baseline configs for other OS'es (for cross compilation) o_path = [] o_directory = [] o_tmpfile = [] o_search = [] -link_file_at = [] proc_self_fd = [] +link_file_at = [] renameat_flags = [] rename_exchange = [] statx = [] @@ -34,6 +31,10 @@ statx = [] [dependencies] libc = "0.2.34" +[build-dependencies] +libc = "0.2.34" +conf_test = "0.1" + [dev-dependencies] argparse = "0.2.1" tempfile = "3.0.3" diff --git a/build.rs b/build.rs new file mode 100644 index 0000000..851c014 --- /dev/null +++ b/build.rs @@ -0,0 +1,5 @@ +use conf_test::ConfTest; + +fn main() { + ConfTest::run(); +} diff --git a/conf_tests/o_path.rs b/conf_tests/o_path.rs new file mode 100644 index 0000000..e454460 --- /dev/null +++ b/conf_tests/o_path.rs @@ -0,0 +1,8 @@ +extern crate libc; + +fn main () { + unsafe { + let conf_tests = std::ffi::CString::new("conf_tests").unwrap(); + libc::open(conf_tests.as_ptr(), libc::O_PATH); + } +} From 69acef0951202b3acfde1b38c7c22eb9d1f1e0f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Tue, 27 Apr 2021 19:03:37 +0200 Subject: [PATCH 16/28] add conf_tests for o_directory|search|tmpfile --- conf_tests/o_directory.rs | 8 ++++++++ conf_tests/o_search.rs | 8 ++++++++ conf_tests/o_tmpfile.rs | 10 ++++++++++ 3 files changed, 26 insertions(+) create mode 100644 conf_tests/o_directory.rs create mode 100644 conf_tests/o_search.rs create mode 100644 conf_tests/o_tmpfile.rs diff --git a/conf_tests/o_directory.rs b/conf_tests/o_directory.rs new file mode 100644 index 0000000..07c0f61 --- /dev/null +++ b/conf_tests/o_directory.rs @@ -0,0 +1,8 @@ +extern crate libc; + +fn main() { + unsafe { + let conf_tests = std::ffi::CString::new("conf_tests").unwrap(); + libc::open(conf_tests.as_ptr(), libc::O_DIRECTORY); + } +} diff --git a/conf_tests/o_search.rs b/conf_tests/o_search.rs new file mode 100644 index 0000000..60a3250 --- /dev/null +++ b/conf_tests/o_search.rs @@ -0,0 +1,8 @@ +extern crate libc; + +fn main() { + unsafe { + let conf_tests = std::ffi::CString::new("conf_tests").unwrap(); + libc::open(conf_tests.as_ptr(), libc::O_SEARCH); + } +} diff --git a/conf_tests/o_tmpfile.rs b/conf_tests/o_tmpfile.rs new file mode 100644 index 0000000..8c0ad16 --- /dev/null +++ b/conf_tests/o_tmpfile.rs @@ -0,0 +1,10 @@ +extern crate libc; + +fn main () { + unsafe { + let conf_tests = std::ffi::CString::new("conf_tests").unwrap(); + libc::open(conf_tests.as_ptr(), + libc::O_TMPFILE | libc::O_RDWR, + libc::S_IRUSR | libc::S_IWUSR); + } +} From 34905a56ed78c09e192327bfe21aaf579b8d6ec6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Tue, 27 Apr 2021 19:19:56 +0200 Subject: [PATCH 17/28] add the remaining tests (for now), remove a misleading note --- conf_tests/link_file_at.rs | 13 +++++++++++++ conf_tests/proc_self_fd.rs | 12 ++++++++++++ conf_tests/rename_exchange.rs | 5 +++++ conf_tests/renameat_flags.rs | 5 +++++ src/dir.rs | 3 --- 5 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 conf_tests/link_file_at.rs create mode 100644 conf_tests/proc_self_fd.rs create mode 100644 conf_tests/rename_exchange.rs create mode 100644 conf_tests/renameat_flags.rs diff --git a/conf_tests/link_file_at.rs b/conf_tests/link_file_at.rs new file mode 100644 index 0000000..0dd292d --- /dev/null +++ b/conf_tests/link_file_at.rs @@ -0,0 +1,13 @@ +extern crate libc; + +fn main() { + //NOTE(cehteh): same as the proc_self_fd test, maybe we need something smarter in future + unsafe { + let conf_tests = std::ffi::CString::new("/proc/self/fd/0").unwrap(); + if libc::open(conf_tests.as_ptr(), libc::O_RDONLY) != -1 { + std::process::exit(0); + } else { + std::process::exit(1); + } + } +} diff --git a/conf_tests/proc_self_fd.rs b/conf_tests/proc_self_fd.rs new file mode 100644 index 0000000..7983b5c --- /dev/null +++ b/conf_tests/proc_self_fd.rs @@ -0,0 +1,12 @@ +extern crate libc; + +fn main() { + unsafe { + let conf_tests = std::ffi::CString::new("/proc/self/fd/0").unwrap(); + if libc::open(conf_tests.as_ptr(), libc::O_RDONLY) != -1 { + std::process::exit(0); + } else { + std::process::exit(1); + } + } +} diff --git a/conf_tests/rename_exchange.rs b/conf_tests/rename_exchange.rs new file mode 100644 index 0000000..76f69f9 --- /dev/null +++ b/conf_tests/rename_exchange.rs @@ -0,0 +1,5 @@ +extern crate libc; + +fn main() { + let does_rename_exchange_exist = libc::RENAME_EXCHANGE; +} diff --git a/conf_tests/renameat_flags.rs b/conf_tests/renameat_flags.rs new file mode 100644 index 0000000..392ba1c --- /dev/null +++ b/conf_tests/renameat_flags.rs @@ -0,0 +1,5 @@ +extern crate libc; + +fn main() { + let does_renameat2_exist = libc::SYS_renameat2; +} diff --git a/src/dir.rs b/src/dir.rs index e5483d4..dd81443 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -260,9 +260,6 @@ impl Dir { pub fn new_unnamed_file(&self, _mode: libc::mode_t) -> io::Result { - //NOTE(cehteh): tempfiles can be obtained by creating a random named file and - // immediately unlink them. This is portable so far, still link_file_at() wont work on - // those. Err(io::Error::new(io::ErrorKind::Other, "creating unnamed tmpfiles is only supported on linux")) } From c8fed25fdff6720c3b0bf75deddea00c868687ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Tue, 27 Apr 2021 20:07:12 +0200 Subject: [PATCH 18/28] FIX: The unnamed_tmp_file_link test needs o_tmpfile and link_file_at --- tests/tmpfile.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tmpfile.rs b/tests/tmpfile.rs index 6dd6e0a..73e8c36 100644 --- a/tests/tmpfile.rs +++ b/tests/tmpfile.rs @@ -6,7 +6,7 @@ use std::os::unix::fs::PermissionsExt; use openat::Dir; #[test] -#[cfg(feature = "link_file_at")] +#[cfg(all(feature = "o_tmpfile", feature = "link_file_at"))] fn unnamed_tmp_file_link() -> Result<(), io::Error> { let tmp = tempfile::tempdir()?; let dir = Dir::open(tmp.path())?; From 7cf25838381354631f3f130947282bff74b469ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Tue, 27 Apr 2021 21:10:00 +0200 Subject: [PATCH 19/28] WIP/FIX: test osx fcntl() returning O_DIRECTORY --- Cargo.toml | 1 + conf_tests/fcntl_o_directory.rs | 16 ++++++++++++++++ src/dir.rs | 12 ++++++------ 3 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 conf_tests/fcntl_o_directory.rs diff --git a/Cargo.toml b/Cargo.toml index 872af05..4d232f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,6 +22,7 @@ o_path = [] o_directory = [] o_tmpfile = [] o_search = [] +fcntl_o_directory = [] proc_self_fd = [] link_file_at = [] renameat_flags = [] diff --git a/conf_tests/fcntl_o_directory.rs b/conf_tests/fcntl_o_directory.rs new file mode 100644 index 0000000..5a135f0 --- /dev/null +++ b/conf_tests/fcntl_o_directory.rs @@ -0,0 +1,16 @@ +extern crate libc; + +fn main() { + unsafe { + let conf_tests = std::ffi::CString::new("conf_tests").unwrap(); + let fd = libc::open(conf_tests.as_ptr(), libc::O_DIRECTORY | libc::O_RDONLY); + + let flags = libc::fcntl(fd, libc::F_GETFL); + + if flags != -1 && flags & libc::O_DIRECTORY != 0 { + std::process::exit(0); + } else { + std::process::exit(1); + } + } +} diff --git a/src/dir.rs b/src/dir.rs index dd81443..36233c5 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -481,7 +481,7 @@ impl Dir { pub unsafe fn from_raw_fd_checked(fd: RawFd) -> io::Result { match fd_type(fd)? { FdType::NormalDir | FdType::LiteDir => Ok(Dir(fd)), - _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), + FdType::Other => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } } @@ -561,9 +561,9 @@ enum FdType { Other, } -// OSes with O_DIRECTORY can use fcntl() +// OSes with fcntl() that returns O_DIRECTORY // Linux hash O_PATH -#[cfg(all(feature = "o_path", feature = "o_directory"))] +#[cfg(all(feature = "o_path", feature = "fcntl_o_directory"))] fn fd_type(fd: libc::c_int) -> io::Result { let flags = unsafe { libc_ok(libc::fcntl(fd, libc::F_GETFL))? }; if flags & libc::O_DIRECTORY != 0 { @@ -577,7 +577,7 @@ fn fd_type(fd: libc::c_int) -> io::Result { } } -#[cfg(all(not(feature = "o_path"), feature = "o_directory"))] +#[cfg(all(not(feature = "o_path"), feature = "fcntl_o_directory"))] fn fd_type(fd: libc::c_int) -> io::Result { let flags = unsafe { libc_ok(libc::fcntl(fd, libc::F_GETFL))? }; if flags & libc::O_DIRECTORY != 0 { @@ -587,8 +587,8 @@ fn fd_type(fd: libc::c_int) -> io::Result { } } -// OSes without O_DIRECTORY use stat() -#[cfg(not(feature = "o_directory"))] +// OSes where fcntl won't return O_DIRECTORY use stat() +#[cfg(not(feature = "fcntl_o_directory"))] fn fd_type(fd: libc::c_int) -> io::Result { unsafe { let mut stat = mem::zeroed(); // TODO(cehteh): uninit From c43ab675b8154922f34223bb355a9744bcc16f0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Wed, 28 Apr 2021 18:58:28 +0200 Subject: [PATCH 20/28] notice about O_SEARCH when cloning --- src/dir.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/dir.rs b/src/dir.rs index 36233c5..cfd5463 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -505,6 +505,7 @@ impl Dir { const CURRENT_DIRECTORY: [libc::c_char; 2] = [b'.' as libc::c_char, 0]; +//TODO(cehteh): eventually the clone calls should replicate O_SEARCH, maybe other file flags fn clone_dirfd(fd: libc::c_int) -> io::Result { unsafe { match fd_type(fd)? { From 4b90fcb26db49244910a9b5f69386037994b8e5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Thu, 29 Apr 2021 00:07:07 +0200 Subject: [PATCH 21/28] add 'is_dir()' check, remove the 'test_open_file()' test This should resolve #27. It does not fix it by adding a expensive stat() check on opening, but by giving the opportunity to explicitly check a Dir handle. When really required a programmer can enforce consistency between platforms by using 'is_dir()'. Failing to do so won't cause any harm because improper handles (on platforms without O_DIRECTORY) would report errors at later time. This also removes the test_open_file() test from the testsuite, as it depended on presence of O_DIRECTORY which makes he testsuite non deterministic testing only where its oblivious that the OS wouldn't open files as directories. --- src/dir.rs | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/dir.rs b/src/dir.rs index cfd5463..3c84399 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -68,9 +68,22 @@ impl Dir { Ok(Dir(fd)) } - //PLANNED(cehteh): add fn is_dir(&self) using fd_type() below, for the cases one *must* - // check that the opened Dir is really a directory. This will be more lightweight than a - // stat() when O_DIRECTORY is supported. + /// Checks if the fd associated with the Dir object is really a directory. + /// There are subtle differences in how directories can be opened and what properties the + /// resulting file handles have. On some platforms it is possible that + /// Dir::open("somefile") succeeds. This will usually raise errors later when one tries to + /// do Directory operations on this. While checking if such an handle comes with cost of a + /// potential expensive 'stat()' operation. This library makes the assumption that in the + /// 'usual' case Dir objects are only created on directories and operations on Dir handles + /// handle errors properly. Still in some cases one may check a freshly created handle + /// explicitly. Thats what 'is_dir()' is for. Returns 'true' when the underlying handles + /// represents a directory and false otherwise. + pub fn is_dir(&self) -> bool { + match fd_type(self.0).unwrap_or(FdType::Other){ + FdType::NormalDir | FdType::LiteDir => true, + FdType::Other => false, + } + } /// List subdirectory of this dir /// @@ -749,12 +762,6 @@ mod test { assert!(Dir::open("src").is_ok()); } - #[test] - #[cfg_attr(feature = "o_directory", should_panic(expected = "Not a directory"))] - fn test_open_file() { - Dir::open("src/lib.rs").unwrap(); - } - #[test] fn test_read_file() { let dir = Dir::open("src").unwrap(); From ec29aca956ec6cf5d0ab1bda0cc3c18ac1e1b1af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Thu, 29 Apr 2021 19:06:03 +0200 Subject: [PATCH 22/28] Add flags builder for 'Dir' objects Allows to build/pass custom flags when opening a 'Dir', see tests/flagsbuilder.rs Should fix/conclude #29 Notice: this removes the 'const BASE_OPEN_FLAGS' as it makes flags handling a bit more elaborate: 1. DirFlags is created with 'O_CLOEXEC' (I considered O_NOFOLLOW but turned that down) 2. The user may modify the flags to his liking 3. The final open() calls will set the required flags (O_DIRECTORY, O_PATH) All functions that used BASE_OPEN_FLAGS fixed up, but semantics stay the same. --- src/builder.rs | 56 +++++++++++++++++++++++++++++++++++++++++++ src/dir.rs | 42 +++++++++++++++++++------------- src/lib.rs | 2 ++ tests/flagsbuilder.rs | 12 ++++++++++ 4 files changed, 95 insertions(+), 17 deletions(-) create mode 100644 src/builder.rs create mode 100644 tests/flagsbuilder.rs diff --git a/src/builder.rs b/src/builder.rs new file mode 100644 index 0000000..633392c --- /dev/null +++ b/src/builder.rs @@ -0,0 +1,56 @@ +use std::io; + +use crate::dir::{to_cstr, O_PATH_FLAG}; +use crate::{AsPath, Dir}; + +/// 'Dir::new()' creates a new DirFlags object with default (O_CLOEXEC) flags. One can then +/// freely add/remove flags to the set. The final open calls will add O_DIRECTORY and O_PATH +/// as applicable/supported but not verify or remove any defined flags. This allows passing +/// flags the 'openat' implementation is not even aware about. Thus the open call may fail +/// with some error when one constructed an invalid flag set. +#[derive(Copy, Clone)] +pub struct DirFlags { + flags: libc::c_int, +} + +impl DirFlags { + #[inline] + pub(crate) fn new(flags: libc::c_int) -> DirFlags { + DirFlags { flags: flags } + } + + /// Sets the given flags + #[inline] + pub fn with(self, flags: libc::c_int) -> DirFlags { + DirFlags { + flags: self.flags | flags, + } + } + + /// Clears the given flags + #[inline] + pub fn without(self, flags: libc::c_int) -> DirFlags { + DirFlags { + flags: self.flags & !flags, + } + } + + /// Queries current flags + #[inline] + pub fn get_flags(&self) -> libc::c_int { + self.flags + } + + /// Open a directory descriptor at specified path + #[inline] + pub fn open(&self, path: P) -> io::Result { + Dir::_open(to_cstr(path)?.as_ref(), self.flags) + } + + /// Open a lite directory descriptor at specified path + #[inline] + pub fn open_lite(&self, path: P) -> io::Result { + Dir::_open(to_cstr(path)?.as_ref(), O_PATH_FLAG | self.flags) + } +} + diff --git a/src/dir.rs b/src/dir.rs index 3c84399..c12d0b7 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -10,7 +10,7 @@ use libc; use crate::list::{open_dirfd, DirIter}; use crate::metadata::{self, Metadata}; -use crate::{Dir, AsPath}; +use crate::{Dir, AsPath, DirFlags}; // NOTE(cehteh): removed O_PATH since it is linux only and highly unportable (semantics can't be emulated) // but see open_lite() below. @@ -22,17 +22,15 @@ const O_DIRECTORY_FLAG: libc::c_int = libc::O_DIRECTORY; const O_DIRECTORY_FLAG: libc::c_int = 0; #[cfg(feature = "o_path")] -const O_PATH_FLAG: libc::c_int = libc::O_PATH; +pub(crate) const O_PATH_FLAG: libc::c_int = libc::O_PATH; #[cfg(not(feature = "o_path"))] -const O_PATH_FLAG: libc::c_int = 0; +pub(crate) const O_PATH_FLAG: libc::c_int = 0; #[cfg(feature = "o_search")] const O_SEARCH_FLAG: libc::c_int = libc::O_SEARCH; #[cfg(not(feature = "o_search"))] const O_SEARCH_FLAG: libc::c_int = 0; -const BASE_OPEN_FLAGS: libc::c_int = O_DIRECTORY_FLAG | libc::O_CLOEXEC; - impl Dir { /// Creates a directory descriptor that resolves paths relative to current /// working directory (AT_FDCWD) @@ -47,10 +45,19 @@ impl Dir { Dir(libc::AT_FDCWD) } + /// Create a flags builder for Dir objects. Initial flags default to `O_CLOEXEC'. More + /// flags can be set added by 'with()' and existing/default flags can be removed by + /// 'without()'. The flags builder can the be used to 'open()' or 'open_lite()' to create + /// a Dir handle. + #[inline] + pub fn new() -> DirFlags { + DirFlags::new(libc::O_CLOEXEC) + } + /// Open a directory descriptor at specified path // TODO(tailhook) maybe accept only absolute paths? pub fn open(path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS) + Dir::_open(to_cstr(path)?.as_ref(), libc::O_CLOEXEC) } /// Open a 'lite' directory descriptor at specified path @@ -60,11 +67,11 @@ impl Dir { /// Using this descriptor for iterating over the content is unspecified. /// Uses O_PATH on Linux pub fn open_lite(path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref(), O_PATH_FLAG | BASE_OPEN_FLAGS) + Dir::_open(to_cstr(path)?.as_ref(), O_PATH_FLAG | libc::O_CLOEXEC) } - fn _open(path: &CStr, flags: libc::c_int) -> io::Result { - let fd = unsafe { libc_ok(libc::open(path.as_ptr(), flags))? }; + pub(crate) fn _open(path: &CStr, flags: libc::c_int) -> io::Result { + let fd = unsafe { libc_ok(libc::open(path.as_ptr(), O_DIRECTORY_FLAG | flags))? }; Ok(Dir(fd)) } @@ -114,7 +121,7 @@ impl Dir { /// /// [`read_link`]: #method.read_link pub fn sub_dir(&self, path: P) -> io::Result { - self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW) + self._sub_dir(to_cstr(path)?.as_ref(), libc::O_CLOEXEC | libc::O_NOFOLLOW) } /// Open subdirectory with a 'lite' descriptor at specified path @@ -130,11 +137,11 @@ impl Dir { /// /// [`read_link`]: #method.read_link pub fn sub_dir_lite(&self, path: P) -> io::Result { - self._sub_dir(to_cstr(path)?.as_ref(), BASE_OPEN_FLAGS | libc::O_NOFOLLOW | O_PATH_FLAG) + self._sub_dir(to_cstr(path)?.as_ref(), libc::O_CLOEXEC | libc::O_NOFOLLOW | O_PATH_FLAG) } fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { - Ok(Dir(unsafe { libc_ok(libc::openat(self.0, path.as_ptr(), flags))? })) + Ok(Dir(unsafe { libc_ok(libc::openat(self.0, path.as_ptr(), flags | O_DIRECTORY_FLAG))? })) } /// Read link in this directory @@ -525,7 +532,7 @@ fn clone_dirfd(fd: libc::c_int) -> io::Result { FdType::NormalDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - BASE_OPEN_FLAGS, + O_DIRECTORY_FLAG | libc::O_CLOEXEC, )), #[cfg(feature = "o_path")] FdType::LiteDir => libc_ok(libc::dup(fd)), @@ -540,7 +547,7 @@ fn clone_dirfd_upgrade(fd: libc::c_int) -> io::Result { FdType::NormalDir | FdType::LiteDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - BASE_OPEN_FLAGS, + O_DIRECTORY_FLAG | libc::O_CLOEXEC, )), _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } @@ -554,13 +561,13 @@ fn clone_dirfd_downgrade(fd: libc::c_int) -> io::Result { FdType::NormalDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - libc::O_PATH | BASE_OPEN_FLAGS, + libc::O_PATH | O_DIRECTORY_FLAG | libc::O_CLOEXEC, )), #[cfg(not(feature = "o_path"))] FdType::NormalDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - BASE_OPEN_FLAGS, + O_DIRECTORY_FLAG | libc::O_CLOEXEC, )), #[cfg(feature = "o_path")] FdType::LiteDir => libc_ok(libc::dup(fd)), @@ -569,6 +576,7 @@ fn clone_dirfd_downgrade(fd: libc::c_int) -> io::Result { } } +//TODO: let fd_type() return the queried flags, clone will need them enum FdType { NormalDir, LiteDir, @@ -742,7 +750,7 @@ impl Drop for Dir { } } -fn to_cstr(path: P) -> io::Result { +pub(crate) fn to_cstr(path: P) -> io::Result { path.to_path() .ok_or_else(|| { io::Error::new(io::ErrorKind::InvalidInput, diff --git a/src/lib.rs b/src/lib.rs index 4bd3e3a..f0b261b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -51,12 +51,14 @@ mod list; mod name; mod filetype; mod metadata; +mod builder; pub use crate::list::DirIter; pub use crate::name::AsPath; pub use crate::dir::{rename, hardlink}; pub use crate::filetype::SimpleType; pub use crate::metadata::Metadata; +pub use crate::builder::{DirFlags}; use std::ffi::CString; use std::os::unix::io::RawFd; diff --git a/tests/flagsbuilder.rs b/tests/flagsbuilder.rs new file mode 100644 index 0000000..c7cafff --- /dev/null +++ b/tests/flagsbuilder.rs @@ -0,0 +1,12 @@ +extern crate openat; +use openat::Dir; + +#[test] +fn dir_flags_builder_basic() { + let dir = Dir::new() + .without(libc::O_CLOEXEC) + .with(libc::O_NOFOLLOW) + .open("src"); + + assert!(dir.is_ok()); +} From 43b5cdf5ae5a45ca0db86951fc38f937f42ec905 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Fri, 30 Apr 2021 15:17:06 +0200 Subject: [PATCH 23/28] Rename the builder ctor from new() to flags() having Dir::new() returning DirFlags and not Dir would be a tpp surprising API --- src/builder.rs | 2 +- src/dir.rs | 2 +- tests/flagsbuilder.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 633392c..b65213b 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -16,7 +16,7 @@ pub struct DirFlags { impl DirFlags { #[inline] pub(crate) fn new(flags: libc::c_int) -> DirFlags { - DirFlags { flags: flags } + DirFlags { flags } } /// Sets the given flags diff --git a/src/dir.rs b/src/dir.rs index c12d0b7..74152d8 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -50,7 +50,7 @@ impl Dir { /// 'without()'. The flags builder can the be used to 'open()' or 'open_lite()' to create /// a Dir handle. #[inline] - pub fn new() -> DirFlags { + pub fn flags() -> DirFlags { DirFlags::new(libc::O_CLOEXEC) } diff --git a/tests/flagsbuilder.rs b/tests/flagsbuilder.rs index c7cafff..e24934f 100644 --- a/tests/flagsbuilder.rs +++ b/tests/flagsbuilder.rs @@ -3,7 +3,7 @@ use openat::Dir; #[test] fn dir_flags_builder_basic() { - let dir = Dir::new() + let dir = Dir::flags() .without(libc::O_CLOEXEC) .with(libc::O_NOFOLLOW) .open("src"); From f57123b8b8a6acf00607e3fb8b2e5f1a05a4a398 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Fri, 30 Apr 2021 16:21:34 +0200 Subject: [PATCH 24/28] Add DirMethodFlags builder With this it becomes possible to construct and pass open flags to the various member functions. Also list_self() and list_dir() will use O_SEARCH when supported. Other than the Dir flags builder, this flags builder for member functions defaults to O_NOFOLLOW (and O_CLOEXEC) to prevent symlink attacks by default. When this is not desired one can still clear O_NOFOLLOW. --- src/builder.rs | 114 +++++++++++++++++++++++++++++++++++++++++- src/dir.rs | 38 +++++++++----- src/lib.rs | 2 +- tests/flagsbuilder.rs | 30 +++++++++++ 4 files changed, 170 insertions(+), 14 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index b65213b..de8c0c8 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -1,6 +1,8 @@ +use std::ffi::CStr; +use std::fs::File; use std::io; -use crate::dir::{to_cstr, O_PATH_FLAG}; +use crate::dir::{clone_dirfd_upgrade, to_cstr, O_PATH_FLAG}; use crate::{AsPath, Dir}; /// 'Dir::new()' creates a new DirFlags object with default (O_CLOEXEC) flags. One can then @@ -54,3 +56,113 @@ impl DirFlags { } } +/// 'Dir::with(&self)'/'Dir::with(&self)' creates a new DirMethodsFlags object with default +/// (O_CLOEXEC|O_NOFOLLOW) flags. One can then freely add/remove flags to the set. +/// Implements proxies for the Dir:: methods that open contained objects. +#[derive(Copy, Clone)] +pub struct DirMethodFlags<'a> { + object: &'a Dir, + flags: libc::c_int, +} + +impl<'a> DirMethodFlags<'a> { + #[inline] + pub(crate) fn new(object: &'a Dir, flags: libc::c_int) -> Self { + Self { object, flags } + } + + /// Sets the given flags + #[inline] + pub fn with(self, flags: libc::c_int) -> Self { + Self { + object: self.object, + flags: self.flags | flags, + } + } + + /// Clears the given flags + #[inline] + pub fn without(self, flags: libc::c_int) -> Self { + Self { + object: self.object, + flags: self.flags & !flags, + } + } + + /// Open subdirectory + #[inline] + pub fn sub_dir(&self, path: P) -> io::Result { + self.object._sub_dir(to_cstr(path)?.as_ref(), self.flags) + } + + /// Open subdirectory with a 'lite' descriptor at specified path + #[inline] + pub fn sub_dir_lite(&self, path: P) -> io::Result { + self.object + ._sub_dir(to_cstr(path)?.as_ref(), self.flags | O_PATH_FLAG) + } + + /// Open file for reading in this directory + #[inline] + pub fn open_file(&self, path: P) -> io::Result { + self.object + ._open_file(to_cstr(path)?.as_ref(), self.flags | libc::O_RDONLY, 0) + } + + /// Open file for writing, create if necessary, truncate on open + #[inline] + pub fn write_file(&self, path: P, mode: libc::mode_t) -> io::Result { + self.object._open_file( + to_cstr(path)?.as_ref(), + self.flags | libc::O_CREAT | libc::O_WRONLY | libc::O_TRUNC, + mode, + ) + } + + /// Open file for append, create if necessary + #[inline] + pub fn append_file(&self, path: P, mode: libc::mode_t) -> io::Result { + self.object._open_file( + to_cstr(path)?.as_ref(), + self.flags | libc::O_CREAT | libc::O_WRONLY | libc::O_APPEND, + mode, + ) + } + + /// Create file for writing (and truncate) in this directory + #[deprecated(since = "0.1.7", note = "please use `write_file` instead")] + #[inline] + pub fn create_file(&self, path: P, mode: libc::mode_t) -> io::Result { + self.object._open_file( + to_cstr(path)?.as_ref(), + self.flags | libc::O_CREAT | libc::O_WRONLY | libc::O_TRUNC, + mode, + ) + } + + /// Create a tmpfile in this directory which isn't linked to any filename + #[cfg(feature = "o_tmpfile")] + #[inline] + pub fn new_unnamed_file(&self, mode: libc::mode_t) -> io::Result { + self.object._open_file( + unsafe { CStr::from_bytes_with_nul_unchecked(b".\0") }, + self.flags | libc::O_TMPFILE | libc::O_WRONLY, + mode, + ) + } + + /// Create file if not exists, fail if exists + #[inline] + pub fn new_file(&self, path: P, mode: libc::mode_t) -> io::Result { + self.object._open_file( + to_cstr(path)?.as_ref(), + self.flags | libc::O_CREAT | libc::O_EXCL | libc::O_WRONLY, + mode, + ) + } + + /// Creates a new 'Normal' independently owned handle to the underlying directory. + pub fn clone_upgrade(&self) -> io::Result { + Ok(Dir(clone_dirfd_upgrade(self.object.0, self.flags)?)) + } +} diff --git a/src/dir.rs b/src/dir.rs index 74152d8..750fe20 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -10,7 +10,7 @@ use libc; use crate::list::{open_dirfd, DirIter}; use crate::metadata::{self, Metadata}; -use crate::{Dir, AsPath, DirFlags}; +use crate::{Dir, AsPath, DirFlags, DirMethodFlags}; // NOTE(cehteh): removed O_PATH since it is linux only and highly unportable (semantics can't be emulated) // but see open_lite() below. @@ -96,14 +96,12 @@ impl Dir { /// /// You can list directory itself with `list_self`. pub fn list_dir(&self, path: P) -> io::Result { - //TODO(cehteh): with(O_SEARCH_FLAG) - self.sub_dir(path)?.list() + self.with(O_SEARCH_FLAG).sub_dir(path)?.list() } /// List this dir pub fn list_self(&self) -> io::Result { - //TODO(cehteh): with(O_SEARCH_FLAG) - self.clone_upgrade()?.list() + self.with(O_SEARCH_FLAG).clone_upgrade()?.list() } /// Create a DirIter from a Dir @@ -114,6 +112,23 @@ impl Dir { open_dirfd(fd) } + /// Create a flags builder for member methods. Defaults to `O_CLOEXEC | O_NOFOLLOW` plus + /// the given flags. Further flags can be added/removed by the 'with()'/'without()' + /// members. And finally be used by 'sub_dir()' and the different 'open()' calls. + #[inline] + pub fn with<'a>(&'a self, flags: libc::c_int) -> DirMethodFlags<'a> { + DirMethodFlags::new(self, libc::O_CLOEXEC | libc::O_NOFOLLOW | flags) + } + + /// Create a flags builder for member methods. Defaults to `O_CLOEXEC | O_NOFOLLOW` with + /// the given flags (which may O_CLOEXEC or O_NOFOLLOW) removed. Further flags can be + /// added/removed by the 'with()'/'without()' members. And finally be used by 'sub_dir()' + /// and the different 'open()' calls. + #[inline] + pub fn without<'a>(&'a self, flags: libc::c_int) -> DirMethodFlags<'a> { + DirMethodFlags::new(self, (libc::O_CLOEXEC | libc::O_NOFOLLOW) & !flags) + } + /// Open subdirectory /// /// Note that this method does not resolve symlinks by default, so you may have to call @@ -140,7 +155,7 @@ impl Dir { self._sub_dir(to_cstr(path)?.as_ref(), libc::O_CLOEXEC | libc::O_NOFOLLOW | O_PATH_FLAG) } - fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { + pub(crate) fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { Ok(Dir(unsafe { libc_ok(libc::openat(self.0, path.as_ptr(), flags | O_DIRECTORY_FLAG))? })) } @@ -355,7 +370,7 @@ impl Dir { mode) } - fn _open_file(&self, path: &CStr, flags: libc::c_int, mode: libc::mode_t) + pub(crate) fn _open_file(&self, path: &CStr, flags: libc::c_int, mode: libc::mode_t) -> io::Result { unsafe { @@ -513,7 +528,7 @@ impl Dir { /// Creates a new 'Normal' independently owned handle to the underlying directory. pub fn clone_upgrade(&self) -> io::Result { - Ok(Dir(clone_dirfd_upgrade(self.0)?)) + Ok(Dir(clone_dirfd_upgrade(self.0, 0)?)) } /// Creates a new 'Lite' independently owned handle to the underlying directory. @@ -541,13 +556,13 @@ fn clone_dirfd(fd: libc::c_int) -> io::Result { } } -fn clone_dirfd_upgrade(fd: libc::c_int) -> io::Result { +pub(crate) fn clone_dirfd_upgrade(fd: libc::c_int, flags: libc::c_int) -> io::Result { unsafe { match fd_type(fd)? { FdType::NormalDir | FdType::LiteDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - O_DIRECTORY_FLAG | libc::O_CLOEXEC, + flags | O_DIRECTORY_FLAG | libc::O_CLOEXEC, )), _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } @@ -576,7 +591,6 @@ fn clone_dirfd_downgrade(fd: libc::c_int) -> io::Result { } } -//TODO: let fd_type() return the queried flags, clone will need them enum FdType { NormalDir, LiteDir, @@ -584,7 +598,7 @@ enum FdType { } // OSes with fcntl() that returns O_DIRECTORY -// Linux hash O_PATH +// Linux has O_PATH #[cfg(all(feature = "o_path", feature = "fcntl_o_directory"))] fn fd_type(fd: libc::c_int) -> io::Result { let flags = unsafe { libc_ok(libc::fcntl(fd, libc::F_GETFL))? }; diff --git a/src/lib.rs b/src/lib.rs index f0b261b..9b7bfce 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -58,7 +58,7 @@ pub use crate::name::AsPath; pub use crate::dir::{rename, hardlink}; pub use crate::filetype::SimpleType; pub use crate::metadata::Metadata; -pub use crate::builder::{DirFlags}; +pub use crate::builder::{DirFlags, DirMethodFlags}; use std::ffi::CString; use std::os::unix::io::RawFd; diff --git a/tests/flagsbuilder.rs b/tests/flagsbuilder.rs index e24934f..505513d 100644 --- a/tests/flagsbuilder.rs +++ b/tests/flagsbuilder.rs @@ -10,3 +10,33 @@ fn dir_flags_builder_basic() { assert!(dir.is_ok()); } + +fn dir_flags_builder_reuse() { + let dir_flags = Dir::flags() + .without(libc::O_CLOEXEC) + .with(libc::O_NOFOLLOW); + + let src_dir = dir_flags.open("src"); + let tests_dir = dir_flags.open("tests"); + + assert!(src_dir.is_ok()); + assert!(tests_dir.is_ok()); +} + +#[test] +fn method_flags_builder_basic() { + let dir = Dir::open("src").unwrap(); + let file = dir.without(libc::O_NOFOLLOW).open_file("dir.rs"); + assert!(file.is_ok()); +} + +fn method_flags_builder_reuse() { + let dir = Dir::open("src").unwrap(); + let dir_flags = dir.without(libc::O_NOFOLLOW); + + let file1 = dir_flags.open_file("dir.rs"); + let file2 = dir_flags.open_file("builders.rs"); + + assert!(file1.is_ok()); + assert!(file2.is_ok()); +} From 313cd869f9f7b5a5aa87473b5684ea5f7a83bcc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Fri, 30 Apr 2021 20:02:40 +0200 Subject: [PATCH 25/28] bump conf_test version to 0.2 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 4d232f8..88f5a15 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ libc = "0.2.34" [build-dependencies] libc = "0.2.34" -conf_test = "0.1" +conf_test = "0.2" [dev-dependencies] argparse = "0.2.1" From 296d5e58da25e99817de85c6183632e10be7c5b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Fri, 30 Apr 2021 20:03:30 +0200 Subject: [PATCH 26/28] forgotten to enable the 'flag_reuse' tests --- tests/flagsbuilder.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/flagsbuilder.rs b/tests/flagsbuilder.rs index 505513d..6f98b2c 100644 --- a/tests/flagsbuilder.rs +++ b/tests/flagsbuilder.rs @@ -11,10 +11,9 @@ fn dir_flags_builder_basic() { assert!(dir.is_ok()); } +#[test] fn dir_flags_builder_reuse() { - let dir_flags = Dir::flags() - .without(libc::O_CLOEXEC) - .with(libc::O_NOFOLLOW); + let dir_flags = Dir::flags().without(libc::O_CLOEXEC).with(libc::O_NOFOLLOW); let src_dir = dir_flags.open("src"); let tests_dir = dir_flags.open("tests"); @@ -30,12 +29,13 @@ fn method_flags_builder_basic() { assert!(file.is_ok()); } +#[test] fn method_flags_builder_reuse() { let dir = Dir::open("src").unwrap(); let dir_flags = dir.without(libc::O_NOFOLLOW); let file1 = dir_flags.open_file("dir.rs"); - let file2 = dir_flags.open_file("builders.rs"); + let file2 = dir_flags.open_file("builder.rs"); assert!(file1.is_ok()); assert!(file2.is_ok()); From d22b9d4a153697f9201ebe03719e38d0bdc80f94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Sat, 1 May 2021 20:13:37 +0200 Subject: [PATCH 27/28] Remove the '*_lite()' versions of open() and sub_dir() With the flags builder the 'open_lite()' I introduced earlier can be removed. 'Dir::open(), sub_dir()' defaults to include O_PATH when available as before. The flags builder does NOT include O_PATH. 'Dir::list_self()' and 'Dir::list_dir()' already clone-upgrade the handle that removes the O_PATH flag and thus will be not affected. 'Dir::list()' may fail when called with a Dir that is opened with O_PATH and the OS (linux) does not support listing. --- src/builder.rs | 13 -------- src/dir.rs | 88 ++++++++++++++++++++++++++------------------------ 2 files changed, 45 insertions(+), 56 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index de8c0c8..7f78930 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -48,12 +48,6 @@ impl DirFlags { pub fn open(&self, path: P) -> io::Result { Dir::_open(to_cstr(path)?.as_ref(), self.flags) } - - /// Open a lite directory descriptor at specified path - #[inline] - pub fn open_lite(&self, path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref(), O_PATH_FLAG | self.flags) - } } /// 'Dir::with(&self)'/'Dir::with(&self)' creates a new DirMethodsFlags object with default @@ -95,13 +89,6 @@ impl<'a> DirMethodFlags<'a> { self.object._sub_dir(to_cstr(path)?.as_ref(), self.flags) } - /// Open subdirectory with a 'lite' descriptor at specified path - #[inline] - pub fn sub_dir_lite(&self, path: P) -> io::Result { - self.object - ._sub_dir(to_cstr(path)?.as_ref(), self.flags | O_PATH_FLAG) - } - /// Open file for reading in this directory #[inline] pub fn open_file(&self, path: P) -> io::Result { diff --git a/src/dir.rs b/src/dir.rs index 750fe20..add7c99 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -12,9 +12,6 @@ use crate::metadata::{self, Metadata}; use crate::{Dir, AsPath, DirFlags, DirMethodFlags}; -// NOTE(cehteh): removed O_PATH since it is linux only and highly unportable (semantics can't be emulated) -// but see open_lite() below. - #[cfg(feature = "o_directory")] const O_DIRECTORY_FLAG: libc::c_int = libc::O_DIRECTORY; @@ -47,26 +44,23 @@ impl Dir { /// Create a flags builder for Dir objects. Initial flags default to `O_CLOEXEC'. More /// flags can be set added by 'with()' and existing/default flags can be removed by - /// 'without()'. The flags builder can the be used to 'open()' or 'open_lite()' to create + /// 'without()'. The flags builder can the be used to 'open()' to create /// a Dir handle. #[inline] pub fn flags() -> DirFlags { DirFlags::new(libc::O_CLOEXEC) } - /// Open a directory descriptor at specified path - // TODO(tailhook) maybe accept only absolute paths? - pub fn open(path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref(), libc::O_CLOEXEC) - } - - /// Open a 'lite' directory descriptor at specified path + /// Open a directory descriptor at specified path with O_PATH when available. /// A descriptor obtained with this flag is restricted to do only certain operations: /// - It may be used as anchor for opening sub-objects /// - One can query metadata of this directory - /// Using this descriptor for iterating over the content is unspecified. - /// Uses O_PATH on Linux - pub fn open_lite(path: P) -> io::Result { + /// + /// This handle is not suitable for a 'Dir::list()' call and may yield a runtime error. + /// Use 'Dir::flags().open()' to get a handle without O_PATH defined or use + /// 'Dir::list_self()' which clone-upgrades the handle it used for the iteration. + // TODO(tailhook) maybe accept only absolute paths? + pub fn open(path: P) -> io::Result { Dir::_open(to_cstr(path)?.as_ref(), O_PATH_FLAG | libc::O_CLOEXEC) } @@ -86,8 +80,8 @@ impl Dir { /// explicitly. Thats what 'is_dir()' is for. Returns 'true' when the underlying handles /// represents a directory and false otherwise. pub fn is_dir(&self) -> bool { - match fd_type(self.0).unwrap_or(FdType::Other){ - FdType::NormalDir | FdType::LiteDir => true, + match fd_type(self.0).unwrap_or(FdType::Other) { + FdType::NormalDir | FdType::OPathDir => true, FdType::Other => false, } } @@ -105,7 +99,7 @@ impl Dir { } /// Create a DirIter from a Dir - /// Dir must not be a 'Lite' handle + /// Dir must not be a handle opened with O_PATH. pub fn list(self) -> io::Result { let fd = self.0; std::mem::forget(self); @@ -129,30 +123,25 @@ impl Dir { DirMethodFlags::new(self, (libc::O_CLOEXEC | libc::O_NOFOLLOW) & !flags) } - /// Open subdirectory - /// - /// Note that this method does not resolve symlinks by default, so you may have to call - /// [`read_link`] to resolve the real path first. - /// - /// [`read_link`]: #method.read_link - pub fn sub_dir(&self, path: P) -> io::Result { - self._sub_dir(to_cstr(path)?.as_ref(), libc::O_CLOEXEC | libc::O_NOFOLLOW) - } - - /// Open subdirectory with a 'lite' descriptor at specified path + /// Open subdirectory with O_PATH when available. /// A descriptor obtained with this flag is restricted to do only certain operations: /// - It may be used as anchor for opening sub-objects /// - One can query metadata of this directory - /// Using this descriptor for iterating over the content is unspecified. - /// Uses O_PATH on Linux /// - /// Note that this method does not resolve symlinks by default, so you may have to call + /// This handle is not suitable for a 'Dir::list()' call and may yield a runtime error. + /// Use 'Dir::with(0).sub_dir()' to get a handle without O_PATH defined or use + /// 'Dir::list_dir()' which clone-upgrades the handle it used for the iteration. /// - /// [`read_link`] to resolve the real path first. + /// Note that this method does not resolve symlinks by default, so you may have to call + /// [`read_link`] to resolve the real path first or create a handle + /// 'without(libc::O_NOFOLLOW)'. /// /// [`read_link`]: #method.read_link - pub fn sub_dir_lite(&self, path: P) -> io::Result { - self._sub_dir(to_cstr(path)?.as_ref(), libc::O_CLOEXEC | libc::O_NOFOLLOW | O_PATH_FLAG) + pub fn sub_dir(&self, path: P) -> io::Result { + self._sub_dir( + to_cstr(path)?.as_ref(), + O_PATH_FLAG | libc::O_CLOEXEC | libc::O_NOFOLLOW, + ) } pub(crate) fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { @@ -515,13 +504,13 @@ impl Dir { /// closing it when it goes out of scope. pub unsafe fn from_raw_fd_checked(fd: RawFd) -> io::Result { match fd_type(fd)? { - FdType::NormalDir | FdType::LiteDir => Ok(Dir(fd)), + FdType::NormalDir | FdType::OPathDir => Ok(Dir(fd)), FdType::Other => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } } /// Creates a new independently owned handle to the underlying directory. - /// The new handle has the same (Normal/Lite) semantics as the original handle. + /// The new handle has the same (Normal/O_PATH) semantics as the original handle. pub fn try_clone(&self) -> io::Result { Ok(Dir(clone_dirfd(self.0)?)) } @@ -531,11 +520,10 @@ impl Dir { Ok(Dir(clone_dirfd_upgrade(self.0, 0)?)) } - /// Creates a new 'Lite' independently owned handle to the underlying directory. + /// Creates a new 'O_PATH' restricted independently owned handle to the underlying directory. pub fn clone_downgrade(&self) -> io::Result { Ok(Dir(clone_dirfd_downgrade(self.0)?)) } - } const CURRENT_DIRECTORY: [libc::c_char; 2] = [b'.' as libc::c_char, 0]; @@ -550,7 +538,7 @@ fn clone_dirfd(fd: libc::c_int) -> io::Result { O_DIRECTORY_FLAG | libc::O_CLOEXEC, )), #[cfg(feature = "o_path")] - FdType::LiteDir => libc_ok(libc::dup(fd)), + FdType::OPathDir => libc_ok(libc::dup(fd)), _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } } @@ -559,7 +547,7 @@ fn clone_dirfd(fd: libc::c_int) -> io::Result { pub(crate) fn clone_dirfd_upgrade(fd: libc::c_int, flags: libc::c_int) -> io::Result { unsafe { match fd_type(fd)? { - FdType::NormalDir | FdType::LiteDir => libc_ok(libc::openat( + FdType::NormalDir | FdType::OPathDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, flags | O_DIRECTORY_FLAG | libc::O_CLOEXEC, @@ -585,7 +573,7 @@ fn clone_dirfd_downgrade(fd: libc::c_int) -> io::Result { O_DIRECTORY_FLAG | libc::O_CLOEXEC, )), #[cfg(feature = "o_path")] - FdType::LiteDir => libc_ok(libc::dup(fd)), + FdType::OPathDir => libc_ok(libc::dup(fd)), _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } } @@ -593,7 +581,7 @@ fn clone_dirfd_downgrade(fd: libc::c_int) -> io::Result { enum FdType { NormalDir, - LiteDir, + OPathDir, Other, } @@ -604,7 +592,7 @@ fn fd_type(fd: libc::c_int) -> io::Result { let flags = unsafe { libc_ok(libc::fcntl(fd, libc::F_GETFL))? }; if flags & libc::O_DIRECTORY != 0 { if flags & libc::O_PATH != 0 { - Ok(FdType::LiteDir) + Ok(FdType::OPathDir) } else { Ok(FdType::NormalDir) } @@ -811,6 +799,20 @@ mod test { #[test] fn test_list() { + let dir = Dir::flags().open("src").unwrap(); + let me = dir.list().unwrap(); + assert!(me + .collect::, _>>() + .unwrap() + .iter() + .find(|x| { x.file_name() == Path::new("lib.rs").as_os_str() }) + .is_some()); + } + + #[test] + #[cfg(feature = "o_path")] + #[should_panic(expected = "Bad file descriptor")] + fn test_list_opath_fail() { let dir = Dir::open("src").unwrap(); let me = dir.list().unwrap(); assert!(me From cf112597267aa57a42d26bc46943370c2a5a0a1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Th=C3=A4ter?= Date: Sat, 1 May 2021 23:39:03 +0200 Subject: [PATCH 28/28] export O_PATH, O_SEARCH, O_DIRECTORY flags This allows portable access to these flags. When not supported by the OS the value is 0. An Application can thus just use these definitions with the flags builder as well as doing runtime checks by comparing them against 0. --- src/builder.rs | 2 +- src/dir.rs | 40 ++++++++++++++++++++++++---------------- src/lib.rs | 2 +- tests/flagsbuilder.rs | 11 ++++++++++- 4 files changed, 36 insertions(+), 19 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 7f78930..909d798 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -2,7 +2,7 @@ use std::ffi::CStr; use std::fs::File; use std::io; -use crate::dir::{clone_dirfd_upgrade, to_cstr, O_PATH_FLAG}; +use crate::dir::{clone_dirfd_upgrade, to_cstr}; use crate::{AsPath, Dir}; /// 'Dir::new()' creates a new DirFlags object with default (O_CLOEXEC) flags. One can then diff --git a/src/dir.rs b/src/dir.rs index add7c99..06f79ec 100644 --- a/src/dir.rs +++ b/src/dir.rs @@ -13,20 +13,26 @@ use crate::metadata::{self, Metadata}; use crate::{Dir, AsPath, DirFlags, DirMethodFlags}; +/// Value if the libc::O_DIRECTORY flag when supported by the system, otherwise 0 #[cfg(feature = "o_directory")] -const O_DIRECTORY_FLAG: libc::c_int = libc::O_DIRECTORY; +pub const O_DIRECTORY: libc::c_int = libc::O_DIRECTORY; +/// Value if the libc::O_DIRECTORY flag when supported by the system, otherwise 0 #[cfg(not(feature = "o_directory"))] -const O_DIRECTORY_FLAG: libc::c_int = 0; +pub const O_DIRECTORY: libc::c_int = 0; +/// Value if the libc::O_PATH flag when supported by the system, otherwise 0 #[cfg(feature = "o_path")] -pub(crate) const O_PATH_FLAG: libc::c_int = libc::O_PATH; +pub const O_PATH: libc::c_int = libc::O_PATH; +/// Value if the libc::O_PATH flag when supported by the system, otherwise 0 #[cfg(not(feature = "o_path"))] -pub(crate) const O_PATH_FLAG: libc::c_int = 0; +pub const O_PATH: libc::c_int = 0; +/// Value if the libc::O_SEARCH flag when supported by the system, otherwise 0 #[cfg(feature = "o_search")] -const O_SEARCH_FLAG: libc::c_int = libc::O_SEARCH; +pub const O_SEARCH: libc::c_int = libc::O_SEARCH; +/// Value if the libc::O_SEARCH flag when supported by the system, otherwise 0 #[cfg(not(feature = "o_search"))] -const O_SEARCH_FLAG: libc::c_int = 0; +pub const O_SEARCH: libc::c_int = 0; impl Dir { /// Creates a directory descriptor that resolves paths relative to current @@ -61,11 +67,11 @@ impl Dir { /// 'Dir::list_self()' which clone-upgrades the handle it used for the iteration. // TODO(tailhook) maybe accept only absolute paths? pub fn open(path: P) -> io::Result { - Dir::_open(to_cstr(path)?.as_ref(), O_PATH_FLAG | libc::O_CLOEXEC) + Dir::_open(to_cstr(path)?.as_ref(), O_PATH | libc::O_CLOEXEC) } pub(crate) fn _open(path: &CStr, flags: libc::c_int) -> io::Result { - let fd = unsafe { libc_ok(libc::open(path.as_ptr(), O_DIRECTORY_FLAG | flags))? }; + let fd = unsafe { libc_ok(libc::open(path.as_ptr(), O_DIRECTORY | flags))? }; Ok(Dir(fd)) } @@ -90,12 +96,12 @@ impl Dir { /// /// You can list directory itself with `list_self`. pub fn list_dir(&self, path: P) -> io::Result { - self.with(O_SEARCH_FLAG).sub_dir(path)?.list() + self.with(O_SEARCH).sub_dir(path)?.list() } /// List this dir pub fn list_self(&self) -> io::Result { - self.with(O_SEARCH_FLAG).clone_upgrade()?.list() + self.with(O_SEARCH).clone_upgrade()?.list() } /// Create a DirIter from a Dir @@ -140,12 +146,14 @@ impl Dir { pub fn sub_dir(&self, path: P) -> io::Result { self._sub_dir( to_cstr(path)?.as_ref(), - O_PATH_FLAG | libc::O_CLOEXEC | libc::O_NOFOLLOW, + O_PATH | libc::O_CLOEXEC | libc::O_NOFOLLOW, ) } pub(crate) fn _sub_dir(&self, path: &CStr, flags: libc::c_int) -> io::Result { - Ok(Dir(unsafe { libc_ok(libc::openat(self.0, path.as_ptr(), flags | O_DIRECTORY_FLAG))? })) + Ok(Dir(unsafe { + libc_ok(libc::openat(self.0, path.as_ptr(), flags | O_DIRECTORY))? + })) } /// Read link in this directory @@ -535,7 +543,7 @@ fn clone_dirfd(fd: libc::c_int) -> io::Result { FdType::NormalDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - O_DIRECTORY_FLAG | libc::O_CLOEXEC, + O_DIRECTORY | libc::O_CLOEXEC, )), #[cfg(feature = "o_path")] FdType::OPathDir => libc_ok(libc::dup(fd)), @@ -550,7 +558,7 @@ pub(crate) fn clone_dirfd_upgrade(fd: libc::c_int, flags: libc::c_int) -> io::Re FdType::NormalDir | FdType::OPathDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - flags | O_DIRECTORY_FLAG | libc::O_CLOEXEC, + flags | O_DIRECTORY | libc::O_CLOEXEC, )), _ => Err(io::Error::from_raw_os_error(libc::ENOTDIR)), } @@ -564,13 +572,13 @@ fn clone_dirfd_downgrade(fd: libc::c_int) -> io::Result { FdType::NormalDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - libc::O_PATH | O_DIRECTORY_FLAG | libc::O_CLOEXEC, + libc::O_PATH | O_DIRECTORY | libc::O_CLOEXEC, )), #[cfg(not(feature = "o_path"))] FdType::NormalDir => libc_ok(libc::openat( fd, &CURRENT_DIRECTORY as *const libc::c_char, - O_DIRECTORY_FLAG | libc::O_CLOEXEC, + O_DIRECTORY | libc::O_CLOEXEC, )), #[cfg(feature = "o_path")] FdType::OPathDir => libc_ok(libc::dup(fd)), diff --git a/src/lib.rs b/src/lib.rs index 9b7bfce..89fdfbd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -55,7 +55,7 @@ mod builder; pub use crate::list::DirIter; pub use crate::name::AsPath; -pub use crate::dir::{rename, hardlink}; +pub use crate::dir::{hardlink, rename, O_DIRECTORY, O_PATH, O_SEARCH}; pub use crate::filetype::SimpleType; pub use crate::metadata::Metadata; pub use crate::builder::{DirFlags, DirMethodFlags}; diff --git a/tests/flagsbuilder.rs b/tests/flagsbuilder.rs index 6f98b2c..4ca0b50 100644 --- a/tests/flagsbuilder.rs +++ b/tests/flagsbuilder.rs @@ -1,5 +1,5 @@ extern crate openat; -use openat::Dir; +use openat::{Dir}; #[test] fn dir_flags_builder_basic() { @@ -40,3 +40,12 @@ fn method_flags_builder_reuse() { assert!(file1.is_ok()); assert!(file2.is_ok()); } + +#[test] +fn method_flags_exported() { + let dir = Dir::flags() + .with(openat::O_PATH) + .open("src"); + + assert!(dir.is_ok()); +}