AK: Lower the requirements for InputStream::eof and rename it.

Consider the following snippet:

    void foo(InputStream& stream) {
        if(!stream.eof()) {
            u8 byte;
            stream >> byte;
        }
    }

There is a very subtle bug in this snippet, for some input streams eof()
might return false even if no more data can be read. In this case an
error flag would be set on the stream.

Until now I've always ensured that this is not the case, but this made
the implementation of eof() unnecessarily complicated.
InputFileStream::eof had to keep a ByteBuffer around just to make this
possible. That meant a ton of unnecessary copies just to get a reliable
eof().

In most cases it isn't actually necessary to have a reliable eof()
implementation.

In most other cases a reliable eof() is avaliable anyways because in
some cases like InputMemoryStream it is very easy to implement.

4 files changed, 18 insertions, 21 deletions

diff --git a/Libraries/LibCompress/Deflate.cpp b/Libraries/LibCompress/Deflate.cpp
index 791a54b20c..dcd058b69c 100644
--- a/Libraries/LibCompress/Deflate.cpp
+++ b/Libraries/LibCompress/Deflate.cpp
@@ -290,7 +290,7 @@ bool DeflateDecompressor::discard_or_error(size_t count)
 
     size_t ndiscarded = 0;
     while (ndiscarded < count) {
-        if (eof()) {
+        if (unreliable_eof()) {
             set_fatal_error();
             return false;
         }
@@ -301,7 +301,7 @@ bool DeflateDecompressor::discard_or_error(size_t count)
     return true;
 }
 
-bool DeflateDecompressor::eof() const { return m_state == State::Idle && m_read_final_bock; }
+bool DeflateDecompressor::unreliable_eof() const { return m_state == State::Idle && m_read_final_bock; }
 
 Optional<ByteBuffer> DeflateDecompressor::decompress_all(ReadonlyBytes bytes)
 {
@@ -310,7 +310,7 @@ Optional<ByteBuffer> DeflateDecompressor::decompress_all(ReadonlyBytes bytes)
     OutputMemoryStream output_stream;
 
     u8 buffer[4096];
-    while (!deflate_stream.has_any_error() && !deflate_stream.eof()) {
+    while (!deflate_stream.has_any_error() && !deflate_stream.unreliable_eof()) {
         const auto nread = deflate_stream.read({ buffer, sizeof(buffer) });
         output_stream.write_or_error({ buffer, nread });
     }
diff --git a/Libraries/LibCompress/Deflate.h b/Libraries/LibCompress/Deflate.h
index 7474a03ca7..c069ba6951 100644
--- a/Libraries/LibCompress/Deflate.h
+++ b/Libraries/LibCompress/Deflate.h
@@ -92,7 +92,8 @@ public:
     size_t read(Bytes) override;
     bool read_or_error(Bytes) override;
     bool discard_or_error(size_t) override;
-    bool eof() const override;
+
+    bool unreliable_eof() const override;
 
     static Optional<ByteBuffer> decompress_all(ReadonlyBytes);
 
diff --git a/Libraries/LibCompress/Gzip.cpp b/Libraries/LibCompress/Gzip.cpp
index 12df241224..de19a0ff3d 100644
--- a/Libraries/LibCompress/Gzip.cpp
+++ b/Libraries/LibCompress/Gzip.cpp
@@ -68,7 +68,7 @@ GzipDecompressor::~GzipDecompressor()
 // FIXME: Again, there are surely a ton of bugs because the code doesn't check for read errors.
 size_t GzipDecompressor::read(Bytes bytes)
 {
-    if (has_any_error())
+    if (has_any_error() || m_eof)
         return 0;
 
     if (m_current_member.has_value()) {
@@ -99,12 +99,14 @@ size_t GzipDecompressor::read(Bytes bytes)
 
         return nread;
     } else {
-        if (m_input_stream.eof())
-            return 0;
-
         BlockHeader header;
         m_input_stream >> Bytes { &header, sizeof(header) };
 
+        if (m_input_stream.handle_any_error()) {
+            m_eof = true;
+            return 0;
+        }
+
         if (!header.valid_magic_number() || !header.supported_by_implementation()) {
             set_fatal_error();
             return 0;
@@ -147,7 +149,7 @@ bool GzipDecompressor::discard_or_error(size_t count)
 
     size_t ndiscarded = 0;
     while (ndiscarded < count) {
-        if (eof()) {
+        if (unreliable_eof()) {
             set_fatal_error();
             return false;
         }
@@ -165,7 +167,7 @@ Optional<ByteBuffer> GzipDecompressor::decompress_all(ReadonlyBytes bytes)
     OutputMemoryStream output_stream;
 
     u8 buffer[4096];
-    while (!gzip_stream.has_any_error() && !gzip_stream.eof()) {
+    while (!gzip_stream.has_any_error() && !gzip_stream.unreliable_eof()) {
         const auto nread = gzip_stream.read({ buffer, sizeof(buffer) });
         output_stream.write_or_error({ buffer, nread });
     }
@@ -176,15 +178,6 @@ Optional<ByteBuffer> GzipDecompressor::decompress_all(ReadonlyBytes bytes)
     return output_stream.copy_into_contiguous_buffer();
 }
 
-bool GzipDecompressor::eof() const
-{
-    if (m_current_member.has_value()) {
-        // FIXME: There is an ugly edge case where we read the whole deflate block
-        //        but haven't read CRC32 and ISIZE.
-        return current_member().m_stream.eof() && m_input_stream.eof();
-    } else {
-        return m_input_stream.eof();
-    }
-}
+bool GzipDecompressor::unreliable_eof() const { return m_eof; }
 
 }
diff --git a/Libraries/LibCompress/Gzip.h b/Libraries/LibCompress/Gzip.h
index 1c1ea4ad7f..26e73b22e5 100644
--- a/Libraries/LibCompress/Gzip.h
+++ b/Libraries/LibCompress/Gzip.h
@@ -39,7 +39,8 @@ public:
     size_t read(Bytes) override;
     bool read_or_error(Bytes) override;
     bool discard_or_error(size_t) override;
-    bool eof() const override;
+
+    bool unreliable_eof() const override;
 
     static Optional<ByteBuffer> decompress_all(ReadonlyBytes);
 
@@ -87,6 +88,8 @@ private:
 
     InputStream& m_input_stream;
     Optional<Member> m_current_member;
+
+    bool m_eof { false };
 };
 
 }