From daf181caa8b520636d561e56d73775e53e98c78d Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Tue, 17 Jan 2023 14:52:46 +0100 Subject: [PATCH] LibCore: Let offset-related Stream functions return an unsigned value A negative return value doesn't make sense for any of those functions. The return types were inherited from POSIX, where they also need to have an indicator for an error (negative values). --- Tests/LibCore/TestLibCoreStream.cpp | 40 ++++++++++----------- Userland/Libraries/LibCore/MemoryStream.cpp | 4 +-- Userland/Libraries/LibCore/MemoryStream.h | 2 +- Userland/Libraries/LibCore/Stream.cpp | 8 ++--- Userland/Libraries/LibCore/Stream.h | 10 +++--- 5 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Tests/LibCore/TestLibCoreStream.cpp b/Tests/LibCore/TestLibCoreStream.cpp index bf256814cc6..a6e3963337d 100644 --- a/Tests/LibCore/TestLibCoreStream.cpp +++ b/Tests/LibCore/TestLibCoreStream.cpp @@ -36,7 +36,7 @@ TEST_CASE(file_open) auto maybe_size = file->size(); EXPECT(!maybe_size.is_error()); - EXPECT_EQ(maybe_size.value(), 0); + EXPECT_EQ(maybe_size.value(), 0ul); } TEST_CASE(file_write_bytes) @@ -80,7 +80,7 @@ TEST_CASE(file_seeking_around) EXPECT(!maybe_file.is_error()); auto file = maybe_file.release_value(); - EXPECT_EQ(file->size().release_value(), 8702); + EXPECT_EQ(file->size().release_value(), 8702ul); auto maybe_buffer = ByteBuffer::create_uninitialized(16); EXPECT(!maybe_buffer.is_error()); @@ -89,17 +89,17 @@ TEST_CASE(file_seeking_around) StringView buffer_contents { buffer.bytes() }; EXPECT(!file->seek(500, Core::Stream::SeekMode::SetPosition).is_error()); - EXPECT_EQ(file->tell().release_value(), 500); + EXPECT_EQ(file->tell().release_value(), 500ul); EXPECT(!file->read_entire_buffer(buffer).is_error()); EXPECT_EQ(buffer_contents, expected_seek_contents1); EXPECT(!file->seek(234, Core::Stream::SeekMode::FromCurrentPosition).is_error()); - EXPECT_EQ(file->tell().release_value(), 750); + EXPECT_EQ(file->tell().release_value(), 750ul); EXPECT(!file->read_entire_buffer(buffer).is_error()); EXPECT_EQ(buffer_contents, expected_seek_contents2); EXPECT(!file->seek(-105, Core::Stream::SeekMode::FromEndPosition).is_error()); - EXPECT_EQ(file->tell().release_value(), 8597); + EXPECT_EQ(file->tell().release_value(), 8597ul); EXPECT(!file->read_entire_buffer(buffer).is_error()); EXPECT_EQ(buffer_contents, expected_seek_contents3); } @@ -113,7 +113,7 @@ TEST_CASE(file_adopt_fd) EXPECT(!maybe_file.is_error()); auto file = maybe_file.release_value(); - EXPECT_EQ(file->size().release_value(), 8702); + EXPECT_EQ(file->size().release_value(), 8702ul); auto maybe_buffer = ByteBuffer::create_uninitialized(16); EXPECT(!maybe_buffer.is_error()); @@ -122,7 +122,7 @@ TEST_CASE(file_adopt_fd) StringView buffer_contents { buffer.bytes() }; EXPECT(!file->seek(500, Core::Stream::SeekMode::SetPosition).is_error()); - EXPECT_EQ(file->tell().release_value(), 500); + EXPECT_EQ(file->tell().release_value(), 500ul); EXPECT(!file->read_entire_buffer(buffer).is_error()); EXPECT_EQ(buffer_contents, expected_seek_contents1); @@ -142,10 +142,10 @@ TEST_CASE(file_truncate) auto file = maybe_file.release_value(); EXPECT(!file->truncate(999).is_error()); - EXPECT_EQ(file->size().release_value(), 999); + EXPECT_EQ(file->size().release_value(), 999ul); EXPECT(!file->truncate(42).is_error()); - EXPECT_EQ(file->size().release_value(), 42); + EXPECT_EQ(file->size().release_value(), 42ul); } // TCPSocket tests @@ -477,7 +477,7 @@ TEST_CASE(buffered_file_tell_and_seek) // Initial state. { auto current_offset = buffered_file->tell().release_value(); - EXPECT_EQ(current_offset, 0); + EXPECT_EQ(current_offset, 0ul); } // Read a character. @@ -485,7 +485,7 @@ TEST_CASE(buffered_file_tell_and_seek) auto character = buffered_file->read_value().release_value(); EXPECT_EQ(character, 'W'); auto current_offset = buffered_file->tell().release_value(); - EXPECT_EQ(current_offset, 1); + EXPECT_EQ(current_offset, 1ul); } // Read one more character. @@ -493,13 +493,13 @@ TEST_CASE(buffered_file_tell_and_seek) auto character = buffered_file->read_value().release_value(); EXPECT_EQ(character, 'e'); auto current_offset = buffered_file->tell().release_value(); - EXPECT_EQ(current_offset, 2); + EXPECT_EQ(current_offset, 2ul); } // Seek seven characters forward. { auto current_offset = buffered_file->seek(7, Core::Stream::SeekMode::FromCurrentPosition).release_value(); - EXPECT_EQ(current_offset, 9); + EXPECT_EQ(current_offset, 9ul); } // Read a character again. @@ -507,13 +507,13 @@ TEST_CASE(buffered_file_tell_and_seek) auto character = buffered_file->read_value().release_value(); EXPECT_EQ(character, 'o'); auto current_offset = buffered_file->tell().release_value(); - EXPECT_EQ(current_offset, 10); + EXPECT_EQ(current_offset, 10ul); } // Seek five characters backwards. { auto current_offset = buffered_file->seek(-5, Core::Stream::SeekMode::FromCurrentPosition).release_value(); - EXPECT_EQ(current_offset, 5); + EXPECT_EQ(current_offset, 5ul); } // Read a character. @@ -521,13 +521,13 @@ TEST_CASE(buffered_file_tell_and_seek) auto character = buffered_file->read_value().release_value(); EXPECT_EQ(character, 'h'); auto current_offset = buffered_file->tell().release_value(); - EXPECT_EQ(current_offset, 6); + EXPECT_EQ(current_offset, 6ul); } // Seek back to the beginning. { auto current_offset = buffered_file->seek(0, Core::Stream::SeekMode::SetPosition).release_value(); - EXPECT_EQ(current_offset, 0); + EXPECT_EQ(current_offset, 0ul); } // Read the first character. This should prime the buffer if it hasn't happened already. @@ -535,13 +535,13 @@ TEST_CASE(buffered_file_tell_and_seek) auto character = buffered_file->read_value().release_value(); EXPECT_EQ(character, 'W'); auto current_offset = buffered_file->tell().release_value(); - EXPECT_EQ(current_offset, 1); + EXPECT_EQ(current_offset, 1ul); } // Seek beyond the buffer size, which should invalidate the buffer. { auto current_offset = buffered_file->seek(12, Core::Stream::SeekMode::SetPosition).release_value(); - EXPECT_EQ(current_offset, 12); + EXPECT_EQ(current_offset, 12ul); } // Ensure that we still read the correct contents from the new offset with a (presumably) freshly filled buffer. @@ -549,7 +549,7 @@ TEST_CASE(buffered_file_tell_and_seek) auto character = buffered_file->read_value().release_value(); EXPECT_EQ(character, 'r'); auto current_offset = buffered_file->tell().release_value(); - EXPECT_EQ(current_offset, 13); + EXPECT_EQ(current_offset, 13ul); } } diff --git a/Userland/Libraries/LibCore/MemoryStream.cpp b/Userland/Libraries/LibCore/MemoryStream.cpp index 6a7d5291599..625a1edcb9c 100644 --- a/Userland/Libraries/LibCore/MemoryStream.cpp +++ b/Userland/Libraries/LibCore/MemoryStream.cpp @@ -63,7 +63,7 @@ ErrorOr FixedMemoryStream::read(Bytes bytes) return bytes.trim(to_read); } -ErrorOr FixedMemoryStream::seek(i64 offset, SeekMode seek_mode) +ErrorOr FixedMemoryStream::seek(i64 offset, SeekMode seek_mode) { switch (seek_mode) { case SeekMode::SetPosition: @@ -85,7 +85,7 @@ ErrorOr FixedMemoryStream::seek(i64 offset, SeekMode seek_mode) m_offset = m_bytes.size() - offset; break; } - return static_cast(m_offset); + return m_offset; } ErrorOr FixedMemoryStream::write(ReadonlyBytes bytes) diff --git a/Userland/Libraries/LibCore/MemoryStream.h b/Userland/Libraries/LibCore/MemoryStream.h index 590e702b591..9ae0f53b54e 100644 --- a/Userland/Libraries/LibCore/MemoryStream.h +++ b/Userland/Libraries/LibCore/MemoryStream.h @@ -28,7 +28,7 @@ public: virtual ErrorOr truncate(off_t) override; virtual ErrorOr read(Bytes bytes) override; - virtual ErrorOr seek(i64 offset, SeekMode seek_mode = SeekMode::SetPosition) override; + virtual ErrorOr seek(i64 offset, SeekMode seek_mode = SeekMode::SetPosition) override; virtual ErrorOr write(ReadonlyBytes bytes) override; virtual ErrorOr write_entire_buffer(ReadonlyBytes bytes) override; diff --git a/Userland/Libraries/LibCore/Stream.cpp b/Userland/Libraries/LibCore/Stream.cpp index 23b26aa85b7..022535a6f67 100644 --- a/Userland/Libraries/LibCore/Stream.cpp +++ b/Userland/Libraries/LibCore/Stream.cpp @@ -108,14 +108,14 @@ ErrorOr Stream::write_entire_buffer(ReadonlyBytes buffer) return {}; } -ErrorOr SeekableStream::tell() const +ErrorOr SeekableStream::tell() const { // Seek with 0 and SEEK_CUR does not modify anything despite the const_cast, // so it's safe to do this. return const_cast(this)->seek(0, SeekMode::FromCurrentPosition); } -ErrorOr SeekableStream::size() +ErrorOr SeekableStream::size() { auto original_position = TRY(tell()); @@ -280,7 +280,7 @@ void File::close() m_fd = -1; } -ErrorOr File::seek(i64 offset, SeekMode mode) +ErrorOr File::seek(i64 offset, SeekMode mode) { int syscall_mode; switch (mode) { @@ -297,7 +297,7 @@ ErrorOr File::seek(i64 offset, SeekMode mode) VERIFY_NOT_REACHED(); } - off_t seek_result = TRY(System::lseek(m_fd, offset, syscall_mode)); + size_t seek_result = TRY(System::lseek(m_fd, offset, syscall_mode)); m_last_read_was_eof = false; return seek_result; } diff --git a/Userland/Libraries/LibCore/Stream.h b/Userland/Libraries/LibCore/Stream.h index 44fb97e8817..7e3cf6f23a8 100644 --- a/Userland/Libraries/LibCore/Stream.h +++ b/Userland/Libraries/LibCore/Stream.h @@ -152,13 +152,13 @@ class SeekableStream : public Stream { public: /// Seeks to the given position in the given mode. Returns either the /// current position of the file, or an errno in the case of an error. - virtual ErrorOr seek(i64 offset, SeekMode) = 0; + virtual ErrorOr seek(i64 offset, SeekMode) = 0; /// Returns the current position of the file, or an errno in the case of /// an error. - virtual ErrorOr tell() const; + virtual ErrorOr tell() const; /// Returns the total size of the stream, or an errno in the case of an /// error. May not preserve the original position on the stream on failure. - virtual ErrorOr size(); + virtual ErrorOr size(); /// Shrinks or extends the stream to the given size. Returns an errno in /// the case of an error. virtual ErrorOr truncate(off_t length) = 0; @@ -306,7 +306,7 @@ public: virtual bool is_eof() const override; virtual bool is_open() const override; virtual void close() override; - virtual ErrorOr seek(i64 offset, SeekMode) override; + virtual ErrorOr seek(i64 offset, SeekMode) override; virtual ErrorOr truncate(off_t length) override; int leak_fd(Badge<::IPC::File>) @@ -871,7 +871,7 @@ public: virtual bool is_eof() const override { return m_helper.is_eof(); } virtual bool is_open() const override { return m_helper.stream().is_open(); } virtual void close() override { m_helper.stream().close(); } - virtual ErrorOr seek(i64 offset, SeekMode mode) override + virtual ErrorOr seek(i64 offset, SeekMode mode) override { if (mode == SeekMode::FromCurrentPosition) { // If possible, seek using the buffer alone.