buffered-reader: Fix Generic::data_helper.

  - The `Generic::data_helper` was not sufficiently careful when
    dealing with errors.  In particular:

    - If `Generic::data_hard(1)` was called, and an error occured, and
      nothing was buffered or read, `Generic::data_helper` would
      return the empty string instead of the error.

    - If `Generic::data(n)` was called, and an error occured, but some
      data (< n bytes) was buffered, `Generic::data_helper` would
      return the error instead of the data that was read.

  - Fix these bugs.

  - Also, simplify the code and don't save whether we hit EOF or the
    error (which we can only return once, anyway).  Instead, rely on
    the underlying reader to return EOF or the error again.

  - Fixes #174.

2 files changed, 31 insertions, 52 deletions

diff --git a/buffered-reader/src/generic.rs b/buffered-reader/src/generic.rs
index 67a3eeee..de1a52c7 100644
--- a/buffered-reader/src/generic.rs
+++ b/buffered-reader/src/generic.rs
@@ -18,10 +18,6 @@ pub struct Generic<T: io::Read, C> {
     // XXX: This is pub for the decompressors.  It would be better to
     // change this to some accessor method.
     pub reader: Box<T>,
-    // Whether we saw an EOF.
-    saw_eof: bool,
-    // The last error that we encountered, but have not yet returned.
-    error: Option<io::Error>,
 
     // The user settable cookie.
     cookie: C,
@@ -44,8 +40,6 @@ impl<T: io::Read, C> fmt::Debug for Generic<T, C> {
         f.debug_struct("Generic")
             .field("preferred_chunk_size", &self.preferred_chunk_size)
             .field("buffer data", &buffered_data)
-            .field("saw eof", &self.saw_eof)
-            .field("error", &self.error)
             .finish()
     }
 }
@@ -74,8 +68,6 @@ impl<T: io::Read, C> Generic<T, C> {
                 if let Some(s) = preferred_chunk_size { s }
                 else { DEFAULT_BUF_SIZE },
             reader: Box::new(reader),
-            saw_eof: false,
-            error: None,
             cookie: cookie,
         }
     }
@@ -100,10 +92,11 @@ impl<T: io::Read, C> Generic<T, C> {
             assert_eq!(self.cursor, 0);
         }
 
-        let amount_buffered =
-            if let Some(ref buffer) = self.buffer { buffer.len() } else { 0 }
-            - self.cursor;
-        if !self.saw_eof && amount > amount_buffered {
+        let mut error = None;
+
+        let amount_buffered
+            = self.buffer.as_ref().map(|b| b.len() - self.cursor).unwrap_or(0);
+        if amount > amount_buffered {
             // The caller wants more data than we have readily
             // available.  Read some more.
 
@@ -119,8 +112,6 @@ impl<T: io::Read, C> Generic<T, C> {
                                        [amount_buffered + amount_read..]) {
                     Ok(read) => {
                         if read == 0 {
-                            // XXX: Likely EOF.
-                            self.saw_eof = true;
                             break;
                         } else {
                             amount_read += read;
@@ -132,8 +123,7 @@ impl<T: io::Read, C> Generic<T, C> {
                     Err(err) => {
                         // Don't return yet, because we may have
                         // actually read something.
-                        self.saw_eof = true;
-                        self.error = Some(err);
+                        error = Some(err);
                         break;
                     },
                 }
@@ -157,42 +147,33 @@ impl<T: io::Read, C> Generic<T, C> {
             }
         }
 
-        if self.error.is_some() {
-            // An error occurred.  If we have enough data to fulfill
-            // the caller's request, then delay returning the error.
-            if let Some(ref buffer) = self.buffer {
-                if amount > buffer.len() {
-                    // We return an error at most once (Recall: take
-                    // clears self.error).
-                    return Err(self.error.take().unwrap());
-                }
+        let amount_buffered
+            = self.buffer.as_ref().map(|b| b.len() - self.cursor).unwrap_or(0);
+
+        if let Some(error) = error {
+            // An error occured.  If we have enough data to fulfill
+            // the caller's request, then don't return the error.
+            if hard && amount > amount_buffered {
+                return Err(error);
+            }
+            if !hard && amount_buffered == 0 {
+                return Err(error);
             }
         }
 
-        match self.buffer {
-            Some(ref buffer) => {
-                let amount_buffered = buffer.len() - self.cursor;
-                if hard && amount_buffered < amount {
-                    return Err(Error::new(ErrorKind::UnexpectedEof, "EOF"));
-                }
-                if and_consume {
-                    let amount_consumed = cmp::min(amount_buffered, amount);
-                    self.cursor += amount_consumed;
-                    assert!(self.cursor <= buffer.len());
-                    return Ok(&buffer[self.cursor-amount_consumed..]);
-                } else {
-                    return Ok(&buffer[self.cursor..]);
-                }
-            },
-            None if self.saw_eof => {
-                if hard && amount > 0 {
-                    Err(Error::new(ErrorKind::UnexpectedEof, "EOF"))
-                } else {
-                    Ok(&b""[..])
-                }
-            },
-            None => {
-                unreachable!();
+        if hard && amount_buffered < amount {
+            Err(Error::new(ErrorKind::UnexpectedEof, "EOF"))
+        } else if amount == 0 || amount_buffered == 0 {
+            Ok(&b""[..])
+        } else {
+            let buffer = self.buffer.as_ref().unwrap();
+            if and_consume {
+                let amount_consumed = cmp::min(amount_buffered, amount);
+                self.cursor += amount_consumed;
+                assert!(self.cursor <= buffer.len());
+                Ok(&buffer[self.cursor-amount_consumed..])
+            } else {
+                Ok(&buffer[self.cursor..])
             }
         }
     }
diff --git a/buffered-reader/src/lib.rs b/buffered-reader/src/lib.rs
index 287ab63f..129eaa9c 100644
--- a/buffered-reader/src/lib.rs
+++ b/buffered-reader/src/lib.rs
@@ -307,9 +307,7 @@ pub trait BufferedReader<C> : io::Read + fmt::Debug + fmt::Display {
     /// EOF has been reached or an error occurs, in which case the
     /// returned slice will contain the rest of the file.
     ///
-    /// If an error occurs, it is not discarded, but saved.  It is
-    /// returned when `data` (or a related function) is called and the
-    /// internal buffer is empty.
+    /// Errors are returned only when the internal buffer is empty.
     ///
     /// This function does not advance the cursor.  To advance the
     /// cursor, use `consume()`.