diff --git a/AK/BitStream.h b/AK/BitStream.h index bd8fe28b41..fbb26d2b5c 100644 --- a/AK/BitStream.h +++ b/AK/BitStream.h @@ -66,7 +66,7 @@ public: return true; } - bool eof() const override { return !m_next_byte.has_value() && m_stream.eof(); } + bool unreliable_eof() const override { return !m_next_byte.has_value() && m_stream.unreliable_eof(); } bool discard_or_error(size_t count) override { @@ -86,6 +86,11 @@ public: size_t nread = 0; while (nread < count) { + if (m_stream.has_any_error()) { + set_fatal_error(); + return 0; + } + if (m_next_byte.has_value()) { const auto bit = (m_next_byte.value() >> m_bit_offset) & 1; result |= bit << nread; @@ -93,9 +98,6 @@ public: if (m_bit_offset++ == 7) m_next_byte.clear(); - } else if (m_stream.eof()) { - set_fatal_error(); - return 0; } else { m_stream >> m_next_byte; m_bit_offset = 0; diff --git a/AK/Buffered.h b/AK/Buffered.h index e6f72a7d98..058ecd6649 100644 --- a/AK/Buffered.h +++ b/AK/Buffered.h @@ -77,7 +77,7 @@ public: return nread; } - virtual bool read_or_error(Bytes bytes) override + bool read_or_error(Bytes bytes) override { if (read(bytes) < bytes.size()) { set_fatal_error(); @@ -87,7 +87,9 @@ public: return true; } - virtual bool eof() const + bool unreliable_eof() const override { return m_buffer_remaining == 0 && m_stream.unreliable_eof(); } + + bool eof() const { if (m_buffer_remaining > 0) return false; @@ -97,7 +99,7 @@ public: return m_buffer_remaining == 0; } - virtual bool discard_or_error(size_t count) override + bool discard_or_error(size_t count) override { size_t ndiscarded = 0; while (ndiscarded < count) { diff --git a/AK/CircularDuplexStream.h b/AK/CircularDuplexStream.h index 36e19461ef..0005d7bf4e 100644 --- a/AK/CircularDuplexStream.h +++ b/AK/CircularDuplexStream.h @@ -112,7 +112,8 @@ public: return true; } - bool eof() const override { return m_queue.size() == 0; } + bool unreliable_eof() const override { return eof(); } + bool eof() const { return m_queue.size() == 0; } size_t remaining_contigous_space() const { diff --git a/AK/MemoryStream.h b/AK/MemoryStream.h index b58f725855..8a8bcd6196 100644 --- a/AK/MemoryStream.h +++ b/AK/MemoryStream.h @@ -40,7 +40,8 @@ public: { } - bool eof() const override { return m_offset >= m_bytes.size(); } + bool unreliable_eof() const override { return eof(); } + bool eof() const { return m_offset >= m_bytes.size(); } size_t read(Bytes bytes) override { @@ -167,7 +168,8 @@ class DuplexMemoryStream final : public DuplexStream { public: static constexpr size_t chunk_size = 4 * 1024; - bool eof() const override { return m_write_offset == m_read_offset; } + bool unreliable_eof() const override { return eof(); } + bool eof() const { return m_write_offset == m_read_offset; } bool discard_or_error(size_t count) override { diff --git a/AK/Stream.h b/AK/Stream.h index d301c15052..11d286acd6 100644 --- a/AK/Stream.h +++ b/AK/Stream.h @@ -75,10 +75,22 @@ namespace AK { class InputStream : public virtual AK::Detail::Stream { public: - // Does nothing and returns zero if there is already an error. + // Reads at least one byte unless none are requested or none are avaliable. Does nothing + // and returns zero if there is already an error. virtual size_t read(Bytes) = 0; + + // If this function returns true, then no more data can be read. If read(Bytes) previously + // returned zero even though bytes were requested, then the inverse is true as well. + virtual bool unreliable_eof() const = 0; + + // Some streams additionally define a method with the signature: + // + // bool eof() const; + // + // This method has the same semantics as unreliable_eof() but returns true if and only if no + // more data can be read. (A failed read is not necessary.) + virtual bool read_or_error(Bytes) = 0; - virtual bool eof() const = 0; virtual bool discard_or_error(size_t count) = 0; }; diff --git a/AK/String.h b/AK/String.h index f4470fe98b..deed272724 100644 --- a/AK/String.h +++ b/AK/String.h @@ -285,16 +285,15 @@ inline InputStream& operator>>(InputStream& stream, String& string) StringBuilder builder; for (;;) { - if (stream.eof()) { - string = nullptr; - - stream.set_fatal_error(); - return stream; - } - char next_char; stream >> next_char; + if (stream.has_any_error()) { + stream.set_fatal_error(); + string = nullptr; + return stream; + } + if (next_char) { builder.append(next_char); } else { 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 DeflateDecompressor::decompress_all(ReadonlyBytes bytes) { @@ -310,7 +310,7 @@ Optional 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 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 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 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 decompress_all(ReadonlyBytes); @@ -87,6 +88,8 @@ private: InputStream& m_input_stream; Optional m_current_member; + + bool m_eof { false }; }; } diff --git a/Libraries/LibCore/FileStream.h b/Libraries/LibCore/FileStream.h index 3b6f5efd68..0a67f983ff 100644 --- a/Libraries/LibCore/FileStream.h +++ b/Libraries/LibCore/FileStream.h @@ -68,25 +68,8 @@ public: if (has_any_error()) return 0; - auto nread = m_buffered.bytes().copy_trimmed_to(bytes); - - m_buffered.bytes().slice(nread, m_buffered.size() - nread).copy_to(m_buffered); - m_buffered.trim(m_buffered.size() - nread); - - while (nread < bytes.size()) { - if (m_file->eof()) - return nread; - - if (m_file->has_error()) { - set_fatal_error(); - return 0; - } - - const auto buffer = m_file->read(bytes.size() - nread); - nread += buffer.bytes().copy_to(bytes.slice(nread)); - } - - return nread; + const auto buffer = m_file->read(bytes.size()); + return buffer.bytes().copy_to(bytes); } bool read_or_error(Bytes bytes) override @@ -99,34 +82,9 @@ public: return true; } - bool discard_or_error(size_t count) override - { - u8 buffer[4096]; + bool discard_or_error(size_t count) override { return m_file->seek(count, IODevice::SeekMode::FromCurrentPosition); } - size_t ndiscarded = 0; - while (ndiscarded < count && !eof()) - ndiscarded += read({ buffer, min(count - ndiscarded, sizeof(buffer)) }); - - if (eof()) { - set_fatal_error(); - return false; - } - - return true; - } - - bool eof() const override - { - if (m_buffered.size() > 0) - return false; - - if (m_file->eof()) - return true; - - m_buffered = m_file->read(4096); - - return m_buffered.size() == 0; - } + bool unreliable_eof() const override { return m_file->eof(); } void close() { @@ -137,8 +95,7 @@ public: private: InputFileStream() = default; - mutable NonnullRefPtr m_file; - mutable ByteBuffer m_buffered; + NonnullRefPtr m_file; }; class OutputFileStream : public OutputStream { diff --git a/Userland/gunzip.cpp b/Userland/gunzip.cpp index 5a344c8d41..cf0d473850 100644 --- a/Userland/gunzip.cpp +++ b/Userland/gunzip.cpp @@ -34,7 +34,7 @@ static void decompress_file(Buffered& input_stream, Buffe u8 buffer[4096]; - while (!gzip_stream.eof()) { + while (!gzip_stream.unreliable_eof()) { const auto nread = gzip_stream.read({ buffer, sizeof(buffer) }); output_stream.write_or_error({ buffer, nread }); }