Cleanup readv & writev + tests

Author: carllerche

Cargo.toml

@@ -22,3 +22,8 @@ rand = "0.1.2"
 [dev-dependencies.nix-test]
 path    = "nix-test"
 version = "*"
+
+[[test]]
+
+name = "test"
+path = "test/test.rs"

nix-test/Makefile

@@ -1,6 +1,7 @@
 OUT    = libnixtest.a
 CFLAGS = -fPIC -D$(OS)
-OBJS   = errno.o
+OBJS   = errno.o \
+		 sizes.o
 
 $(OUT): $(OBJS)
 	ar -rcs $@ $^

nix-test/src/lib.rs

@@ -6,24 +6,41 @@ use std::ffi::CString;
 use libc::{c_int};
 
 mod ffi {
-    use libc::{c_int, c_char};
+    use libc::{c_int, c_char, size_t};
 
     #[link(name = "nixtest", kind = "static")]
     extern {
         pub fn assert_errno_eq(errno: *const c_char) -> c_int;
+        pub fn size_of(ty: *const c_char) -> size_t;
     }
 }
 
-pub fn assert_errno_eq(err: &str, val: c_int) {
+pub fn assert_errno_eq(name: &str, actual: c_int) {
     unsafe {
-        let name = CString::new(err).unwrap();
-        let actual = ffi::assert_errno_eq(name.as_ptr());
+        let cstr = CString::new(name).unwrap();
+        let expect = ffi::assert_errno_eq(cstr.as_ptr());
 
-        assert!(actual > 0);
+        assert!(expect > 0, "undefined errno {}", name);
 
-        if val != actual {
-            panic!("incorrect value for errno {}; got={}; expected={}",
-                   err, val, actual);
+        if actual != expect {
+            panic!("incorrect value for errno {}; expect={}; actual={}",
+                   name, expect, actual);
+        }
+    }
+}
+
+pub fn assert_size_of<T>(name: &str) {
+    use std::mem;
+
+    unsafe {
+        let cstr = CString::new(name).unwrap();
+        let expect = ffi::size_of(cstr.as_ptr()) as usize;
+
+        assert!(expect > 0, "undefined type {}", name);
+
+        if mem::size_of::<T>() != expect {
+            panic!("incorrectly sized type; expect={}; actual={}",
+                   expect, mem::size_of::<T>());
         }
     }
 }

nix-test/src/sizes.c

@@ -0,0 +1,23 @@
+#include "sys/uio.h"
+
+#define SIZE_OF_T(TYPE)                   \
+    do {                                  \
+        if (0 == strcmp(type, #TYPE)) {   \
+            return sizeof(TYPE);          \
+        }                                 \
+    } while (0)
+
+#define SIZE_OF_S(TYPE)                   \
+    do {                                  \
+        if (0 == strcmp(type, #TYPE)) {   \
+            return sizeof(struct TYPE);   \
+        }                                 \
+    } while (0)
+
+size_t
+size_of(const char* type) {
+    // sys/uio
+    SIZE_OF_S(iovec);
+
+    return 0;
+}

src/sys/uio.rs

@@ -0,0 +1,78 @@
+
+mod ffi {
+    use super::IoVec;
+    use libc::{ssize_t];
+
+    // vectorized version of write
+    // doc: http://man7.org/linux/man-pages/man2/writev.2.html
+    pub fn writev(fd: Fd, iov: *const IoVec<&[u8]>, iovcnt: c_int) -> ssize_t;
+
+    // vectorized version of read
+    // doc: http://man7.org/linux/man-pages/man2/readv.2.html
+    pub fn readv(fd: Fd, iov: *const IoVec<&mut [u8]>, iovcnt: c_int) -> ssize_t;
+}
+
+pub fn writev(fd: Fd, iov: &[IoVec<&[u8]>]) -> NixResult<usize> {
+    let res = unsafe { ffi::writev(fd, iov.as_ptr(), iov.len() as c_int) };
+
+    if res < 0 {
+        return Err(NixError::Sys(Errno::last()));
+    }
+
+    return Ok(res as usize)
+}
+
+pub fn readv(fd: Fd, iov: &mut [IoVec<&mut [u8]>]) -> NixResult<usize> {
+    let res = unsafe { ffi::readv(fd, iov.as_ptr(), iov.len() as c_int) };
+    if res < 0 {
+        return Err(NixError::Sys(Errno::last()));
+    }
+
+    return Ok(res as usize)
+}
+
+#[repr(C)]
+pub struct IoVec<T> {
+    iov_base: *mut c_void,
+    iov_len: size_t,
+    phantom: PhantomData<T>
+}
+
+impl<T> IoVec<T> {
+    #[inline]
+    pub fn as_slice<'a>(&'a self) -> &'a [u8] {
+        use std::slice;
+
+        unsafe {
+            slice::from_raw_parts(
+                self.iov_base as *const u8,
+                self.iov_len as usize)
+        }
+    }
+}
+
+impl<'a> IoVec<&'a [u8]> {
+    pub fn from_slice(buf: &'a [u8]) -> IoVec<&'a [u8]> {
+        IoVec {
+            iov_base: buf.as_ptr() as *mut c_void,
+            iov_len: buf.len() as size_t,
+            phantom: PhantomData
+        }
+    }
+}
+
+impl<'a> IoVec<&'a mut [u8]> {
+    pub fn from_mut_slice(buf: &'a mut [u8]) -> IoVec<&'a mut [u8]> {
+        IoVec {
+            iov_base: buf.as_ptr() as *mut c_void,
+            iov_len: buf.len() as size_t,
+            phantom: PhantomData
+        }
+    }
+}
+
+#[test]
+pub fn test_size_of_io_vec() {
+    use nixtest;
+    nixtest::assert_size_of::<IoVec<&[u8]>>("iovec");
+}

src/unistd.rs

@@ -1,23 +1,18 @@
-use std::{mem, ptr};
-use std::marker::PhantomData;
-use libc::{c_char, c_void, c_int, size_t, pid_t, off_t};
+use {NixError, NixResult, NixPath, from_ffi};
 use errno::Errno;
 use fcntl::{fcntl, Fd, OFlag, O_NONBLOCK, O_CLOEXEC, FD_CLOEXEC};
 use fcntl::FcntlArg::{F_SETFD, F_SETFL};
-use {NixError, NixResult, NixPath, from_ffi};
-
-use core::raw::Slice as RawSlice;
+use libc::{c_char, c_void, c_int, size_t, pid_t, off_t};
+use std::{mem, ptr};
 use std::ffi::CString;
 
 #[cfg(target_os = "linux")]
 pub use self::linux::*;
 
 mod ffi {
-    use super::{IovecR,IovecW};
-    use libc::{c_char, c_int, size_t, ssize_t};
+    use libc::{c_char, c_int, size_t};
     pub use libc::{close, read, write, pipe, ftruncate, unlink};
     pub use libc::funcs::posix88::unistd::fork;
-    use fcntl::Fd;
 
     extern {
         // duplicate a file descriptor
@@ -44,14 +39,6 @@ mod ffi {
         // gets the hostname
         // doc: http://man7.org/linux/man-pages/man2/gethostname.2.html
         pub fn sethostname(name: *const c_char, len: size_t) -> c_int;
-
-        // vectorized version of write
-        // doc: http://man7.org/linux/man-pages/man2/writev.2.html
-        pub fn writev(fd: Fd, iov: *const IovecW, iovcnt: c_int) -> ssize_t;
-
-        // vectorized version of read
-        // doc: http://man7.org/linux/man-pages/man2/readv.2.html
-        pub fn readv(fd: Fd, iov: *const IovecR, iovcnt: c_int) -> ssize_t;
     }
 }
 
@@ -91,61 +78,6 @@ pub fn fork() -> NixResult<Fork> {
     }
 }
 
-// We use phantom types to maintain memory safety.
-// If readv/writev were using simple &[Iovec] we could initialize
-// Iovec with immutable slice and then pass it to readv, overwriting content
-// we dont have write access to:
-// let mut v = Vec::new();
-// let iov = Iovec::from_slice(immutable_vec.as_slice());
-// v.push(iov);
-// let _:NixResult<usize> = readv(fd, v.as_slice());
-
-// We do not want <T> to appear in ffi functions, so we provide this aliases.
-type IovecR = Iovec<ToRead>;
-type IovecW = Iovec<ToWrite>;
-
-#[derive(Copy)]
-pub struct ToRead;
-#[derive(Copy)]
-pub struct ToWrite;
-
-#[repr(C)]
-pub struct Iovec<T> {
-    iov_base: *mut c_void,
-    iov_len: size_t,
-    phantom: PhantomData<T>
-}
-
-impl <T> Iovec<T> {
-    #[inline]
-    pub fn as_slice<'a>(&'a self) -> &'a [u8] {
-        unsafe { mem::transmute(RawSlice { data: self.iov_base as *const u8, len: self.iov_len as usize }) }
-    }
-}
-
-impl Iovec<ToWrite> {
-    #[inline]
-    pub fn from_slice(buf: &[u8]) -> Iovec<ToWrite> {
-        Iovec {
-            iov_base: buf.as_ptr() as *mut c_void,
-            iov_len: buf.len() as size_t,
-            phantom: PhantomData
-        }
-    }
-}
-
-impl Iovec<ToRead> {
-    #[inline]
-    pub fn from_mut_slice(buf: &mut [u8]) -> Iovec<ToRead> {
-        Iovec {
-            iov_base: buf.as_ptr() as *mut c_void,
-            iov_len: buf.len() as size_t,
-            phantom: PhantomData
-        }
-    }
-}
-
-
 #[inline]
 pub fn dup(oldfd: Fd) -> NixResult<Fd> {
     let res = unsafe { ffi::dup(oldfd) };
@@ -296,24 +228,6 @@ pub fn write(fd: Fd, buf: &[u8]) -> NixResult<usize> {
     return Ok(res as usize)
 }
 
-pub fn writev(fd: Fd, iov: &[Iovec<ToWrite>]) -> NixResult<usize> {
-    let res = unsafe { ffi::writev(fd, iov.as_ptr(), iov.len() as c_int) };
-    if res < 0 {
-        return Err(NixError::Sys(Errno::last()));
-    }
-
-    return Ok(res as usize)
-}
-
-pub fn readv(fd: Fd, iov: &mut [Iovec<ToRead>]) -> NixResult<usize> {
-    let res = unsafe { ffi::readv(fd, iov.as_ptr(), iov.len() as c_int) };
-    if res < 0 {
-        return Err(NixError::Sys(Errno::last()));
-    }
-
-    return Ok(res as usize)
-}
-
 pub fn pipe() -> NixResult<(Fd, Fd)> {
     unsafe {
         let mut res;

test/sys/mod.rs

@@ -0,0 +1 @@
+mod test_termios;

tests/sys/termios.rs renamed to test/sys/test_termios.rs

@@ -0,0 +1,89 @@
+use nix::sys::uio::*;
+
+#[test]
+fn test_writev() {
+    let mut to_write = Vec::with_capacity(16 * 128);
+    for _ in 0..16 {
+        let s: String = thread_rng().gen_ascii_chars().take(128).collect();
+        let b = s.as_bytes();
+        to_write.extend(b.iter().map(|x| x.clone()));
+    }
+    // Allocate and fill iovecs
+    let mut iovecs = Vec::new();
+    let mut consumed = 0;
+    while consumed < to_write.len() {
+        let left = to_write.len() - consumed;
+        let slice_len = if left < 64 { left } else { thread_rng().gen_range(64, min(256, left)) };
+        let b = &to_write[consumed..consumed+slice_len];
+        iovecs.push(IoVec::from_slice(b));
+        consumed += slice_len;
+    }
+    let pipe_res = pipe();
+    assert!(pipe_res.is_ok());
+    let (reader, writer) = pipe_res.ok().unwrap();
+    // FileDesc will close its filedesc (reader).
+    let mut read_buf: Vec<u8> = repeat(0u8).take(128 * 16).collect();
+    // Blocking io, should write all data.
+    let write_res = writev(writer, iovecs.as_slice());
+    // Successful write
+    assert!(write_res.is_ok());
+    let written = write_res.ok().unwrap();
+    // Check whether we written all data
+    assert_eq!(to_write.len(), written);
+    let read_res = read(reader, read_buf.as_mut_slice());
+    // Successful read
+    assert!(read_res.is_ok());
+    let read = read_res.ok().unwrap() as usize;
+    // Check we have read as much as we written
+    assert_eq!(read, written);
+    // Check equality of written and read data
+    assert_eq!(to_write.as_slice(), read_buf.as_slice());
+    let close_res = close(writer);
+    assert!(close_res.is_ok());
+    let close_res = close(reader);
+    assert!(close_res.is_ok());
+}
+
+#[test]
+fn test_readv() {
+    let s:String = thread_rng().gen_ascii_chars().take(128).collect();
+    let to_write = s.as_bytes().to_vec();
+    let mut storage = Vec::new();
+    let mut allocated = 0;
+    while allocated < to_write.len() {
+        let left = to_write.len() - allocated;
+        let vec_len = if left < 64 { left } else { thread_rng().gen_range(64, min(256, left)) };
+        let v: Vec<u8> = repeat(0u8).take(vec_len).collect();
+        storage.push(v);
+        allocated += vec_len;
+    }
+    let mut iovecs = Vec::with_capacity(storage.len());
+    for v in storage.iter_mut() {
+        iovecs.push(IoVec::from_mut_slice(v.as_mut_slice()));
+    }
+    let pipe_res = pipe();
+    assert!(pipe_res.is_ok());
+    let (reader, writer) = pipe_res.ok().unwrap();
+    // Blocking io, should write all data.
+    let write_res = write(writer, to_write.as_slice());
+    // Successful write
+    assert!(write_res.is_ok());
+    let read_res = readv(reader, iovecs.as_mut_slice());
+    assert!(read_res.is_ok());
+    let read = read_res.ok().unwrap();
+    // Check whether we've read all data
+    assert_eq!(to_write.len(), read);
+    // Cccumulate data from iovecs
+    let mut read_buf = Vec::with_capacity(to_write.len());
+    for iovec in iovecs.iter() {
+        read_buf.extend(iovec.as_slice().iter().map(|x| x.clone()));
+    }
+    // Check whether iovecs contain all written data
+    assert_eq!(read_buf.len(), to_write.len());
+    // Check equality of written and read data
+    assert_eq!(read_buf.as_slice(), to_write.as_slice());
+    let close_res = close(reader);
+    assert!(close_res.is_ok());
+    let close_res = close(writer);
+    assert!(close_res.is_ok());
+}