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

@@ -8,7 +8,6 @@
 
 #include <AK/StringBuilder.h>
 #include <LibCore/ConfigFile.h>
-#include <LibCore/File.h>
 #include <LibCore/StandardPaths.h>
 #include <pwd.h>
 
@@ -36,27 +35,37 @@ ErrorOr<NonnullRefPtr<ConfigFile>> ConfigFile::open_for_system(String const& app
 
 ErrorOr<NonnullRefPtr<ConfigFile>> ConfigFile::open(String const& filename, AllowWriting allow_altering)
 {
-    auto file = File::construct(filename);
+    auto maybe_file = Stream::File::open(filename, allow_altering == AllowWriting::Yes ? Stream::OpenMode::ReadWrite : Stream::OpenMode::Read);
-    if (!file->open(allow_altering == AllowWriting::Yes ? OpenMode::ReadWrite : OpenMode::ReadOnly)) {
+    OwnPtr<Stream::BufferedFile> buffered_file;
-        // Failure to open a read-only file is OK, and behaves as if the file was empty.
+    if (maybe_file.is_error()) {
-        if (allow_altering == AllowWriting::Yes)
+        // If we attempted to open a read-only file that does not exist, we ignore the error, making it appear
-            return Error::from_string_literal("Unable to open config file");
+        // 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<NonnullRefPtr<ConfigFile>> ConfigFile::open(String const& filename, int fd)
 {
-    auto file = File::construct(filename);
+    auto file = TRY(Stream::File::adopt_fd(fd, Stream::OpenMode::ReadWrite));
-    if (!file->open(fd, OpenMode::ReadWrite, File::ShouldCloseFileDescriptor::Yes))
+    auto buffered_file = TRY(Stream::BufferedFile::create(move(file)));
-        return Error::from_string_literal("Unable to open config file");
+
-    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;
 }
 
-ConfigFile::ConfigFile(String const&, NonnullRefPtr<File> open_file)
+ConfigFile::ConfigFile(String const& filename, OwnPtr<Stream::BufferedFile> open_file)
-    : m_file(move(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<void> ConfigFile::reparse()
 {
     m_groups.clear();
+    if (!m_file)
+        return {};
 
     HashMap<String, String>* current_group = nullptr;
 
-    while (m_file->can_read_line()) {
+    auto buffer = TRY(ByteBuffer::create_uninitialized(4096));
-        auto line = m_file->read_line();
+    while (TRY(m_file->can_read_line())) {
-
+        auto length = TRY(m_file->read_line(buffer));
-        if (line.is_null()) {
-            m_groups.clear();
-            return;
-        }
 
+        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<void> ConfigFile::sync()
     if (!m_dirty)
         return {};
 
-    m_file->truncate(0);
+    if (!m_file)
-    m_file->seek(0);
+        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));
+            TRY(m_file->write(String::formatted("{}={}\n", jt.key, jt.value).bytes()));
-        m_file->write("\n");
+        TRY(m_file->write("\n"sv.bytes()));
     }
 
     m_dirty = false;

Userland/Libraries/LibCore/ConfigFile.h

@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <kling@serenityos.org>
  * Copyright (c) 2021, Jakob-Niklas See <git@nwex.de>
+ * Copyright (c) 2022, Sam Atkins <atkinssj@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -12,7 +13,7 @@
 #include <AK/RefPtr.h>
 #include <AK/String.h>
 #include <AK/Vector.h>
-#include <LibCore/File.h>
+#include <LibCore/Stream.h>
 #include <LibGfx/Color.h>
 
 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<File> open_file);
+    ConfigFile(String const& filename, OwnPtr<Stream::BufferedFile> open_file);
 
-    void reparse();
+    ErrorOr<void> reparse();
 
-    NonnullRefPtr<File> m_file;
+    String m_filename;
+    OwnPtr<Stream::BufferedFile> m_file;
     HashMap<String, HashMap<String, String>> m_groups;
     bool m_dirty { false };
 };