AK: Use an enum to specify the open mode instead of a bool

Let's replace this bool with an `enum class` in order to enhance
readability. This is done by repurposing `MappedFile`'s `OpenMode` into
a shared `enum` simply called `Mode`.
This commit is contained in:
Lucas CHOLLET 2023-11-05 15:31:17 -05:00 committed by Tim Schumacher
parent 00ad8419cf
commit b00476abac
Notes: sideshowbarker 2024-07-17 06:45:52 +09:00
7 changed files with 24 additions and 25 deletions

View file

@ -12,9 +12,9 @@
namespace AK { namespace AK {
FixedMemoryStream::FixedMemoryStream(Bytes bytes, bool writing_enabled) FixedMemoryStream::FixedMemoryStream(Bytes bytes, Mode mode)
: m_bytes(bytes) : m_bytes(bytes)
, m_writing_enabled(writing_enabled) , m_writing_enabled(mode == Mode::ReadWrite)
{ {
} }

View file

@ -18,7 +18,12 @@ namespace AK {
/// using a single read/write head. /// using a single read/write head.
class FixedMemoryStream : public SeekableStream { class FixedMemoryStream : public SeekableStream {
public: public:
explicit FixedMemoryStream(Bytes bytes, bool writing_enabled = true); enum class Mode {
ReadOnly,
ReadWrite,
};
explicit FixedMemoryStream(Bytes bytes, Mode mode = Mode::ReadWrite);
explicit FixedMemoryStream(ReadonlyBytes bytes); explicit FixedMemoryStream(ReadonlyBytes bytes);
virtual bool is_eof() const override; virtual bool is_eof() const override;

View file

@ -259,7 +259,7 @@ TEST_CASE(fixed_memory_read_in_place)
EXPECT_EQ(characters, some_words.bytes()); EXPECT_EQ(characters, some_words.bytes());
EXPECT(readonly_stream.is_eof()); EXPECT(readonly_stream.is_eof());
FixedMemoryStream mutable_stream { Bytes { const_cast<u8*>(some_words.bytes().data()), some_words.bytes().size() }, true }; FixedMemoryStream mutable_stream { Bytes { const_cast<u8*>(some_words.bytes().data()), some_words.bytes().size() }, FixedMemoryStream::Mode::ReadWrite };
// Trying to read mutable values from a mutable stream should succeed. // Trying to read mutable values from a mutable stream should succeed.
TRY_OR_FAIL(mutable_stream.read_in_place<u8>(1)); TRY_OR_FAIL(mutable_stream.read_in_place<u8>(1));
EXPECT_EQ(mutable_stream.offset(), 1u); EXPECT_EQ(mutable_stream.offset(), 1u);

View file

@ -27,7 +27,7 @@ TEST_CASE(mapped_file_open)
TRY_OR_FAIL(maybe_file.value()->write_until_depleted(text.bytes())); TRY_OR_FAIL(maybe_file.value()->write_until_depleted(text.bytes()));
} }
auto maybe_file = Core::MappedFile::map("/tmp/file-open-test.txt"sv, Core::MappedFile::OpenMode::ReadWrite); auto maybe_file = Core::MappedFile::map("/tmp/file-open-test.txt"sv, Core::MappedFile::Mode::ReadWrite);
if (maybe_file.is_error()) { if (maybe_file.is_error()) {
warnln("Failed to open the file: {}", strerror(maybe_file.error().code())); warnln("Failed to open the file: {}", strerror(maybe_file.error().code()));
VERIFY_NOT_REACHED(); VERIFY_NOT_REACHED();
@ -46,7 +46,7 @@ constexpr auto expected_buffer_contents = "&lt;small&gt;(Please consider transla
TEST_CASE(mapped_file_read_bytes) TEST_CASE(mapped_file_read_bytes)
{ {
auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/long_lines.txt"sv, Core::MappedFile::OpenMode::ReadOnly)); auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/long_lines.txt"sv, Core::MappedFile::Mode::ReadOnly));
auto buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(131)); auto buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(131));
@ -63,7 +63,7 @@ constexpr auto expected_seek_contents3 = "levels of advanc"sv;
TEST_CASE(mapped_file_seeking_around) TEST_CASE(mapped_file_seeking_around)
{ {
auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/long_lines.txt"sv, Core::MappedFile::OpenMode::ReadOnly)); auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/long_lines.txt"sv, Core::MappedFile::Mode::ReadOnly));
EXPECT_EQ(file->size().release_value(), 8702ul); EXPECT_EQ(file->size().release_value(), 8702ul);
@ -89,7 +89,7 @@ TEST_CASE(mapped_file_seeking_around)
BENCHMARK_CASE(file_tell) BENCHMARK_CASE(file_tell)
{ {
auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/10kb.txt"sv, Core::MappedFile::OpenMode::ReadOnly)); auto file = TRY_OR_FAIL(Core::MappedFile::map("/usr/Tests/LibCore/10kb.txt"sv, Core::MappedFile::Mode::ReadOnly));
auto expected_file_offset = 0u; auto expected_file_offset = 0u;
auto ten_byte_buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(1)); auto ten_byte_buffer = TRY_OR_FAIL(ByteBuffer::create_uninitialized(1));
for (auto i = 0u; i < 4000; ++i) { for (auto i = 0u; i < 4000; ++i) {

View file

@ -44,7 +44,7 @@ static constexpr LoaderPluginInitializer s_initializers[] = {
ErrorOr<NonnullRefPtr<Loader>, LoaderError> Loader::create(StringView path) ErrorOr<NonnullRefPtr<Loader>, LoaderError> Loader::create(StringView path)
{ {
auto stream = TRY(Core::MappedFile::map(path, Core::MappedFile::OpenMode::ReadOnly)); auto stream = TRY(Core::MappedFile::map(path, Core::MappedFile::Mode::ReadOnly));
return adopt_ref(*new (nothrow) Loader(TRY(Loader::create_plugin(move(stream))))); return adopt_ref(*new (nothrow) Loader(TRY(Loader::create_plugin(move(stream)))));
} }

View file

@ -15,9 +15,9 @@
namespace Core { namespace Core {
ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map(StringView path, OpenMode mode) ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map(StringView path, Mode mode)
{ {
auto const file_mode = mode == OpenMode::ReadOnly ? O_RDONLY : O_RDWR; auto const file_mode = mode == Mode::ReadOnly ? O_RDONLY : O_RDWR;
auto fd = TRY(Core::System::open(path, file_mode | O_CLOEXEC, 0)); auto fd = TRY(Core::System::open(path, file_mode | O_CLOEXEC, 0));
return map_from_fd_and_close(fd, path, mode); return map_from_fd_and_close(fd, path, mode);
} }
@ -27,7 +27,7 @@ ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_file(NonnullOwnPtr<Core:
return map_from_fd_and_close(stream->leak_fd(Badge<MappedFile> {}), path); return map_from_fd_and_close(stream->leak_fd(Badge<MappedFile> {}), path);
} }
ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_fd_and_close(int fd, [[maybe_unused]] StringView path, OpenMode mode) ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_fd_and_close(int fd, [[maybe_unused]] StringView path, Mode mode)
{ {
TRY(Core::System::fcntl(fd, F_SETFD, FD_CLOEXEC)); TRY(Core::System::fcntl(fd, F_SETFD, FD_CLOEXEC));
@ -41,11 +41,11 @@ ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_fd_and_close(int fd, [[m
int protection; int protection;
int flags; int flags;
switch (mode) { switch (mode) {
case OpenMode::ReadOnly: case Mode::ReadOnly:
protection = PROT_READ; protection = PROT_READ;
flags = MAP_SHARED; flags = MAP_SHARED;
break; break;
case OpenMode::ReadWrite: case Mode::ReadWrite:
protection = PROT_READ | PROT_WRITE; protection = PROT_READ | PROT_WRITE;
// Don't map a read-write mapping shared as a precaution. // Don't map a read-write mapping shared as a precaution.
flags = MAP_PRIVATE; flags = MAP_PRIVATE;
@ -57,8 +57,8 @@ ErrorOr<NonnullOwnPtr<MappedFile>> MappedFile::map_from_fd_and_close(int fd, [[m
return adopt_own(*new MappedFile(ptr, size, mode)); return adopt_own(*new MappedFile(ptr, size, mode));
} }
MappedFile::MappedFile(void* ptr, size_t size, OpenMode mode) MappedFile::MappedFile(void* ptr, size_t size, Mode mode)
: FixedMemoryStream(Bytes { ptr, size }, mode == OpenMode::ReadWrite) : FixedMemoryStream(Bytes { ptr, size }, mode)
, m_data(ptr) , m_data(ptr)
, m_size(size) , m_size(size)
{ {

View file

@ -22,15 +22,9 @@ class MappedFile : public FixedMemoryStream {
AK_MAKE_NONMOVABLE(MappedFile); AK_MAKE_NONMOVABLE(MappedFile);
public: public:
// Reflects a simplified version of mmap protection and flags. static ErrorOr<NonnullOwnPtr<MappedFile>> map(StringView path, Mode mode = Mode::ReadOnly);
enum class OpenMode {
ReadOnly,
ReadWrite,
};
static ErrorOr<NonnullOwnPtr<MappedFile>> map(StringView path, OpenMode mode = OpenMode::ReadOnly);
static ErrorOr<NonnullOwnPtr<MappedFile>> map_from_file(NonnullOwnPtr<Core::File>, StringView path); static ErrorOr<NonnullOwnPtr<MappedFile>> map_from_file(NonnullOwnPtr<Core::File>, StringView path);
static ErrorOr<NonnullOwnPtr<MappedFile>> map_from_fd_and_close(int fd, StringView path, OpenMode mode = OpenMode::ReadOnly); static ErrorOr<NonnullOwnPtr<MappedFile>> map_from_fd_and_close(int fd, StringView path, Mode mode = Mode::ReadOnly);
virtual ~MappedFile(); virtual ~MappedFile();
// Non-stream APIs for using MappedFile as a simple POSIX API wrapper. // Non-stream APIs for using MappedFile as a simple POSIX API wrapper.
@ -39,7 +33,7 @@ public:
ReadonlyBytes bytes() const { return { m_data, m_size }; } ReadonlyBytes bytes() const { return { m_data, m_size }; }
private: private:
explicit MappedFile(void*, size_t, OpenMode); explicit MappedFile(void*, size_t, Mode);
void* m_data { nullptr }; void* m_data { nullptr };
size_t m_size { 0 }; size_t m_size { 0 };