address review feedback

* use ? rather than try! * in rename tests, assert the new path exists after the call
nix-rust · Jul 17, 2019 · e634614 · e634614
1 parent b3edf81
commit e634614
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 9 deletions.
diff --git a/src/fcntl.rs b/src/fcntl.rs
@@ -167,22 +167,22 @@ pub fn openat<P: ?Sized + NixPath>(dirfd: RawFd, path: &P, oflag: OFlag, mode: M
 }
 
 pub fn rename<P1: ?Sized + NixPath, P2: ?Sized + NixPath>(old: &P1, new: &P2) -> Result<()> {
-    let res = try!(try!(old.with_nix_path(|old_cstr| {
-        new.with_nix_path(|new_cstr| {
-            unsafe { libc::rename(old_cstr.as_ptr(), new_cstr.as_ptr()) }
+    let res = old.with_nix_path(|old_cstr| {
+        new.with_nix_path(|new_cstr| unsafe {
+            libc::rename(old_cstr.as_ptr(), new_cstr.as_ptr())
         })
-    })));
+    })??;
     Errno::result(res).map(drop)
 }
 
 pub fn renameat<P1: ?Sized + NixPath, P2: ?Sized + NixPath>(old_dirfd: RawFd, old_path: &P1,
                                                             new_dirfd: RawFd, new_path: &P2)
                                                             -> Result<()> {
-    let res = try!(try!(old_path.with_nix_path(|old_cstr| {
-        new_path.with_nix_path(|new_cstr| {
-            unsafe { libc::renameat(old_dirfd, old_cstr.as_ptr(), new_dirfd, new_cstr.as_ptr()) }
+    let res = old_path.with_nix_path(|old_cstr| {
+        new_path.with_nix_path(|new_cstr| unsafe {
+            libc::renameat(old_dirfd, old_cstr.as_ptr(), new_dirfd, new_cstr.as_ptr())
         })
-    })));
+    })??;
     Errno::result(res).map(drop)
 }
 

diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs
@@ -39,20 +39,25 @@ fn test_rename() {
     let new_path = new_dir.path().join("new");
     rename(&old_path, &new_path).unwrap();
     assert_eq!(rename(&old_path, &new_path).unwrap_err(), Error::Sys(Errno::ENOENT));
+    assert!(!old_path.exists());
+    assert!(new_path.exists());
 }
 
 #[test]
 fn test_renameat() {
     let old_dir = tempfile::tempdir().unwrap();
     let old_dirfd = open(old_dir.path(), OFlag::empty(), Mode::empty()).unwrap();
-    File::create(&old_dir.path().join("old")).unwrap();
+    let old_path = old_dir.path().join("old");
+    File::create(&old_path).unwrap();
     let new_dir = tempfile::tempdir().unwrap();
     let new_dirfd = open(new_dir.path(), OFlag::empty(), Mode::empty()).unwrap();
     renameat(old_dirfd, "old", new_dirfd, "new").unwrap();
     assert_eq!(renameat(old_dirfd, "old", new_dirfd, "new").unwrap_err(),
                Error::Sys(Errno::ENOENT));
     close(old_dirfd).unwrap();
     close(new_dirfd).unwrap();
+    assert!(!old_path.exists());
+    assert!(new_dir.path().join("new").exists());
 }
 
 #[test]