From d564cf1e8970275a52a1ef43debe4b1d14b2c063 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Fri, 15 Apr 2022 14:52:33 +0100 Subject: [PATCH] LibCore+Everywhere: Make Core::Stream read_line() return StringView Similar reasoning to making Core::Stream::read() return Bytes, except that every user of read_line() creates a StringView from the result, so let's just return one right away. --- .../LibTimeZone/GenerateTimeZoneData.cpp | 6 ++-- .../LibUnicode/GenerateUnicodeData.cpp | 24 +++++--------- Tests/LibCore/TestLibCoreStream.cpp | 32 +++++++++---------- Userland/Applications/Browser/main.cpp | 3 +- .../ContentFilterSettingsWidget.cpp | 3 +- .../FileOperationProgressWidget.cpp | 6 ++-- Userland/Libraries/LibCore/ConfigFile.cpp | 4 +-- Userland/Libraries/LibCore/Stream.h | 12 +++---- .../LibWebSocket/Impl/WebSocketImpl.cpp | 4 +-- 9 files changed, 40 insertions(+), 54 deletions(-) diff --git a/Meta/Lagom/Tools/CodeGenerators/LibTimeZone/GenerateTimeZoneData.cpp b/Meta/Lagom/Tools/CodeGenerators/LibTimeZone/GenerateTimeZoneData.cpp index 5c13579df83..4772b4a8e95 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibTimeZone/GenerateTimeZoneData.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/LibTimeZone/GenerateTimeZoneData.cpp @@ -316,8 +316,7 @@ static ErrorOr parse_time_zones(StringView time_zone_path, TimeZoneData& t Vector* last_parsed_zone = nullptr; while (TRY(file->can_read_line())) { - auto nread = TRY(file->read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file->read_line(buffer)); if (line.is_empty() || line.trim_whitespace(TrimMode::Left).starts_with('#')) continue; @@ -374,8 +373,7 @@ static ErrorOr parse_time_zone_coordinates(Core::Stream::BufferedFile& fil Array buffer {}; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty() || line.trim_whitespace(TrimMode::Left).starts_with('#')) continue; diff --git a/Meta/Lagom/Tools/CodeGenerators/LibUnicode/GenerateUnicodeData.cpp b/Meta/Lagom/Tools/CodeGenerators/LibUnicode/GenerateUnicodeData.cpp index 8beb463413f..191562a038d 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibUnicode/GenerateUnicodeData.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/LibUnicode/GenerateUnicodeData.cpp @@ -197,8 +197,7 @@ static ErrorOr parse_special_casing(Core::Stream::BufferedFile& file, Unic Array buffer; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty() || line.starts_with('#')) continue; @@ -264,8 +263,7 @@ static ErrorOr parse_prop_list(Core::Stream::BufferedFile& file, PropList& Array buffer; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty() || line.starts_with('#')) continue; @@ -311,8 +309,7 @@ static ErrorOr parse_alias_list(Core::Stream::BufferedFile& file, PropList }; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty() || line.starts_with('#')) { if (line.ends_with("Properties"sv)) @@ -345,8 +342,7 @@ static ErrorOr parse_name_aliases(Core::Stream::BufferedFile& file, Unicod Array buffer; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty() || line.starts_with('#')) continue; @@ -387,8 +383,7 @@ static ErrorOr parse_value_alias_list(Core::Stream::BufferedFile& file, St }; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty() || line.starts_with('#')) continue; @@ -421,8 +416,7 @@ static ErrorOr parse_normalization_props(Core::Stream::BufferedFile& file, Array buffer; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty() || line.starts_with('#')) continue; @@ -518,8 +512,7 @@ static ErrorOr parse_block_display_names(Core::Stream::BufferedFile& file, { Array buffer; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty() || line.starts_with('#')) continue; @@ -547,8 +540,7 @@ static ErrorOr parse_unicode_data(Core::Stream::BufferedFile& file, Unicod Array buffer; while (TRY(file.can_read_line())) { - auto nread = TRY(file.read_line(buffer)); - StringView line { buffer.data(), nread }; + auto line = TRY(file.read_line(buffer)); if (line.is_empty()) continue; diff --git a/Tests/LibCore/TestLibCoreStream.cpp b/Tests/LibCore/TestLibCoreStream.cpp index b8a47283d04..fcb326cc149 100644 --- a/Tests/LibCore/TestLibCoreStream.cpp +++ b/Tests/LibCore/TestLibCoreStream.cpp @@ -426,15 +426,15 @@ TEST_CASE(buffered_long_file_read) auto buffer = ByteBuffer::create_uninitialized(4096).release_value(); EXPECT(!file->seek(255, Core::Stream::SeekMode::SetPosition).is_error()); EXPECT(file->can_read_line().release_value()); - auto maybe_nread = file->read_line(buffer); - EXPECT(!maybe_nread.is_error()); - EXPECT_EQ(maybe_nread.value(), 4095ul); // 4095 bytes on the third line + auto maybe_line = file->read_line(buffer); + EXPECT(!maybe_line.is_error()); + EXPECT_EQ(maybe_line.value().length(), 4095ul); // 4095 bytes on the third line // Testing that buffering with seeking works properly EXPECT(!file->seek(365, Core::Stream::SeekMode::SetPosition).is_error()); - auto maybe_after_seek_nread = file->read_line(buffer); - EXPECT(!maybe_after_seek_nread.is_error()); - EXPECT_EQ(maybe_after_seek_nread.value(), 3985ul); // 4095 - 110 + auto maybe_after_seek_line = file->read_line(buffer); + EXPECT(!maybe_after_seek_line.is_error()); + EXPECT_EQ(maybe_after_seek_line.value().length(), 3985ul); // 4095 - 110 } TEST_CASE(buffered_small_file_read) @@ -456,10 +456,10 @@ TEST_CASE(buffered_small_file_read) auto buffer = ByteBuffer::create_uninitialized(4096).release_value(); for (auto const& line : expected_lines) { VERIFY(file->can_read_line().release_value()); - auto maybe_nread = file->read_line(buffer); - EXPECT(!maybe_nread.is_error()); - EXPECT_EQ(maybe_nread.value(), line.length()); - EXPECT_EQ(StringView(buffer.span().trim(maybe_nread.value())), line); + auto maybe_read_line = file->read_line(buffer); + EXPECT(!maybe_read_line.is_error()); + EXPECT_EQ(maybe_read_line.value().length(), line.length()); + EXPECT_EQ(StringView(buffer.span().trim(maybe_read_line.value().length())), line); } EXPECT(!file->can_read_line().is_error()); EXPECT(!file->can_read_line().value()); @@ -496,13 +496,13 @@ TEST_CASE(buffered_tcp_socket_read) auto receive_buffer = ByteBuffer::create_uninitialized(64).release_value(); - auto maybe_first_nread = client_socket->read_line(receive_buffer); - EXPECT(!maybe_first_nread.is_error()); - StringView first_received_line { receive_buffer.data(), maybe_first_nread.value() }; + auto maybe_first_received_line = client_socket->read_line(receive_buffer); + EXPECT(!maybe_first_received_line.is_error()); + auto first_received_line = maybe_first_received_line.value(); EXPECT_EQ(first_received_line, first_line); - auto maybe_second_nread = client_socket->read_line(receive_buffer); - EXPECT(!maybe_second_nread.is_error()); - StringView second_received_line { receive_buffer.data(), maybe_second_nread.value() }; + auto maybe_second_received_line = client_socket->read_line(receive_buffer); + EXPECT(!maybe_second_received_line.is_error()); + auto second_received_line = maybe_second_received_line.value(); EXPECT_EQ(second_received_line, second_line); } diff --git a/Userland/Applications/Browser/main.cpp b/Userland/Applications/Browser/main.cpp index e963e312c38..4c6839c8c28 100644 --- a/Userland/Applications/Browser/main.cpp +++ b/Userland/Applications/Browser/main.cpp @@ -43,8 +43,7 @@ static ErrorOr load_content_filters() auto ad_filter_list = TRY(Core::Stream::BufferedFile::create(move(file))); auto buffer = TRY(ByteBuffer::create_uninitialized(4096)); while (TRY(ad_filter_list->can_read_line())) { - auto length = TRY(ad_filter_list->read_line(buffer)); - StringView line { buffer.data(), length }; + auto line = TRY(ad_filter_list->read_line(buffer)); if (!line.is_empty()) Browser::g_content_filters.append(line); } diff --git a/Userland/Applications/BrowserSettings/ContentFilterSettingsWidget.cpp b/Userland/Applications/BrowserSettings/ContentFilterSettingsWidget.cpp index 04dd93e4dbc..5d66d0d6d70 100644 --- a/Userland/Applications/BrowserSettings/ContentFilterSettingsWidget.cpp +++ b/Userland/Applications/BrowserSettings/ContentFilterSettingsWidget.cpp @@ -30,8 +30,7 @@ ErrorOr DomainListModel::load() auto content_filter_list = TRY(Core::Stream::BufferedFile::create(move(file))); auto buffer = TRY(ByteBuffer::create_uninitialized(4096)); while (TRY(content_filter_list->can_read_line())) { - auto length = TRY(content_filter_list->read_line(buffer)); - StringView line { buffer.data(), length }; + auto line = TRY(content_filter_list->read_line(buffer)); dbgln("Content filter for {}", line); if (!line.is_empty()) m_domain_list.append(line); diff --git a/Userland/Applications/FileManager/FileOperationProgressWidget.cpp b/Userland/Applications/FileManager/FileOperationProgressWidget.cpp index 8df185d1dc1..51532cad110 100644 --- a/Userland/Applications/FileManager/FileOperationProgressWidget.cpp +++ b/Userland/Applications/FileManager/FileOperationProgressWidget.cpp @@ -72,13 +72,13 @@ FileOperationProgressWidget::FileOperationProgressWidget(FileOperation operation m_notifier = Core::Notifier::construct(helper_pipe_fd, Core::Notifier::Read); m_notifier->on_ready_to_read = [this] { auto line_buffer = ByteBuffer::create_zeroed(1 * KiB).release_value_but_fixme_should_propagate_errors(); - auto line_length_or_error = m_helper_pipe->read_line(line_buffer.bytes()); - if (line_length_or_error.is_error() || line_length_or_error.value() == 0) { + auto line_or_error = m_helper_pipe->read_line(line_buffer.bytes()); + if (line_or_error.is_error() || line_or_error.value().is_empty()) { did_error("Read from pipe returned null."sv); return; } - StringView line { line_buffer.bytes().data(), line_length_or_error.value() }; + auto line = line_or_error.release_value(); auto parts = line.split_view(' '); VERIFY(!parts.is_empty()); diff --git a/Userland/Libraries/LibCore/ConfigFile.cpp b/Userland/Libraries/LibCore/ConfigFile.cpp index bab469e7473..b8474ca37f5 100644 --- a/Userland/Libraries/LibCore/ConfigFile.cpp +++ b/Userland/Libraries/LibCore/ConfigFile.cpp @@ -88,9 +88,7 @@ ErrorOr ConfigFile::reparse() auto buffer = TRY(ByteBuffer::create_uninitialized(4096)); while (TRY(m_file->can_read_line())) { - auto length = TRY(m_file->read_line(buffer)); - - StringView line { buffer.data(), length }; + auto line = TRY(m_file->read_line(buffer)); size_t i = 0; while (i < line.length() && (line[i] == ' ' || line[i] == '\t' || line[i] == '\n')) diff --git a/Userland/Libraries/LibCore/Stream.h b/Userland/Libraries/LibCore/Stream.h index 067cf238fc7..71c3432f52a 100644 --- a/Userland/Libraries/LibCore/Stream.h +++ b/Userland/Libraries/LibCore/Stream.h @@ -557,10 +557,10 @@ public: // Reads into the buffer until \n is encountered. // The size of the Bytes object is the maximum amount of bytes that will be - // read. Returns the amount of bytes read. - ErrorOr read_line(Bytes buffer) + // read. Returns the bytes read as a StringView. + ErrorOr read_line(Bytes buffer) { - return TRY(read_until(buffer, "\n"sv)).size(); + return StringView { TRY(read_until(buffer, "\n"sv)) }; } ErrorOr read_until(Bytes buffer, StringView candidate) @@ -769,7 +769,7 @@ public: return m_helper.stream().truncate(length); } - ErrorOr read_line(Bytes buffer) { return m_helper.read_line(move(buffer)); } + ErrorOr read_line(Bytes buffer) { return m_helper.read_line(move(buffer)); } ErrorOr read_until(Bytes buffer, StringView candidate) { return m_helper.read_until(move(buffer), move(candidate)); } template ErrorOr read_until_any_of(Bytes buffer, Array candidates) { return m_helper.read_until_any_of(move(buffer), move(candidates)); } @@ -790,7 +790,7 @@ private: class BufferedSocketBase : public Socket { public: - virtual ErrorOr read_line(Bytes buffer) = 0; + virtual ErrorOr read_line(Bytes buffer) = 0; virtual ErrorOr read_until(Bytes buffer, StringView candidate) = 0; virtual ErrorOr can_read_line() = 0; virtual size_t buffer_size() const = 0; @@ -838,7 +838,7 @@ public: virtual ErrorOr set_close_on_exec(bool enabled) override { return m_helper.stream().set_close_on_exec(enabled); } virtual void set_notifications_enabled(bool enabled) override { m_helper.stream().set_notifications_enabled(enabled); } - virtual ErrorOr read_line(Bytes buffer) override { return m_helper.read_line(move(buffer)); } + virtual ErrorOr read_line(Bytes buffer) override { return m_helper.read_line(move(buffer)); } virtual ErrorOr read_until(Bytes buffer, StringView candidate) override { return m_helper.read_until(move(buffer), move(candidate)); } template ErrorOr read_until_any_of(Bytes buffer, Array candidates) { return m_helper.read_until_any_of(move(buffer), move(candidates)); } diff --git a/Userland/Libraries/LibWebSocket/Impl/WebSocketImpl.cpp b/Userland/Libraries/LibWebSocket/Impl/WebSocketImpl.cpp index 3486b56cb7d..379bd0a4b4d 100644 --- a/Userland/Libraries/LibWebSocket/Impl/WebSocketImpl.cpp +++ b/Userland/Libraries/LibWebSocket/Impl/WebSocketImpl.cpp @@ -63,8 +63,8 @@ ErrorOr WebSocketImpl::read(int max_size) ErrorOr WebSocketImpl::read_line(size_t size) { auto buffer = TRY(ByteBuffer::create_uninitialized(size)); - auto nread = TRY(m_socket->read_line(buffer)); - return String::copy(buffer.span().slice(0, nread)); + auto line = TRY(m_socket->read_line(buffer)); + return line.to_string(); } }