From ef3caec22618b0e82b846df3a98b12cdfad3023d Mon Sep 17 00:00:00 2001
From: Alan Somers <asomers@gmail.com>
Date: Sun, 30 Jul 2017 21:40:46 -0600
Subject: [PATCH] Fixed error handling in `AioCb::fsync`, `AioCb::read`, and
 `AioCb::write`

Previously, the `AioCb`'s `in_progress` field would erroneously be set
to `true`, even if the syscall had an error

Fixes #714
---
 CHANGELOG.md         |  3 +++
 src/sys/aio.rs       | 21 +++++++++++++------
 test/sys/test_aio.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 98227b319a..1020ec41d4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Renamed existing `ptrace` wrappers to encourage namespacing ([#692](https://github.com/nix-rust/nix/pull/692))
 - `AioCb::Drop` will now panic if the `AioCb` is still in-progress ([#XXX](https://github.com/nix-rust/nix/pull/XXX))
 
+### Fixed
+- Fixed error handling in `AioCb::fsync`, `AioCb::read`, and `AioCb::write` ([#XXX](https://github.com/nix-rust/nix/pull/XXX))
+
 ## [0.9.0] 2017-07-23
 
 ### Added
diff --git a/src/sys/aio.rs b/src/sys/aio.rs
index 5d56f1cdf3..22bd3959ec 100644
--- a/src/sys/aio.rs
+++ b/src/sys/aio.rs
@@ -232,16 +232,22 @@ impl<'a> AioCb<'a> {
     /// An asynchronous version of `fsync`.
     pub fn fsync(&mut self, mode: AioFsyncMode) -> Result<()> {
         let p: *mut libc::aiocb = &mut self.aiocb;
-        self.in_progress = true;
-        Errno::result(unsafe { libc::aio_fsync(mode as libc::c_int, p) }).map(drop)
+        Errno::result(unsafe {
+                libc::aio_fsync(mode as libc::c_int, p)
+        }).map(|_| {
+            self.in_progress = true;
+        })
     }
 
     /// Asynchronously reads from a file descriptor into a buffer
     pub fn read(&mut self) -> Result<()> {
         assert!(self.mutable, "Can't read into an immutable buffer");
         let p: *mut libc::aiocb = &mut self.aiocb;
-        self.in_progress = true;
-        Errno::result(unsafe { libc::aio_read(p) }).map(drop)
+        Errno::result(unsafe {
+            libc::aio_read(p)
+        }).map(|_| {
+            self.in_progress = true;
+        })
     }
 
     /// Retrieve return status of an asynchronous operation.  Should only be
@@ -257,8 +263,11 @@ impl<'a> AioCb<'a> {
     /// Asynchronously writes from a buffer to a file descriptor
     pub fn write(&mut self) -> Result<()> {
         let p: *mut libc::aiocb = &mut self.aiocb;
-        self.in_progress = true;
-        Errno::result(unsafe { libc::aio_write(p) }).map(drop)
+        Errno::result(unsafe {
+            libc::aio_write(p)
+        }).map(|_| {
+            self.in_progress = true;
+        })
     }
 
 }
diff --git a/test/sys/test_aio.rs b/test/sys/test_aio.rs
index 441a6d8b90..9763e50d01 100644
--- a/test/sys/test_aio.rs
+++ b/test/sys/test_aio.rs
@@ -5,6 +5,7 @@ use nix::sys::aio::*;
 use nix::sys::signal::*;
 use nix::sys::time::{TimeSpec, TimeValLike};
 use std::io::{Write, Read, Seek, SeekFrom};
+use std::mem;
 use std::ops::Deref;
 use std::os::unix::io::AsRawFd;
 use std::rc::Rc;
@@ -88,6 +89,22 @@ fn test_fsync() {
     aiocb.aio_return().unwrap();
 }
 
+/// `AioCb::fsync` should not modify the `AioCb` object if libc::aio_fsync returns
+/// an error
+#[test]
+#[cfg_attr(all(target_env = "musl", target_arch = "x86_64"), ignore)]
+fn test_fsync_error() {
+    const INITIAL: &'static [u8] = b"abcdef123456";
+    // Create an invalid AioFsyncMode
+    let mode = unsafe { mem::transmute(666) };
+    let mut f = tempfile().unwrap();
+    f.write(INITIAL).unwrap();
+    let mut aiocb = AioCb::from_fd( f.as_raw_fd(),
+                            0,   //priority
+                            SigevNotify::SigevNone);
+    let err = aiocb.fsync(mode);
+    assert!(err.is_err());
+}
 
 #[test]
 #[cfg_attr(all(target_env = "musl", target_arch = "x86_64"), ignore)]
@@ -156,6 +173,24 @@ fn test_read() {
     assert!(EXPECT == rbuf.deref().deref());
 }
 
+/// `AioCb::read` should not modify the `AioCb` object if libc::aio_read returns
+/// an error
+#[test]
+#[cfg_attr(all(target_env = "musl", target_arch = "x86_64"), ignore)]
+fn test_read_error() {
+    const INITIAL: &'static [u8] = b"abcdef123456";
+    let rbuf = Rc::new(vec![0; 4].into_boxed_slice());
+    let mut f = tempfile().unwrap();
+    f.write(INITIAL).unwrap();
+    let mut aiocb = AioCb::from_boxed_slice( f.as_raw_fd(),
+                           -1,   //an invalid offset
+                           rbuf.clone(),
+                           0,   //priority
+                           SigevNotify::SigevNone,
+                           LioOpcode::LIO_NOP);
+    assert!(aiocb.read().is_err());
+}
+
 // Tests from_mut_slice
 #[test]
 #[cfg_attr(all(target_env = "musl", target_arch = "x86_64"), ignore)]
@@ -230,6 +265,21 @@ fn test_write() {
     assert!(rbuf == EXPECT);
 }
 
+/// `AioCb::write` should not modify the `AioCb` object if libc::aio_write returns
+/// an error
+#[test]
+#[cfg_attr(all(target_env = "musl", target_arch = "x86_64"), ignore)]
+fn test_write_error() {
+    let wbuf = "CDEF".to_string().into_bytes();
+    let mut aiocb = AioCb::from_slice( 666, // An invalid file descriptor
+                           0,   //offset
+                           &wbuf,
+                           0,   //priority
+                           SigevNotify::SigevNone,
+                           LioOpcode::LIO_NOP);
+    assert!(aiocb.write().is_err());
+}
+
 lazy_static! {
     pub static ref SIGNALED: AtomicBool = AtomicBool::new(false);
 }