Skip to content

Commit

Permalink
AK: Remove the fallible constructor from FixedMemoryStream
Browse files Browse the repository at this point in the history
  • Loading branch information
timschumi authored and linusg committed Feb 8, 2023
1 parent 8b2f23d commit 220fbca
Show file tree
Hide file tree
Showing 31 changed files with 185 additions and 209 deletions.
10 changes: 0 additions & 10 deletions AK/MemoryStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,6 @@ FixedMemoryStream::FixedMemoryStream(ReadonlyBytes bytes)
{
}

ErrorOr<NonnullOwnPtr<FixedMemoryStream>> FixedMemoryStream::construct(Bytes bytes)
{
return adopt_nonnull_own_or_enomem<FixedMemoryStream>(new (nothrow) FixedMemoryStream(bytes));
}

ErrorOr<NonnullOwnPtr<FixedMemoryStream>> FixedMemoryStream::construct(ReadonlyBytes bytes)
{
return adopt_nonnull_own_or_enomem<FixedMemoryStream>(new (nothrow) FixedMemoryStream(bytes));
}

bool FixedMemoryStream::is_eof() const
{
return m_offset >= m_bytes.size();
Expand Down
7 changes: 2 additions & 5 deletions AK/MemoryStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ namespace AK {
/// using a single read/write head.
class FixedMemoryStream final : public SeekableStream {
public:
static ErrorOr<NonnullOwnPtr<FixedMemoryStream>> construct(Bytes bytes);
static ErrorOr<NonnullOwnPtr<FixedMemoryStream>> construct(ReadonlyBytes bytes);
explicit FixedMemoryStream(Bytes bytes);
explicit FixedMemoryStream(ReadonlyBytes bytes);

virtual bool is_eof() const override;
virtual bool is_open() const override;
Expand All @@ -36,9 +36,6 @@ class FixedMemoryStream final : public SeekableStream {
size_t remaining() const;

private:
explicit FixedMemoryStream(Bytes bytes);
explicit FixedMemoryStream(ReadonlyBytes bytes);

Bytes m_bytes;
size_t m_offset { 0 };
bool m_writing_enabled { true };
Expand Down
9 changes: 2 additions & 7 deletions Meta/Lagom/Fuzzers/FuzzBrotli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,9 @@

extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
{
auto bufstream_result = FixedMemoryStream::construct({ data, size });
if (bufstream_result.is_error()) {
dbgln("MemoryStream::construct() failed.");
return 0;
}
auto bufstream = bufstream_result.release_value();
FixedMemoryStream bufstream { { data, size } };

auto brotli_stream = Compress::BrotliDecompressionStream { *bufstream };
auto brotli_stream = Compress::BrotliDecompressionStream { bufstream };

(void)brotli_stream.read_until_eof();
return 0;
Expand Down
2 changes: 1 addition & 1 deletion Meta/Lagom/Fuzzers/FuzzTar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
{
auto input_stream_or_error = FixedMemoryStream::construct({ data, size });
auto input_stream_or_error = try_make<FixedMemoryStream>(ReadonlyBytes { data, size });

if (input_stream_or_error.is_error())
return 0;
Expand Down
7 changes: 2 additions & 5 deletions Meta/Lagom/Fuzzers/FuzzWasmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@
extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
{
ReadonlyBytes bytes { data, size };
auto stream_or_error = FixedMemoryStream::construct(bytes);
if (stream_or_error.is_error())
return 0;
auto stream = stream_or_error.release_value();
[[maybe_unused]] auto result = Wasm::Module::parse(*stream);
FixedMemoryStream stream { bytes };
[[maybe_unused]] auto result = Wasm::Module::parse(stream);
return 0;
}
8 changes: 4 additions & 4 deletions Meta/Lagom/Tools/CodeGenerators/IPCCompiler/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,8 +586,8 @@ class @endpoint.name@Endpoint {
static ErrorOr<NonnullOwnPtr<IPC::Message>> decode_message(ReadonlyBytes buffer, [[maybe_unused]] Core::Stream::LocalSocket& socket)
{
auto stream = TRY(FixedMemoryStream::construct(buffer));
auto message_endpoint_magic = TRY(stream->read_value<u32>());)~~~");
FixedMemoryStream stream { buffer };
auto message_endpoint_magic = TRY(stream.read_value<u32>());)~~~");
generator.append(R"~~~(
if (message_endpoint_magic != @endpoint.magic@) {)~~~");
Expand All @@ -599,7 +599,7 @@ class @endpoint.name@Endpoint {
return Error::from_string_literal("Endpoint magic number mismatch, not my message!");
}
auto message_id = TRY(stream->read_value<i32>());)~~~");
auto message_id = TRY(stream.read_value<i32>());)~~~");
generator.appendln(R"~~~(
switch (message_id) {)~~~");
Expand All @@ -613,7 +613,7 @@ class @endpoint.name@Endpoint {

message_generator.append(R"~~~(
case (int)Messages::@endpoint.name@::MessageID::@message.pascal_name@:
return TRY(Messages::@endpoint.name@::@message.pascal_name@::decode(*stream, socket));)~~~");
return TRY(Messages::@endpoint.name@::@message.pascal_name@::decode(stream, socket));)~~~");
};

do_decode_message(message.name);
Expand Down
100 changes: 50 additions & 50 deletions Tests/AK/TestLEB128.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,42 +14,42 @@ TEST_CASE(single_byte)
u32 output = {};
i32 output_signed = {};
u8 buf[] = { 0x00 };
auto stream = MUST(FixedMemoryStream::construct(ReadonlyBytes { buf, sizeof(buf) }));
FixedMemoryStream stream { ReadonlyBytes { buf, sizeof(buf) } };

// less than/eq 0b0011_1111, signed == unsigned == raw byte
for (u8 i = 0u; i <= 0x3F; ++i) {
buf[0] = i;

MUST(stream->seek(0));
output = MUST(stream->read_value<LEB128<u32>>());
MUST(stream.seek(0));
output = MUST(stream.read_value<LEB128<u32>>());
EXPECT_EQ(output, i);

MUST(stream->seek(0));
output_signed = MUST(stream->read_value<LEB128<i32>>());
MUST(stream.seek(0));
output_signed = MUST(stream.read_value<LEB128<i32>>());
EXPECT_EQ(output_signed, i);
}

// 0b0100_0000 to 0b0111_1111 unsigned == byte, signed = {{ 26'b(-1), 6'b(byte) }}
for (u8 i = 0x40u; i < 0x80; ++i) {
buf[0] = i;

MUST(stream->seek(0));
output = MUST(stream->read_value<LEB128<u32>>());
MUST(stream.seek(0));
output = MUST(stream.read_value<LEB128<u32>>());
EXPECT_EQ(output, i);

MUST(stream->seek(0));
output_signed = MUST(stream->read_value<LEB128<i32>>());
MUST(stream.seek(0));
output_signed = MUST(stream.read_value<LEB128<i32>>());
EXPECT_EQ(output_signed, (i | (-1 & (~0x3F))));
}
// MSB set, but input too short
for (u16 i = 0x80; i <= 0xFF; ++i) {
buf[0] = static_cast<u8>(i);

MUST(stream->seek(0));
EXPECT(stream->read_value<LEB128<u32>>().is_error());
MUST(stream.seek(0));
EXPECT(stream.read_value<LEB128<u32>>().is_error());

MUST(stream->seek(0));
EXPECT(stream->read_value<LEB128<i32>>().is_error());
MUST(stream.seek(0));
EXPECT(stream.read_value<LEB128<i32>>().is_error());
}
}

Expand All @@ -58,7 +58,7 @@ TEST_CASE(two_bytes)
u32 output = {};
i32 output_signed = {};
u8 buf[] = { 0x00, 0x1 };
auto stream = MUST(FixedMemoryStream::construct(ReadonlyBytes { buf, sizeof(buf) }));
FixedMemoryStream stream { ReadonlyBytes { buf, sizeof(buf) } };

// Only test with first byte expecting more, otherwise equivalent to single byte case
for (u16 i = 0x80; i <= 0xFF; ++i) {
Expand All @@ -68,37 +68,37 @@ TEST_CASE(two_bytes)
for (u8 j = 0u; j <= 0x3F; ++j) {
buf[1] = j;

MUST(stream->seek(0));
output = MUST(stream->read_value<LEB128<u32>>());
MUST(stream.seek(0));
output = MUST(stream.read_value<LEB128<u32>>());
EXPECT_EQ(output, (static_cast<u32>(j) << 7) + (i & 0x7F));

MUST(stream->seek(0));
output_signed = MUST(stream->read_value<LEB128<i32>>());
MUST(stream.seek(0));
output_signed = MUST(stream.read_value<LEB128<i32>>());
EXPECT_EQ(output_signed, (static_cast<i32>(j) << 7) + (i & 0x7F));
}

// 0b0100_0000 to 0b0111_1111: unsigned == (j << 7) + (7 MSB of i), signed == {{ 19'b(-1), 6'b(j), 7'b(i) }}
for (u8 j = 0x40u; j < 0x80; ++j) {
buf[1] = j;

MUST(stream->seek(0));
output = MUST(stream->read_value<LEB128<u32>>());
MUST(stream.seek(0));
output = MUST(stream.read_value<LEB128<u32>>());
EXPECT_EQ(output, (static_cast<u32>(j) << 7) + (i & 0x7F));

MUST(stream->seek(0));
output_signed = MUST(stream->read_value<LEB128<i32>>());
MUST(stream.seek(0));
output_signed = MUST(stream.read_value<LEB128<i32>>());
EXPECT_EQ(output_signed, ((static_cast<i32>(j) << 7) + (i & 0x7F)) | (-1 & (~0x3FFF)));
}

// MSB set on last byte, but input too short
for (u16 j = 0x80; j <= 0xFF; ++j) {
buf[1] = static_cast<u8>(j);

MUST(stream->seek(0));
EXPECT(stream->read_value<LEB128<u32>>().is_error());
MUST(stream.seek(0));
EXPECT(stream.read_value<LEB128<u32>>().is_error());

MUST(stream->seek(0));
EXPECT(stream->read_value<LEB128<i32>>().is_error());
MUST(stream.seek(0));
EXPECT(stream.read_value<LEB128<i32>>().is_error());
}
}
}
Expand All @@ -107,22 +107,22 @@ TEST_CASE(overflow_sizeof_output_unsigned)
{
u8 u32_max_plus_one[] = { 0x80, 0x80, 0x80, 0x80, 0x10 };
{
auto stream = MUST(FixedMemoryStream::construct(ReadonlyBytes { u32_max_plus_one, sizeof(u32_max_plus_one) }));
EXPECT(stream->read_value<LEB128<u32>>().is_error());
FixedMemoryStream stream { ReadonlyBytes { u32_max_plus_one, sizeof(u32_max_plus_one) } };
EXPECT(stream.read_value<LEB128<u32>>().is_error());

MUST(stream->seek(0));
u64 out64 = MUST(stream->read_value<LEB128<u64>>());
MUST(stream.seek(0));
u64 out64 = MUST(stream.read_value<LEB128<u64>>());
EXPECT_EQ(out64, static_cast<u64>(NumericLimits<u32>::max()) + 1);
}

u8 u32_max[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0x0F };
{
auto stream = MUST(FixedMemoryStream::construct(ReadonlyBytes { u32_max, sizeof(u32_max) }));
u32 out = MUST(stream->read_value<LEB128<u32>>());
FixedMemoryStream stream { ReadonlyBytes { u32_max, sizeof(u32_max) } };
u32 out = MUST(stream.read_value<LEB128<u32>>());
EXPECT_EQ(out, NumericLimits<u32>::max());

MUST(stream->seek(0));
u64 out64 = MUST(stream->read_value<LEB128<u64>>());
MUST(stream.seek(0));
u64 out64 = MUST(stream.read_value<LEB128<u64>>());
EXPECT_EQ(out64, NumericLimits<u32>::max());
}
}
Expand All @@ -131,43 +131,43 @@ TEST_CASE(overflow_sizeof_output_signed)
{
u8 i32_max_plus_one[] = { 0x80, 0x80, 0x80, 0x80, 0x08 };
{
auto stream = MUST(FixedMemoryStream::construct(ReadonlyBytes { i32_max_plus_one, sizeof(i32_max_plus_one) }));
EXPECT(stream->read_value<LEB128<i32>>().is_error());
FixedMemoryStream stream { ReadonlyBytes { i32_max_plus_one, sizeof(i32_max_plus_one) } };
EXPECT(stream.read_value<LEB128<i32>>().is_error());

MUST(stream->seek(0));
i64 out64 = MUST(stream->read_value<LEB128<i64>>());
MUST(stream.seek(0));
i64 out64 = MUST(stream.read_value<LEB128<i64>>());
EXPECT_EQ(out64, static_cast<i64>(NumericLimits<i32>::max()) + 1);
}

u8 i32_max[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0x07 };
{
auto stream = MUST(FixedMemoryStream::construct(ReadonlyBytes { i32_max, sizeof(i32_max) }));
i32 out = MUST(stream->read_value<LEB128<i32>>());
FixedMemoryStream stream { ReadonlyBytes { i32_max, sizeof(i32_max) } };
i32 out = MUST(stream.read_value<LEB128<i32>>());
EXPECT_EQ(out, NumericLimits<i32>::max());

MUST(stream->seek(0));
i64 out64 = MUST(stream->read_value<LEB128<i64>>());
MUST(stream.seek(0));
i64 out64 = MUST(stream.read_value<LEB128<i64>>());
EXPECT_EQ(out64, NumericLimits<i32>::max());
}

u8 i32_min_minus_one[] = { 0xFF, 0xFF, 0xFF, 0xFF, 0x77 };
{
auto stream = MUST(FixedMemoryStream::construct(ReadonlyBytes { i32_min_minus_one, sizeof(i32_min_minus_one) }));
EXPECT(stream->read_value<LEB128<i32>>().is_error());
FixedMemoryStream stream { ReadonlyBytes { i32_min_minus_one, sizeof(i32_min_minus_one) } };
EXPECT(stream.read_value<LEB128<i32>>().is_error());

MUST(stream->seek(0));
i64 out64 = MUST(stream->read_value<LEB128<i64>>());
MUST(stream.seek(0));
i64 out64 = MUST(stream.read_value<LEB128<i64>>());
EXPECT_EQ(out64, static_cast<i64>(NumericLimits<i32>::min()) - 1);
}

u8 i32_min[] = { 0x80, 0x80, 0x80, 0x80, 0x78 };
{
auto stream = MUST(FixedMemoryStream::construct(ReadonlyBytes { i32_min, sizeof(i32_min) }));
i32 out = MUST(stream->read_value<LEB128<i32>>());
FixedMemoryStream stream { ReadonlyBytes { i32_min, sizeof(i32_min) } };
i32 out = MUST(stream.read_value<LEB128<i32>>());
EXPECT_EQ(out, NumericLimits<i32>::min());

MUST(stream->seek(0));
i64 out64 = MUST(stream->read_value<LEB128<i64>>());
MUST(stream.seek(0));
i64 out64 = MUST(stream.read_value<LEB128<i64>>());
EXPECT_EQ(out64, NumericLimits<i32>::min());
}
}
4 changes: 2 additions & 2 deletions Tests/LibCompress/TestDeflate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ TEST_CASE(canonical_code_simple)
};

auto const huffman = Compress::CanonicalCode::from_bytes(code).value();
auto memory_stream = MUST(FixedMemoryStream::construct(input));
auto memory_stream = MUST(try_make<FixedMemoryStream>(input));
LittleEndianInputBitStream bit_stream { move(memory_stream) };

for (size_t idx = 0; idx < 9; ++idx)
Expand All @@ -48,7 +48,7 @@ TEST_CASE(canonical_code_complex)
};

auto const huffman = Compress::CanonicalCode::from_bytes(code).value();
auto memory_stream = MUST(FixedMemoryStream::construct(input));
auto memory_stream = MUST(try_make<FixedMemoryStream>(input));
LittleEndianInputBitStream bit_stream { move(memory_stream) };

for (size_t idx = 0; idx < 12; ++idx)
Expand Down
4 changes: 2 additions & 2 deletions Tests/LibWasm/test-wasm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ TESTJS_GLOBAL_FUNCTION(parse_webassembly_module, parseWebAssemblyModule)
if (!is<JS::Uint8Array>(object))
return vm.throw_completion<JS::TypeError>("Expected a Uint8Array argument to parse_webassembly_module");
auto& array = static_cast<JS::Uint8Array&>(*object);
auto stream = FixedMemoryStream::construct(array.data()).release_value_but_fixme_should_propagate_errors();
auto result = Wasm::Module::parse(*stream);
FixedMemoryStream stream { array.data() };
auto result = Wasm::Module::parse(stream);
if (result.is_error())
return vm.throw_completion<JS::SyntaxError>(Wasm::parse_error_to_deprecated_string(result.error()));

Expand Down
Loading

0 comments on commit 220fbca

Please sign in to comment.