From e548aab387ecf9778fbb4b8393fb18f83b874c55 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Sun, 6 Feb 2022 15:27:17 +0000 Subject: [PATCH] LibCore: Migrate ConfigFile to Core::Stream API :^) As part of this, moved the call to `reparse()` out of the constructor and into the factory methods, to allow the error to propagate. --- Userland/Libraries/LibCore/ConfigFile.cpp | 69 ++++++++++++++--------- Userland/Libraries/LibCore/ConfigFile.h | 12 ++-- 2 files changed, 48 insertions(+), 33 deletions(-) diff --git a/Userland/Libraries/LibCore/ConfigFile.cpp b/Userland/Libraries/LibCore/ConfigFile.cpp index ec2fb3007b..5e494c8213 100644 --- a/Userland/Libraries/LibCore/ConfigFile.cpp +++ b/Userland/Libraries/LibCore/ConfigFile.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include @@ -36,27 +35,37 @@ ErrorOr> ConfigFile::open_for_system(String const& app ErrorOr> ConfigFile::open(String const& filename, AllowWriting allow_altering) { - auto file = File::construct(filename); - if (!file->open(allow_altering == AllowWriting::Yes ? OpenMode::ReadWrite : OpenMode::ReadOnly)) { - // Failure to open a read-only file is OK, and behaves as if the file was empty. - if (allow_altering == AllowWriting::Yes) - return Error::from_string_literal("Unable to open config file"); + auto maybe_file = Stream::File::open(filename, allow_altering == AllowWriting::Yes ? Stream::OpenMode::ReadWrite : Stream::OpenMode::Read); + OwnPtr buffered_file; + if (maybe_file.is_error()) { + // If we attempted to open a read-only file that does not exist, we ignore the error, making it appear + // the same as if we had opened an empty file. This behavior is a little weird, but is required by + // user code, which does not check the config file exists before opening. + if (!(allow_altering == AllowWriting::No && maybe_file.error().code() == ENOENT)) + return maybe_file.error(); + } else { + buffered_file = TRY(Stream::BufferedFile::create(maybe_file.release_value())); } - return adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(filename, move(file))); + + auto config_file = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(filename, move(buffered_file)))); + TRY(config_file->reparse()); + return config_file; } ErrorOr> ConfigFile::open(String const& filename, int fd) { - auto file = File::construct(filename); - if (!file->open(fd, OpenMode::ReadWrite, File::ShouldCloseFileDescriptor::Yes)) - return Error::from_string_literal("Unable to open config file"); - return adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(filename, move(file))); + auto file = TRY(Stream::File::adopt_fd(fd, Stream::OpenMode::ReadWrite)); + auto buffered_file = TRY(Stream::BufferedFile::create(move(file))); + + auto config_file = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(filename, move(buffered_file)))); + TRY(config_file->reparse()); + return config_file; } -ConfigFile::ConfigFile(String const&, NonnullRefPtr open_file) - : m_file(move(open_file)) +ConfigFile::ConfigFile(String const& filename, OwnPtr open_file) + : m_filename(filename) + , m_file(move(open_file)) { - reparse(); } ConfigFile::~ConfigFile() @@ -64,26 +73,24 @@ ConfigFile::~ConfigFile() MUST(sync()); } -void ConfigFile::reparse() +ErrorOr ConfigFile::reparse() { m_groups.clear(); + if (!m_file) + return {}; HashMap* current_group = nullptr; - while (m_file->can_read_line()) { - auto line = m_file->read_line(); - - if (line.is_null()) { - m_groups.clear(); - return; - } + 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 }; size_t i = 0; while (i < line.length() && (line[i] == ' ' || line[i] == '\t' || line[i] == '\n')) ++i; - // EOL if (i >= line.length()) continue; @@ -122,6 +129,7 @@ void ConfigFile::reparse() } } } + return {}; } String ConfigFile::read_entry(String const& group, String const& key, String const& default_value) const @@ -159,10 +167,12 @@ void ConfigFile::write_num_entry(String const& group, String const& key, int val { write_entry(group, key, String::number(value)); } + void ConfigFile::write_bool_entry(String const& group, String const& key, bool value) { write_entry(group, key, value ? "true" : "false"); } + void ConfigFile::write_color_entry(String const& group, String const& key, Color value) { write_entry(group, key, String::formatted("{},{},{},{}", value.red(), value.green(), value.blue(), value.alpha())); @@ -173,14 +183,17 @@ ErrorOr ConfigFile::sync() if (!m_dirty) return {}; - m_file->truncate(0); - m_file->seek(0); + if (!m_file) + return Error::from_errno(ENOENT); + + TRY(m_file->truncate(0)); + TRY(m_file->seek(0, Stream::SeekMode::SetPosition)); for (auto& it : m_groups) { - m_file->write(String::formatted("[{}]\n", it.key)); + TRY(m_file->write(String::formatted("[{}]\n", it.key).bytes())); for (auto& jt : it.value) - m_file->write(String::formatted("{}={}\n", jt.key, jt.value)); - m_file->write("\n"); + TRY(m_file->write(String::formatted("{}={}\n", jt.key, jt.value).bytes())); + TRY(m_file->write("\n"sv.bytes())); } m_dirty = false; diff --git a/Userland/Libraries/LibCore/ConfigFile.h b/Userland/Libraries/LibCore/ConfigFile.h index 14b149ff39..8e64d97b39 100644 --- a/Userland/Libraries/LibCore/ConfigFile.h +++ b/Userland/Libraries/LibCore/ConfigFile.h @@ -1,6 +1,7 @@ /* * Copyright (c) 2018-2020, Andreas Kling * Copyright (c) 2021, Jakob-Niklas See + * Copyright (c) 2022, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -12,7 +13,7 @@ #include #include #include -#include +#include #include namespace Core { @@ -57,14 +58,15 @@ public: void remove_group(String const& group); void remove_entry(String const& group, String const& key); - String filename() const { return m_file->filename(); } + String const& filename() const { return m_filename; } private: - ConfigFile(String const& filename, NonnullRefPtr open_file); + ConfigFile(String const& filename, OwnPtr open_file); - void reparse(); + ErrorOr reparse(); - NonnullRefPtr m_file; + String m_filename; + OwnPtr m_file; HashMap> m_groups; bool m_dirty { false }; };