Moved raw fd holding into a seperate inner structure to handle fd closing and ownership.

Fixed returning `(rx, tx)` instead of `(tx, rx)` from `unix_pipe()`.

Fortune for comfork's current commit: Small curse − 小凶

src/sys/pipe.rs

@@ -18,36 +18,110 @@ pub enum ErrorKind
     Unknown,
 }
 
-/// A raw unix pipe
-// TODO: Change uses of `i32` for socket fds. Or change this to a struct (see below.)
-pub type RawPipe = i32;
+/// A raw unix pipe.
+///
+/// Encapsulates a raw fd, proventing arbitrary operations on it while it is owned by the instance.
+/// The fd is closed when this instance is dropped.
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[repr(transparent)]
+pub struct RawPipe(i32);
+
+impl io::Read for RawPipe
+{
+    #[inline] fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+	Ok(read_raw(self.0, buf)?)
+    }
+}
+impl io::Write for RawPipe
+{
+    #[inline] fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+	Ok(write_raw(self.0, buf)?)
+    }
+    #[inline] fn flush(&mut self) -> io::Result<()> {
+	Ok(())
+    }
+}
+
+impl From<RawPipe> for i32
+{
+    #[inline] fn from(from: RawPipe) -> Self
+    {
+	from.take()
+    }
+}
+
+impl RawPipe
+{
+    /// Transfer ownership of the internal fd to the caller
+    #[inline] pub fn take(self) -> i32
+    {
+	let i = self.0;
+	std::mem::forget(self);
+	i
+    }
+
+    /// Transfer ownership of an fd into a new managed instance.
+    ///
+    /// # Safety
+    /// The caller should ensure the fd is valid, or all operations on the instance returned will fail.
+    #[inline] pub const unsafe fn new_unchecked(int: i32) -> Self
+    {
+	Self(int)
+    }
+
+    /// Get a raw handle to the fd
+    ///
+    /// # Safety
+    /// This operation is unsafe as the caller could close the fd while its still owned by the instance.
+    /// If you want to transfer ownership of the fd to yourself, use `take()` or `Into::into()`.
+    #[inline] pub const unsafe fn as_raw_fd(&self) -> i32
+    {
+	self.0
+    }
+    
+    /// Transfer ownership of an fd into a new managed instance.
+    ///
+    /// # Panics
+    /// If the fd value was below 0.
+    pub fn new(int: i32) -> Self
+    {
+	assert!(int > 0, "invalid fd");
+	Self(int)
+    }
+}
 
-/// Create a raw pipe pair.
-pub(crate) fn unix_pipe() -> Result<(i32, i32), Error>
+impl Drop for RawPipe
+{
+    fn drop(&mut self) {
+	unsafe {
+	    libc::close(self.0);
+	}
+    }
+}
+
+/// Create a raw pipe pair. Returns `(tx, rx)`: writer then reader.
+pub(crate) fn unix_pipe() -> Result<(RawPipe, RawPipe), Error>
 {
     use libc::pipe;
     let mut pipe_fds: [libc::c_int; 2] = [-1;2];
     if unsafe{pipe(pipe_fds.as_mut_ptr())} == 0 {
-	Ok((i32::from(pipe_fds[0]), i32::from(pipe_fds[1])))
+	Ok((RawPipe(i32::from(pipe_fds[1])), RawPipe(i32::from(pipe_fds[0]))))
     } else {
 	Err(ErrorKind::Create.into_error())
     }
 }
 
-//TODO: Factor out the `i32` raw sockets in these types below into a seperate `RawPipe` struct, which has the Drop impl and has functions for transfering ownership of the raw socket (it should hide the raw fd when not explicitly needed ideally.)
-//      This will prevent us from having to ensure we `forget()` the outer type each time we want to move the inner one around without closing it.
-
 /// A writer for a bi-directinoal unix pipe
 ///
 /// Created by splitting `Pipe`, data written to this is available to be read from its `ReadHalf` counterpart.
 #[derive(Debug, PartialEq, Eq)]
-pub struct WriteHalf(i32);
+pub struct WriteHalf(RawPipe);
 
 /// A reader for a bi-directional unix pipe.
 ///
 /// Created by splitting `Pipe`, data read from this is data that was sent to its `WriteHalf` counterpart.
 #[derive(Debug, PartialEq, Eq)]
-pub struct ReadHalf(i32);
+pub struct ReadHalf(RawPipe);
 
 /// A bi-drectional unix pipe
 ///
@@ -55,11 +129,10 @@ pub struct ReadHalf(i32);
 #[derive(Debug, PartialEq, Eq)]
 pub struct Pipe
 {
-    tx: i32,
-    rx: i32,
+    tx: RawPipe,
+    rx: RawPipe,
 }
 
-// Make sure when operating on consumers of Pipe of Read/WriteHalf, the pipe structures are `mem::forget`ed before returning, since the Drop impl of those types closes the raw socket.
 impl Pipe
 {
     /// Split this pipe into a read and write half.
@@ -67,9 +140,7 @@ impl Pipe
     /// Data written to the `WriteHalf` is sent to the `ReadHalf` (unless the pipe is mismatched.)
     #[inline] pub fn split(self) -> (WriteHalf, ReadHalf)
     {
-	let rv = (WriteHalf(self.tx), ReadHalf(self.rx));	
-	std::mem::forget(self);
-	rv
+	(WriteHalf(self.tx), ReadHalf(self.rx))
     }
 
     /// Create a `Pipe` from two halves.
@@ -79,16 +150,14 @@ impl Pipe
     /// If they are not, then writing to the `Pipe` will send the data to its original receiver, and reading from it will take the data from its original sender.
     #[inline] pub fn unsplit(tx: WriteHalf, rx: ReadHalf) -> Self
     {
-	let v = Self::new_from_raw((tx.0, rx.0));
-	std::mem::forget((tx, rx));
-	v
+	Self::new_from_raw((tx.0, rx.0))
     }
 
     /// Create a new `Pipe` from a pair of raw file descriptors.
     ///
     /// # Safety
     /// Does not check that the integers provided are valid and open file descriptors.
-    #[inline(always)] fn new_from_raw((tx, rx): (i32, i32)) -> Self
+    #[inline(always)] fn new_from_raw((tx, rx): (RawPipe, RawPipe)) -> Self
     {
 	Self{tx,rx}
     }
@@ -98,17 +167,15 @@ impl Pipe
     /// # Safety
     /// The caller must check that `tx` and `rx` are valid, open file descriptors
     // TODO: When writing the async version make sure to state that the fds need to be non-blocking.
-    #[inline] pub unsafe fn from_raw(tx: i32, rx: i32) -> Self
+    #[inline] pub unsafe fn from_raw(tx: RawPipe, rx: RawPipe) -> Self
     {
 	Self::new_from_raw((tx, rx))
     }
 
     /// Transfer the ownership of the socket's inner Tx and Rx fds.
-    pub fn into_raw(self) -> (i32, i32)
+    pub fn into_raw(self) -> (RawPipe, RawPipe)
     {
-	let txrx = (self.tx, self.rx);
-	std::mem::forget(self);
-	txrx
+	(self.tx, self.rx)
     }
 
     /// Create a new `Pipe` from two new file descriptors that stream to eachother.
@@ -163,59 +230,32 @@ fn read_raw(sock: i32, data: &mut [u8]) -> Result<usize, Errno>
 impl io::Read for Pipe
 {
     #[inline] fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-	Ok(read_raw(self.rx, buf)?)
+	self.rx.read(buf)
     }
 }
 impl io::Write for Pipe
 {
     #[inline] fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-	Ok(write_raw(self.tx, buf)?)
+	self.tx.write(buf)
     }
     #[inline] fn flush(&mut self) -> io::Result<()> {
-	Ok(())
+	self.tx.flush()
     }
 }
 
 impl io::Read for ReadHalf
 {
     #[inline] fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-	Ok(read_raw(self.0, buf)?)
+	self.0.read(buf)
     }
 }
 impl io::Write for WriteHalf
 {
     #[inline] fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-	Ok(write_raw(self.0, buf)?)
+	self.0.write(buf)
     }
     #[inline] fn flush(&mut self) -> io::Result<()> {
-	Ok(())
-    }
-}
-
-
-impl Drop for ReadHalf
-{
-    fn drop(&mut self) {
-	unsafe {
-	    libc::close(self.0);
-	}
-    }
-}
-impl Drop for WriteHalf
-{
-    fn drop(&mut self) {
-	unsafe {
-	    libc::close(self.0);
-	}
-    }
-}
-impl Drop for Pipe
-{
-    fn drop(&mut self) {
-	unsafe {
-	    libc::close(self.tx);
-	    libc::close(self.rx);
-	}
+	self.0.flush()
     }
 }
 
@@ -323,3 +363,19 @@ impl fmt::Display for Error
     }
 }
 
+#[cfg(test)]
+mod tests
+{
+    use std::io::{Read, Write};
+    #[test]
+    fn pipe_single_rw()
+    {
+	const DATA: &'static [u8] = b"hello world";
+	let mut pipe = super::Pipe::new().expect("Creation");
+	print!("Pipe: {:?}", pipe);
+	pipe.write_all(DATA).expect("write (ex)");
+	let mut buffer = [0u8; DATA.len()];
+	pipe.read_exact(&mut buffer[..]).expect("read (ex)");
+	assert_eq!(&buffer, DATA, "data different");
+    }
+}