refactor: change dirfd arg of xxxat() to Option<fd> (#2157)

SteveLauC · web-flow · commit e5cc5cea4640 · 2023-11-05T06:02:40.000Z
* refactor: change dirfd arg of xxxat() to Option&lt;fd&gt;

* changelog

* gate at_rawfd() with features fs or process

* fix import

* fix test on Android

* simplify at_rawfd()

* update changelog
diff --git a/changelog/2157.changed.md b/changelog/2157.changed.md
@@ -0,0 +1,7 @@
+The following APIs now take optional `dirfd`s:
+
+- `readlinkat()`
+- `fstatat()`
+- `mknodat()`
+- `mkdirat()`
+- `execveat()`
diff --git a/src/fcntl.rs b/src/fcntl.rs
@@ -217,6 +217,15 @@ libc_bitflags!(
     }
 );
 
+/// Computes the raw fd consumed by a function of the form `*at`.
+#[cfg(any(
+    all(feature = "fs", not(target_os = "redox")),
+    all(feature = "process", any(target_os = "android", target_os = "linux"))
+))]
+pub(crate) fn at_rawfd(fd: Option<RawFd>) -> raw::c_int {
+    fd.unwrap_or(libc::AT_FDCWD)
+}
+
 feature! {
 #![feature = "fs"]
 
@@ -366,7 +375,7 @@ fn inner_readlink<P: ?Sized + NixPath>(
                     AtFlags::empty()
                 };
                 super::sys::stat::fstatat(
-                    dirfd,
+                    Some(dirfd),
                     path,
                     flags | AtFlags::AT_SYMLINK_NOFOLLOW,
                 )
@@ -377,7 +386,7 @@ fn inner_readlink<P: ?Sized + NixPath>(
                 target_os = "redox"
             )))]
             Some(dirfd) => super::sys::stat::fstatat(
-                dirfd,
+                Some(dirfd),
                 path,
                 AtFlags::AT_SYMLINK_NOFOLLOW,
             ),
@@ -424,20 +433,12 @@ pub fn readlink<P: ?Sized + NixPath>(path: &P) -> Result<OsString> {
 
 #[cfg(not(target_os = "redox"))]
 pub fn readlinkat<P: ?Sized + NixPath>(
-    dirfd: RawFd,
+    dirfd: Option<RawFd>,
     path: &P,
 ) -> Result<OsString> {
+    let dirfd = at_rawfd(dirfd);
     inner_readlink(Some(dirfd), path)
 }
-
-/// Computes the raw fd consumed by a function of the form `*at`.
-#[cfg(not(target_os = "redox"))]
-pub(crate) fn at_rawfd(fd: Option<RawFd>) -> raw::c_int {
-    match fd {
-        None => libc::AT_FDCWD,
-        Some(fd) => fd,
-    }
-}
 }
 
 #[cfg(any(target_os = "android", target_os = "linux", target_os = "freebsd"))]
diff --git a/src/sys/stat.rs b/src/sys/stat.rs
@@ -192,12 +192,13 @@ pub fn mknod<P: ?Sized + NixPath>(
 )))]
 #[cfg_attr(docsrs, doc(cfg(all())))]
 pub fn mknodat<P: ?Sized + NixPath>(
-    dirfd: RawFd,
+    dirfd: Option<RawFd>,
     path: &P,
     kind: SFlag,
     perm: Mode,
     dev: dev_t,
 ) -> Result<()> {
+    let dirfd = at_rawfd(dirfd);
     let res = path.with_nix_path(|cstr| unsafe {
         libc::mknodat(
             dirfd,
@@ -270,10 +271,11 @@ pub fn fstat(fd: RawFd) -> Result<FileStat> {
 #[cfg(not(target_os = "redox"))]
 #[cfg_attr(docsrs, doc(cfg(all())))]
 pub fn fstatat<P: ?Sized + NixPath>(
-    dirfd: RawFd,
+    dirfd: Option<RawFd>,
     pathname: &P,
     f: AtFlags,
 ) -> Result<FileStat> {
+    let dirfd = at_rawfd(dirfd);
     let mut dst = mem::MaybeUninit::uninit();
     let res = pathname.with_nix_path(|cstr| unsafe {
         libc::fstatat(
@@ -468,10 +470,11 @@ pub fn utimensat<P: ?Sized + NixPath>(
 #[cfg(not(target_os = "redox"))]
 #[cfg_attr(docsrs, doc(cfg(all())))]
 pub fn mkdirat<P: ?Sized + NixPath>(
-    fd: RawFd,
+    fd: Option<RawFd>,
     path: &P,
     mode: Mode,
 ) -> Result<()> {
+    let fd = at_rawfd(fd);
     let res = path.with_nix_path(|cstr| unsafe {
         libc::mkdirat(fd, cstr.as_ptr(), mode.bits() as mode_t)
     })?;
diff --git a/src/unistd.rs b/src/unistd.rs
@@ -1,9 +1,16 @@
 //! Safe wrappers around functions found in libc "unistd.h" header
 
 use crate::errno::{self, Errno};
+
+#[cfg(any(
+    all(feature = "fs", not(target_os = "redox")),
+    all(feature = "process", any(target_os = "android", target_os = "linux"))
+))]
+use crate::fcntl::at_rawfd;
 #[cfg(not(target_os = "redox"))]
 #[cfg(feature = "fs")]
-use crate::fcntl::{at_rawfd, AtFlags};
+use crate::fcntl::AtFlags;
+
 #[cfg(feature = "fs")]
 use crate::fcntl::{fcntl, FcntlArg::F_SETFD, FdFlag, OFlag};
 #[cfg(all(
@@ -939,12 +946,13 @@ pub fn fexecve<SA: AsRef<CStr>, SE: AsRef<CStr>>(
 #[cfg(any(target_os = "android", target_os = "linux"))]
 #[inline]
 pub fn execveat<SA: AsRef<CStr>, SE: AsRef<CStr>>(
-    dirfd: RawFd,
+    dirfd: Option<RawFd>,
     pathname: &CStr,
     args: &[SA],
     env: &[SE],
     flags: super::fcntl::AtFlags,
 ) -> Result<Infallible> {
+    let dirfd = at_rawfd(dirfd);
     let args_p = to_exec_array(args);
     let env_p = to_exec_array(env);
 
diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs
@@ -222,7 +222,7 @@ fn test_readlink() {
 
     assert_eq!(readlink(&dst).unwrap().to_str().unwrap(), expected_dir);
     assert_eq!(
-        readlinkat(dirfd, "b").unwrap().to_str().unwrap(),
+        readlinkat(Some(dirfd), "b").unwrap().to_str().unwrap(),
         expected_dir
     );
 }
diff --git a/test/test_stat.rs b/test/test_stat.rs
@@ -117,7 +117,7 @@ fn test_fstatat() {
         fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty());
 
     let result =
-        stat::fstatat(dirfd.unwrap(), &filename, fcntl::AtFlags::empty());
+        stat::fstatat(Some(dirfd.unwrap()), &filename, fcntl::AtFlags::empty());
     assert_stat_results(result);
 }
 
@@ -323,7 +323,7 @@ fn test_mkdirat_success_path() {
     let dirfd =
         fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty())
             .unwrap();
-    mkdirat(dirfd, filename, Mode::S_IRWXU).expect("mkdirat failed");
+    mkdirat(Some(dirfd), filename, Mode::S_IRWXU).expect("mkdirat failed");
     assert!(Path::exists(&tempdir.path().join(filename)));
 }
 
@@ -337,7 +337,7 @@ fn test_mkdirat_success_mode() {
     let dirfd =
         fcntl::open(tempdir.path(), fcntl::OFlag::empty(), stat::Mode::empty())
             .unwrap();
-    mkdirat(dirfd, filename, Mode::S_IRWXU).expect("mkdirat failed");
+    mkdirat(Some(dirfd), filename, Mode::S_IRWXU).expect("mkdirat failed");
     let permissions = fs::metadata(tempdir.path().join(filename))
         .unwrap()
         .permissions();
@@ -357,7 +357,7 @@ fn test_mkdirat_fail() {
         stat::Mode::empty(),
     )
     .unwrap();
-    let result = mkdirat(dirfd, filename, Mode::S_IRWXU).unwrap_err();
+    let result = mkdirat(Some(dirfd), filename, Mode::S_IRWXU).unwrap_err();
     assert_eq!(result, Errno::ENOTDIR);
 }
 
@@ -402,15 +402,15 @@ fn test_mknodat() {
     let target_dir =
         Dir::open(tempdir.path(), OFlag::O_DIRECTORY, Mode::S_IRWXU).unwrap();
     mknodat(
-        target_dir.as_raw_fd(),
+        Some(target_dir.as_raw_fd()),
         file_name,
         SFlag::S_IFREG,
         Mode::S_IRWXU,
         0,
     )
     .unwrap();
     let mode = fstatat(
-        target_dir.as_raw_fd(),
+        Some(target_dir.as_raw_fd()),
         file_name,
         AtFlags::AT_SYMLINK_NOFOLLOW,
     )
diff --git a/test/test_unistd.rs b/test/test_unistd.rs
@@ -163,7 +163,8 @@ fn test_mkfifoat() {
     mkfifoat(Some(dirfd), mkfifoat_name, Mode::S_IRUSR).unwrap();
 
     let stats =
-        stat::fstatat(dirfd, mkfifoat_name, fcntl::AtFlags::empty()).unwrap();
+        stat::fstatat(Some(dirfd), mkfifoat_name, fcntl::AtFlags::empty())
+            .unwrap();
     let typ = stat::SFlag::from_bits_truncate(stats.st_mode);
     assert_eq!(typ, SFlag::S_IFIFO);
 }
@@ -197,7 +198,7 @@ fn test_mkfifoat_directory() {
     let tempdir = tempdir().unwrap();
     let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap();
     let mkfifoat_dir = "mkfifoat_dir";
-    stat::mkdirat(dirfd, mkfifoat_dir, Mode::S_IRUSR).unwrap();
+    stat::mkdirat(Some(dirfd), mkfifoat_dir, Mode::S_IRUSR).unwrap();
 
     mkfifoat(Some(dirfd), mkfifoat_dir, Mode::S_IRUSR)
         .expect_err("assertion failed");
@@ -429,21 +430,21 @@ cfg_if! {
     if #[cfg(target_os = "android")] {
         use nix::fcntl::AtFlags;
         execve_test_factory!(test_execveat_empty, execveat,
-                             File::open("/system/bin/sh").unwrap().into_raw_fd(),
+                             Some(File::open("/system/bin/sh").unwrap().into_raw_fd()),
                              "", AtFlags::AT_EMPTY_PATH);
         execve_test_factory!(test_execveat_relative, execveat,
-                             File::open("/system/bin/").unwrap().into_raw_fd(),
+                             Some(File::open("/system/bin/").unwrap().into_raw_fd()),
                              "./sh", AtFlags::empty());
         execve_test_factory!(test_execveat_absolute, execveat,
-                             File::open("/").unwrap().into_raw_fd(),
+                             Some(File::open("/").unwrap().into_raw_fd()),
                              "/system/bin/sh", AtFlags::empty());
     } else if #[cfg(all(target_os = "linux", any(target_arch ="x86_64", target_arch ="x86")))] {
         use nix::fcntl::AtFlags;
-        execve_test_factory!(test_execveat_empty, execveat, File::open("/bin/sh").unwrap().into_raw_fd(),
+        execve_test_factory!(test_execveat_empty, execveat, Some(File::open("/bin/sh").unwrap().into_raw_fd()),
                              "", AtFlags::AT_EMPTY_PATH);
-        execve_test_factory!(test_execveat_relative, execveat, File::open("/bin/").unwrap().into_raw_fd(),
+        execve_test_factory!(test_execveat_relative, execveat, Some(File::open("/bin/").unwrap().into_raw_fd()),
                              "./sh", AtFlags::empty());
-        execve_test_factory!(test_execveat_absolute, execveat, File::open("/").unwrap().into_raw_fd(),
+        execve_test_factory!(test_execveat_absolute, execveat, Some(File::open("/").unwrap().into_raw_fd()),
                              "/bin/sh", AtFlags::empty());
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -222,7 +222,7 @@ fn test_readlink() {`
`222`	`222`
`223`	`223`	`assert_eq!(readlink(&dst).unwrap().to_str().unwrap(), expected_dir);`
`224`	`224`	`assert_eq!(`
`225`		`- readlinkat(dirfd, "b").unwrap().to_str().unwrap(),`
	`225`	`+ readlinkat(Some(dirfd), "b").unwrap().to_str().unwrap(),`
`226`	`226`	`expected_dir`
`227`	`227`	`);`
`228`	`228`	`}`