From b90dc408bdd249ca04f10cf0d288e6f8549e9832 Mon Sep 17 00:00:00 2001
From: Sam Atkins <atkinssj@serenityos.org>
Date: Sun, 6 Feb 2022 13:58:29 +0000
Subject: [PATCH] LibCore: Create ConfigFiles with an already-open File

This moves the fallible action of opening the file, from the
constructor, into the factory methods which can propagate any errors.

The wrinkle here is that failure to open a ConfigFile in read-only mode
is allowed (and expected, since the file may not exist), and treated as
if an empty file was successfully opened.
---
 Userland/Libraries/LibCore/ConfigFile.cpp | 36 ++++++++++-------------
 Userland/Libraries/LibCore/ConfigFile.h   |  3 +-
 2 files changed, 17 insertions(+), 22 deletions(-)

diff --git a/Userland/Libraries/LibCore/ConfigFile.cpp b/Userland/Libraries/LibCore/ConfigFile.cpp
index 3ca378cc02..78a75a398b 100644
--- a/Userland/Libraries/LibCore/ConfigFile.cpp
+++ b/Userland/Libraries/LibCore/ConfigFile.cpp
@@ -18,48 +18,44 @@ ErrorOr<NonnullRefPtr<ConfigFile>> ConfigFile::open_for_lib(String const& lib_na
 {
     String directory = StandardPaths::config_directory();
     auto path = String::formatted("{}/lib/{}.ini", directory, lib_name);
-
-    return adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(path, allow_altering));
+    return ConfigFile::open(path, allow_altering);
 }
 
 ErrorOr<NonnullRefPtr<ConfigFile>> ConfigFile::open_for_app(String const& app_name, AllowWriting allow_altering)
 {
     String directory = StandardPaths::config_directory();
     auto path = String::formatted("{}/{}.ini", directory, app_name);
-    return adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(path, allow_altering));
+    return ConfigFile::open(path, allow_altering);
 }
 
 ErrorOr<NonnullRefPtr<ConfigFile>> ConfigFile::open_for_system(String const& app_name, AllowWriting allow_altering)
 {
     auto path = String::formatted("/etc/{}.ini", app_name);
-    return adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(path, allow_altering));
+    return ConfigFile::open(path, allow_altering);
 }
 
 ErrorOr<NonnullRefPtr<ConfigFile>> ConfigFile::open(String const& filename, AllowWriting allow_altering)
 {
-    return adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(filename, 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");
+    }
+    return adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(filename, move(file)));
 }
 
 ErrorOr<NonnullRefPtr<ConfigFile>> ConfigFile::open(String const& filename, int fd)
 {
-    return adopt_nonnull_ref_or_enomem(new (nothrow) ConfigFile(filename, 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)));
 }
 
-ConfigFile::ConfigFile(String const& filename, AllowWriting allow_altering)
-    : m_file(File::construct(filename))
+ConfigFile::ConfigFile(String const&, NonnullRefPtr<File> open_file)
+    : m_file(move(open_file))
 {
-    if (!m_file->open(allow_altering == AllowWriting::Yes ? OpenMode::ReadWrite : OpenMode::ReadOnly))
-        return;
-
-    reparse();
-}
-
-ConfigFile::ConfigFile(String const& filename, int fd)
-    : m_file(File::construct(filename))
-{
-    if (!m_file->open(fd, OpenMode::ReadWrite, File::ShouldCloseFileDescriptor::Yes))
-        return;
-
     reparse();
 }
 
diff --git a/Userland/Libraries/LibCore/ConfigFile.h b/Userland/Libraries/LibCore/ConfigFile.h
index ff3de3bb7b..00da8d8346 100644
--- a/Userland/Libraries/LibCore/ConfigFile.h
+++ b/Userland/Libraries/LibCore/ConfigFile.h
@@ -60,8 +60,7 @@ public:
     String filename() const { return m_file->filename(); }
 
 private:
-    explicit ConfigFile(String const& filename, AllowWriting);
-    explicit ConfigFile(String const& filename, int fd);
+    ConfigFile(String const& filename, NonnullRefPtr<File> open_file);
 
     void reparse();