From a50b4765780adbbf637ba0ea2190b33260d4c38e Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Sat, 8 Jul 2017 12:11:07 -0600 Subject: Fix double close bugs in test_lseek and test_lseek64 std::fs::File closes the underlying file descriptor on Drop, without checking for errors. test_lseek and test_lseek64 also manually close the file descriptor. That works for single threaded test runs. But for multithreaded runs, it causes EBADF errors in other tests. Fix the tests by consuming the File with into_raw_fd(), so its drop method will never be called. Also, fix a potential short read bug in the same tests. --- test/test.rs | 12 ++++++++++++ test/test_pty.rs | 24 +++++++----------------- test/test_unistd.rs | 26 ++++++++++++++------------ 3 files changed, 33 insertions(+), 29 deletions(-) diff --git a/test/test.rs b/test/test.rs index e2f5a024..2cf30360 100644 --- a/test/test.rs +++ b/test/test.rs @@ -24,6 +24,18 @@ mod test_stat; mod test_unistd; use nixtest::assert_size_of; +use std::os::unix::io::RawFd; +use nix::unistd::read; + +/// Helper function analogous to std::io::Read::read_exact, but for `RawFD`s +fn read_exact(f: RawFd, buf: &mut [u8]) { + let mut len = 0; + while len < buf.len() { + // get_mut would be better than split_at_mut, but it requires nightly + let (_, remaining) = buf.split_at_mut(len); + len += read(f, remaining).unwrap(); + } +} #[test] pub fn test_size_of_long() { diff --git a/test/test_pty.rs b/test/test_pty.rs index b1c2d630..8528caf7 100644 --- a/test/test_pty.rs +++ b/test/test_pty.rs @@ -5,17 +5,7 @@ use nix::fcntl::{O_RDWR, open}; use nix::pty::*; use nix::sys::stat; use nix::sys::termios::*; -use nix::unistd::{read, write, close}; - -/// Helper function analogous to std::io::Read::read_exact, but for `RawFD`s -fn read_exact(f: RawFd, buf: &mut [u8]) { - let mut len = 0; - while len < buf.len() { - // get_mut would be better than split_at_mut, but it requires nightly - let (_, remaining) = buf.split_at_mut(len); - len += read(f, remaining).unwrap(); - } -} +use nix::unistd::{write, close}; /// Test equivalence of `ptsname` and `ptsname_r` #[test] @@ -115,21 +105,21 @@ fn test_openpty() { let string = "foofoofoo\n"; let mut buf = [0u8; 10]; write(pty.master, string.as_bytes()).unwrap(); - read_exact(pty.slave, &mut buf); + ::read_exact(pty.slave, &mut buf); assert_eq!(&buf, string.as_bytes()); // Read the echo as well let echoed_string = "foofoofoo\r\n"; let mut buf = [0u8; 11]; - read_exact(pty.master, &mut buf); + ::read_exact(pty.master, &mut buf); assert_eq!(&buf, echoed_string.as_bytes()); let string2 = "barbarbarbar\n"; let echoed_string2 = "barbarbarbar\r\n"; let mut buf = [0u8; 14]; write(pty.slave, string2.as_bytes()).unwrap(); - read_exact(pty.master, &mut buf); + ::read_exact(pty.master, &mut buf); assert_eq!(&buf, echoed_string2.as_bytes()); @@ -160,20 +150,20 @@ fn test_openpty_with_termios() { let string = "foofoofoo\n"; let mut buf = [0u8; 10]; write(pty.master, string.as_bytes()).unwrap(); - read_exact(pty.slave, &mut buf); + ::read_exact(pty.slave, &mut buf); assert_eq!(&buf, string.as_bytes()); // read the echo as well let echoed_string = "foofoofoo\n"; - read_exact(pty.master, &mut buf); + ::read_exact(pty.master, &mut buf); assert_eq!(&buf, echoed_string.as_bytes()); let string2 = "barbarbarbar\n"; let echoed_string2 = "barbarbarbar\n"; let mut buf = [0u8; 13]; write(pty.slave, string2.as_bytes()).unwrap(); - read_exact(pty.master, &mut buf); + ::read_exact(pty.master, &mut buf); assert_eq!(&buf, echoed_string2.as_bytes()); diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 6012de97..22aa5da8 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -7,7 +7,7 @@ use nix::sys::stat; use std::iter; use std::ffi::CString; use std::fs::File; -use std::io::{Write, Read}; +use std::io::Write; use std::os::unix::prelude::*; use std::env::current_dir; use tempfile::tempfile; @@ -185,16 +185,17 @@ fn test_getcwd() { fn test_lseek() { const CONTENTS: &'static [u8] = b"abcdef123456"; let mut tmp = tempfile().unwrap(); - tmp.write(CONTENTS).unwrap(); + tmp.write_all(CONTENTS).unwrap(); + let tmpfd = tmp.into_raw_fd(); let offset: off_t = 5; - lseek(tmp.as_raw_fd(), offset, Whence::SeekSet).unwrap(); + lseek(tmpfd, offset, Whence::SeekSet).unwrap(); - let mut buf = String::new(); - tmp.read_to_string(&mut buf).unwrap(); - assert_eq!(b"f123456", buf.as_bytes()); + let mut buf = [0u8; 7]; + ::read_exact(tmpfd, &mut buf); + assert_eq!(b"f123456", &buf); - close(tmp.as_raw_fd()).unwrap(); + close(tmpfd).unwrap(); } #[cfg(any(target_os = "linux", target_os = "android"))] @@ -203,14 +204,15 @@ fn test_lseek64() { const CONTENTS: &'static [u8] = b"abcdef123456"; let mut tmp = tempfile().unwrap(); tmp.write(CONTENTS).unwrap(); + let tmpfd = tmp.into_raw_fd(); - lseek64(tmp.as_raw_fd(), 5, Whence::SeekSet).unwrap(); + lseek64(tmpfd, 5, Whence::SeekSet).unwrap(); - let mut buf = String::new(); - tmp.read_to_string(&mut buf).unwrap(); - assert_eq!(b"f123456", buf.as_bytes()); + let mut buf = [0u8; 7]; + ::read_exact(tmpfd, &mut buf); + assert_eq!(b"f123456", &buf); - close(tmp.as_raw_fd()).unwrap(); + close(tmpfd).unwrap(); } execve_test_factory!(test_execve, execve, b"/bin/sh", b"/system/bin/sh"); -- cgit v1.2.3