From a15ed8743d03c6c683f19447be20ca7dac768485 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Wed, 10 Nov 2021 14:33:44 +0100 Subject: [PATCH] AK: Make ByteBuffer::try_* functions return ErrorOr Same as Vector, ByteBuffer now also signals allocation failure by returning an ENOMEM Error instead of a bool, allowing us to use the TRY() and MUST() patterns. --- AK/ByteBuffer.h | 53 ++++++++----------- AK/StringBuilder.cpp | 19 +++---- AK/StringBuilder.h | 2 +- Tests/LibTLS/TestTLSHandshake.cpp | 2 +- Userland/Applications/Debugger/main.cpp | 2 +- Userland/Libraries/LibC/netdb.cpp | 4 +- Userland/Libraries/LibCrypto/ASN1/PEM.cpp | 2 +- .../Libraries/LibCrypto/PK/Code/EMSA_PSS.h | 4 +- Userland/Libraries/LibGfx/BMPWriter.cpp | 4 +- Userland/Libraries/LibLine/Editor.cpp | 2 +- Userland/Libraries/LibPDF/Parser.cpp | 4 +- Userland/Libraries/LibSQL/Heap.cpp | 2 +- Userland/Libraries/LibTLS/HandshakeClient.cpp | 2 +- Userland/Libraries/LibTLS/Record.cpp | 5 +- Userland/Libraries/LibTLS/TLSv12.cpp | 2 +- .../LibWasm/AbstractMachine/AbstractMachine.h | 2 +- Userland/Libraries/LibWasm/Parser/Parser.cpp | 4 +- .../InspectorServer/InspectableProcess.cpp | 4 +- Userland/Utilities/pro.cpp | 3 +- Userland/Utilities/test-crypto.cpp | 7 ++- 20 files changed, 59 insertions(+), 70 deletions(-) diff --git a/AK/ByteBuffer.h b/AK/ByteBuffer.h index 641371cd68..045c6ebfd4 100644 --- a/AK/ByteBuffer.h +++ b/AK/ByteBuffer.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2020, Andreas Kling + * Copyright (c) 2018-2021, Andreas Kling * Copyright (c) 2021, Gunnar Beutner * * SPDX-License-Identifier: BSD-2-Clause @@ -8,6 +8,7 @@ #pragma once #include +#include #include #include #include @@ -27,8 +28,7 @@ public: ByteBuffer(ByteBuffer const& other) { - auto ok = try_resize(other.size()); - VERIFY(ok); + MUST(try_resize(other.size())); VERIFY(m_size == other.size()); __builtin_memcpy(data(), other.data(), other.size()); } @@ -54,8 +54,7 @@ public: if (m_size > other.size()) { trim(other.size(), true); } else { - auto ok = try_resize(other.size()); - VERIFY(ok); + MUST(try_resize(other.size())); } __builtin_memcpy(data(), other.data(), other.size()); } @@ -65,7 +64,7 @@ public: [[nodiscard]] static Optional create_uninitialized(size_t size) { auto buffer = ByteBuffer(); - if (!buffer.try_resize(size)) + if (buffer.try_resize(size).is_error()) return {}; return { move(buffer) }; } @@ -157,64 +156,58 @@ public: ALWAYS_INLINE void resize(size_t new_size) { - auto ok = try_resize(new_size); - VERIFY(ok); + MUST(try_resize(new_size)); } ALWAYS_INLINE void ensure_capacity(size_t new_capacity) { - auto ok = try_ensure_capacity(new_capacity); - VERIFY(ok); + MUST(try_ensure_capacity(new_capacity)); } - [[nodiscard]] ALWAYS_INLINE bool try_resize(size_t new_size) + ErrorOr try_resize(size_t new_size) { if (new_size <= m_size) { trim(new_size, false); - return true; + return {}; } - if (!try_ensure_capacity(new_size)) - return false; + TRY(try_ensure_capacity(new_size)); m_size = new_size; - return true; + return {}; } - [[nodiscard]] ALWAYS_INLINE bool try_ensure_capacity(size_t new_capacity) + ErrorOr try_ensure_capacity(size_t new_capacity) { if (new_capacity <= capacity()) - return true; + return {}; return try_ensure_capacity_slowpath(new_capacity); } void append(ReadonlyBytes const& bytes) { - auto ok = try_append(bytes); - VERIFY(ok); + MUST(try_append(bytes)); } void append(void const* data, size_t data_size) { append({ data, data_size }); } - [[nodiscard]] bool try_append(ReadonlyBytes const& bytes) + ErrorOr try_append(ReadonlyBytes const& bytes) { return try_append(bytes.data(), bytes.size()); } - [[nodiscard]] bool try_append(void const* data, size_t data_size) + ErrorOr try_append(void const* data, size_t data_size) { if (data_size == 0) - return true; + return {}; VERIFY(data != nullptr); int old_size = size(); - if (!try_resize(size() + data_size)) - return false; + TRY(try_resize(size() + data_size)); __builtin_memcpy(this->data() + old_size, data, data_size); - return true; + return {}; } void operator+=(ByteBuffer const& other) { - auto ok = try_append(other.data(), other.size()); - VERIFY(ok); + MUST(try_append(other.data(), other.size())); } void overwrite(size_t offset, void const* data, size_t data_size) @@ -269,12 +262,12 @@ private: m_inline = true; } - [[nodiscard]] NEVER_INLINE bool try_ensure_capacity_slowpath(size_t new_capacity) + NEVER_INLINE ErrorOr try_ensure_capacity_slowpath(size_t new_capacity) { new_capacity = kmalloc_good_size(new_capacity); auto new_buffer = (u8*)kmalloc(new_capacity); if (!new_buffer) - return false; + return Error::from_errno(ENOMEM); if (m_inline) { __builtin_memcpy(new_buffer, data(), m_size); @@ -286,7 +279,7 @@ private: m_outline_buffer = new_buffer; m_outline_capacity = new_capacity; m_inline = false; - return true; + return {}; } union { diff --git a/AK/StringBuilder.cpp b/AK/StringBuilder.cpp index df4331f9ca..f382be467c 100644 --- a/AK/StringBuilder.cpp +++ b/AK/StringBuilder.cpp @@ -18,18 +18,19 @@ namespace AK { -inline bool StringBuilder::will_append(size_t size) +inline ErrorOr StringBuilder::will_append(size_t size) { Checked needed_capacity = m_buffer.size(); needed_capacity += size; VERIFY(!needed_capacity.has_overflow()); // Prefer to completely use the existing capacity first if (needed_capacity <= m_buffer.capacity()) - return true; + return {}; Checked expanded_capacity = needed_capacity; expanded_capacity *= 2; VERIFY(!expanded_capacity.has_overflow()); - return m_buffer.try_ensure_capacity(expanded_capacity.value()); + TRY(m_buffer.try_ensure_capacity(expanded_capacity.value())); + return {}; } StringBuilder::StringBuilder(size_t initial_capacity) @@ -41,10 +42,8 @@ void StringBuilder::append(StringView const& str) { if (str.is_empty()) return; - auto ok = will_append(str.length()); - VERIFY(ok); - ok = m_buffer.try_append(str.characters_without_null_termination(), str.length()); - VERIFY(ok); + MUST(will_append(str.length())); + MUST(m_buffer.try_append(str.characters_without_null_termination(), str.length())); } void StringBuilder::append(char const* characters, size_t length) @@ -54,10 +53,8 @@ void StringBuilder::append(char const* characters, size_t length) void StringBuilder::append(char ch) { - auto ok = will_append(1); - VERIFY(ok); - ok = m_buffer.try_append(&ch, 1); - VERIFY(ok); + MUST(will_append(1)); + MUST(m_buffer.try_append(&ch, 1)); } void StringBuilder::appendvf(char const* fmt, va_list ap) diff --git a/AK/StringBuilder.h b/AK/StringBuilder.h index bc29e12f06..fc7f830dc4 100644 --- a/AK/StringBuilder.h +++ b/AK/StringBuilder.h @@ -64,7 +64,7 @@ public: } private: - bool will_append(size_t); + ErrorOr will_append(size_t); u8* data() { return m_buffer.data(); } u8 const* data() const { return m_buffer.data(); } diff --git a/Tests/LibTLS/TestTLSHandshake.cpp b/Tests/LibTLS/TestTLSHandshake.cpp index 9494f258e8..6218e5b0d1 100644 --- a/Tests/LibTLS/TestTLSHandshake.cpp +++ b/Tests/LibTLS/TestTLSHandshake.cpp @@ -96,7 +96,7 @@ TEST_CASE(test_TLS_hello_handshake) loop.quit(1); } else { // print_buffer(data.value(), 16); - if (!contents.try_append(data.value().data(), data.value().size())) { + if (contents.try_append(data.value().data(), data.value().size()).is_error()) { FAIL("Allocation failure"); loop.quit(1); } diff --git a/Userland/Applications/Debugger/main.cpp b/Userland/Applications/Debugger/main.cpp index ec3d4c4efd..9751c28df2 100644 --- a/Userland/Applications/Debugger/main.cpp +++ b/Userland/Applications/Debugger/main.cpp @@ -67,7 +67,7 @@ static bool handle_disassemble_command(const String& command, void* first_instru auto value = g_debug_session->peek(reinterpret_cast(first_instruction) + i); if (!value.has_value()) break; - if (!code.try_append(&value, sizeof(u32))) + if (code.try_append(&value, sizeof(u32)).is_error()) break; } diff --git a/Userland/Libraries/LibC/netdb.cpp b/Userland/Libraries/LibC/netdb.cpp index d521a78aff..d91bd4b1f1 100644 --- a/Userland/Libraries/LibC/netdb.cpp +++ b/Userland/Libraries/LibC/netdb.cpp @@ -460,7 +460,7 @@ static bool fill_getserv_buffers(const char* line, ssize_t read) break; } auto alias = split_line[i].to_byte_buffer(); - if (!alias.try_append("\0", sizeof(char))) + if (alias.try_append("\0", sizeof(char)).is_error()) return false; __getserv_alias_list_buffer.append(move(alias)); } @@ -630,7 +630,7 @@ static bool fill_getproto_buffers(const char* line, ssize_t read) if (split_line[i].starts_with('#')) break; auto alias = split_line[i].to_byte_buffer(); - if (!alias.try_append("\0", sizeof(char))) + if (alias.try_append("\0", sizeof(char)).is_error()) return false; __getproto_alias_list_buffer.append(move(alias)); } diff --git a/Userland/Libraries/LibCrypto/ASN1/PEM.cpp b/Userland/Libraries/LibCrypto/ASN1/PEM.cpp index c901febe41..90f957cfc9 100644 --- a/Userland/Libraries/LibCrypto/ASN1/PEM.cpp +++ b/Userland/Libraries/LibCrypto/ASN1/PEM.cpp @@ -39,7 +39,7 @@ ByteBuffer decode_pem(ReadonlyBytes data) dbgln("Failed to decode PEM, likely bad Base64"); return {}; } - if (!decoded.try_append(b64decoded.value().data(), b64decoded.value().size())) { + if (decoded.try_append(b64decoded.value().data(), b64decoded.value().size()).is_error()) { dbgln("Failed to decode PEM, likely OOM condition"); return {}; } diff --git a/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h b/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h index b1b1a569a9..3193c772d4 100644 --- a/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h +++ b/Userland/Libraries/LibCrypto/PK/Code/EMSA_PSS.h @@ -152,8 +152,8 @@ public: for (size_t counter = 0; counter < length / HashFunction::DigestSize - 1; ++counter) { hash_fn.update(seed); hash_fn.update((u8*)&counter, 4); - if (!T.try_append(hash_fn.digest().data, HashFunction::DigestSize)) { - dbgln("EMSA_PSS: MGF1 digest failed, not enough space"); + if (auto result = T.try_append(hash_fn.digest().data, HashFunction::DigestSize); result.is_error()) { + dbgln("EMSA_PSS: MGF1 digest failed: {}", result.error()); return; } } diff --git a/Userland/Libraries/LibGfx/BMPWriter.cpp b/Userland/Libraries/LibGfx/BMPWriter.cpp index 87377707a1..e52f9d07f0 100644 --- a/Userland/Libraries/LibGfx/BMPWriter.cpp +++ b/Userland/Libraries/LibGfx/BMPWriter.cpp @@ -143,8 +143,8 @@ ByteBuffer BMPWriter::dump(const RefPtr bitmap, DibHeader dib_header) } } - if (!buffer.try_append(pixel_data.data(), pixel_data.size())) - dbgln("Failed to write {} bytes of pixel data to buffer", pixel_data.size()); + if (auto result = buffer.try_append(pixel_data.data(), pixel_data.size()); result.is_error()) + dbgln("Failed to write {} bytes of pixel data to buffer: {}", pixel_data.size(), result.error()); return buffer; } diff --git a/Userland/Libraries/LibLine/Editor.cpp b/Userland/Libraries/LibLine/Editor.cpp index c55c70e08e..1969aed99a 100644 --- a/Userland/Libraries/LibLine/Editor.cpp +++ b/Userland/Libraries/LibLine/Editor.cpp @@ -370,7 +370,7 @@ void Editor::insert(const u32 cp) StringBuilder builder; builder.append(Utf32View(&cp, 1)); auto str = builder.build(); - if (!m_pending_chars.try_append(str.characters(), str.length())) + if (m_pending_chars.try_append(str.characters(), str.length()).is_error()) return; readjust_anchored_styles(m_cursor, ModificationKind::Insertion); diff --git a/Userland/Libraries/LibPDF/Parser.cpp b/Userland/Libraries/LibPDF/Parser.cpp index 1872c0e9b8..a591060c47 100644 --- a/Userland/Libraries/LibPDF/Parser.cpp +++ b/Userland/Libraries/LibPDF/Parser.cpp @@ -273,8 +273,8 @@ bool Parser::initialize_hint_tables() if (!buffer_result.has_value()) return false; possible_merged_stream_buffer = buffer_result.release_value(); - auto ok = possible_merged_stream_buffer.try_append(primary_hint_stream->bytes()); - ok = ok && possible_merged_stream_buffer.try_append(overflow_hint_stream->bytes()); + auto ok = !possible_merged_stream_buffer.try_append(primary_hint_stream->bytes()).is_error(); + ok = ok && !possible_merged_stream_buffer.try_append(overflow_hint_stream->bytes()).is_error(); if (!ok) return false; hint_stream_bytes = possible_merged_stream_buffer.bytes(); diff --git a/Userland/Libraries/LibSQL/Heap.cpp b/Userland/Libraries/LibSQL/Heap.cpp index 7bf8bf9fd6..fb99fa5fa4 100644 --- a/Userland/Libraries/LibSQL/Heap.cpp +++ b/Userland/Libraries/LibSQL/Heap.cpp @@ -75,7 +75,7 @@ bool Heap::write_block(u32 block, ByteBuffer& buffer) VERIFY(buffer.size() <= BLOCKSIZE); auto sz = buffer.size(); if (sz < BLOCKSIZE) { - if (!buffer.try_resize(BLOCKSIZE)) + if (buffer.try_resize(BLOCKSIZE).is_error()) return false; memset(buffer.offset_pointer((int)sz), 0, BLOCKSIZE - sz); } diff --git a/Userland/Libraries/LibTLS/HandshakeClient.cpp b/Userland/Libraries/LibTLS/HandshakeClient.cpp index 7299f3418a..8a4304eea2 100644 --- a/Userland/Libraries/LibTLS/HandshakeClient.cpp +++ b/Userland/Libraries/LibTLS/HandshakeClient.cpp @@ -119,7 +119,7 @@ bool TLSv12::compute_master_secret_from_pre_master_secret(size_t length) return false; } - if (!m_context.master_key.try_resize(length)) { + if (m_context.master_key.try_resize(length).is_error()) { dbgln("Couldn't allocate enough space for the master key :("); return false; } diff --git a/Userland/Libraries/LibTLS/Record.cpp b/Userland/Libraries/LibTLS/Record.cpp index 0488dba089..d654fd4f62 100644 --- a/Userland/Libraries/LibTLS/Record.cpp +++ b/Userland/Libraries/LibTLS/Record.cpp @@ -56,8 +56,7 @@ void TLSv12::write_packet(ByteBuffer& packet) if (m_context.tls_buffer.size() + packet.size() > 16 * KiB) schedule_or_perform_flush(true); - auto ok = m_context.tls_buffer.try_append(packet.data(), packet.size()); - if (!ok) { + if (m_context.tls_buffer.try_append(packet.data(), packet.size()).is_error()) { // Toooooo bad, drop the record on the ground. return; } @@ -498,7 +497,7 @@ ssize_t TLSv12::handle_message(ReadonlyBytes buffer) } else { dbgln_if(TLS_DEBUG, "application data message of size {}", plain.size()); - if (!m_context.application_buffer.try_append(plain.data(), plain.size())) { + if (m_context.application_buffer.try_append(plain.data(), plain.size()).is_error()) { payload_res = (i8)Error::DecryptionFailed; auto packet = build_alert(true, (u8)AlertDescription::DecryptionFailed); write_packet(packet); diff --git a/Userland/Libraries/LibTLS/TLSv12.cpp b/Userland/Libraries/LibTLS/TLSv12.cpp index 8f31933d9c..bce3c2fd14 100644 --- a/Userland/Libraries/LibTLS/TLSv12.cpp +++ b/Userland/Libraries/LibTLS/TLSv12.cpp @@ -35,7 +35,7 @@ void TLSv12::consume(ReadonlyBytes record) dbgln_if(TLS_DEBUG, "Consuming {} bytes", record.size()); - if (!m_context.message_buffer.try_append(record)) { + if (m_context.message_buffer.try_append(record).is_error()) { dbgln("Not enough space in message buffer, dropping the record"); return; } diff --git a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h index 5f127365f8..aeec26d1f6 100644 --- a/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h +++ b/Userland/Libraries/LibWasm/AbstractMachine/AbstractMachine.h @@ -349,7 +349,7 @@ public: return false; } auto previous_size = m_size; - if (!m_data.try_resize(new_size)) + if (m_data.try_resize(new_size).is_error()) return false; m_size = new_size; // The spec requires that we zero out everything on grow diff --git a/Userland/Libraries/LibWasm/Parser/Parser.cpp b/Userland/Libraries/LibWasm/Parser/Parser.cpp index c201265088..d2ae4d4561 100644 --- a/Userland/Libraries/LibWasm/Parser/Parser.cpp +++ b/Userland/Libraries/LibWasm/Parser/Parser.cpp @@ -739,7 +739,7 @@ ParseResult CustomSection::parse(InputStream& stream) return name.error(); ByteBuffer data_buffer; - if (!data_buffer.try_resize(64)) + if (data_buffer.try_resize(64).is_error()) return ParseError::OutOfMemory; while (!stream.has_any_error() && !stream.unreliable_eof()) { @@ -747,7 +747,7 @@ ParseResult CustomSection::parse(InputStream& stream) auto size = stream.read({ buf, 16 }); if (size == 0) break; - if (!data_buffer.try_append(buf, size)) + if (data_buffer.try_append(buf, size).is_error()) return with_eof_check(stream, ParseError::HugeAllocationRequested); } diff --git a/Userland/Services/InspectorServer/InspectableProcess.cpp b/Userland/Services/InspectorServer/InspectableProcess.cpp index 140d8a1427..578945c314 100644 --- a/Userland/Services/InspectorServer/InspectableProcess.cpp +++ b/Userland/Services/InspectorServer/InspectableProcess.cpp @@ -57,8 +57,8 @@ String InspectableProcess::wait_for_response() auto packet = m_socket->read(remaining_bytes); if (packet.size() == 0) break; - if (!data.try_append(packet.data(), packet.size())) { - dbgln("Failed to append {} bytes to data buffer", packet.size()); + if (auto result = data.try_append(packet.data(), packet.size()); result.is_error()) { + dbgln("Failed to append {} bytes to data buffer: {}", packet.size(), result.error()); break; } remaining_bytes -= packet.size(); diff --git a/Userland/Utilities/pro.cpp b/Userland/Utilities/pro.cpp index 6bf653d1bc..28091cf683 100644 --- a/Userland/Utilities/pro.cpp +++ b/Userland/Utilities/pro.cpp @@ -122,7 +122,8 @@ private: { if (!m_condition()) { write_to_buffer:; - if (!m_buffer.try_append(bytes.data(), bytes.size())) + // FIXME: Propagate errors. + if (m_buffer.try_append(bytes.data(), bytes.size()).is_error()) return 0; return bytes.size(); } diff --git a/Userland/Utilities/test-crypto.cpp b/Userland/Utilities/test-crypto.cpp index 1d94219d00..456966439f 100644 --- a/Userland/Utilities/test-crypto.cpp +++ b/Userland/Utilities/test-crypto.cpp @@ -165,9 +165,8 @@ static void tls(const char* message, size_t len) g_loop.quit(0); }; } - auto ok = write.try_append(message, len); - ok = ok && write.try_append("\r\n", 2); - VERIFY(ok); + MUST(write.try_append(message, len)); + MUST(write.try_append("\r\n", 2)); } static void aes_cbc(const char* message, size_t len) @@ -2039,7 +2038,7 @@ static void tls_test_client_hello() loop.quit(1); } else { // print_buffer(data.value(), 16); - if (!contents.try_append(data.value().data(), data.value().size())) { + if (contents.try_append(data.value().data(), data.value().size()).is_error()) { FAIL(Allocation failed); loop.quit(1); }