Refactorings from libfud.HEADmaster

10 files changed, 112 insertions, 69 deletions

diff --git a/.gitignore b/.gitignore
index 88df23b..51769a9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,4 @@
 build/
 .cache
 *.kra
+*.user
diff --git a/src/archive.cpp b/src/archive.cpp
index b4bee29..f46f6a7 100644
--- a/src/archive.cpp
+++ b/src/archive.cpp
@@ -8,7 +8,7 @@ using fud::FudStatus;
 ZipArchive::ZipArchive(const QString& filename)
 {
     auto filenameUtf8 = filename.toUtf8();
-    int err;
+    int err{};
     m_archive = zip_open(filenameUtf8.data(), 0, &err);
 
     if (m_archive == nullptr) {
@@ -26,7 +26,7 @@ ZipArchive::ZipArchive(const QString& filename)
     }
 }
 
-ZipArchive::ZipArchive(ZipArchive&& rhs)
+ZipArchive::ZipArchive(ZipArchive&& rhs) noexcept
     :m_archive{rhs.m_archive},
      m_sortedIndices{std::move(rhs.m_sortedIndices)},
      m_filenames{std::move(rhs.m_filenames)},
@@ -41,7 +41,7 @@ ZipArchive::~ZipArchive()
     cleanup();
 }
 
-ZipArchive& ZipArchive::operator=(ZipArchive&& rhs)
+ZipArchive& ZipArchive::operator=(ZipArchive&& rhs) noexcept
 {
     cleanup();
 
@@ -78,7 +78,7 @@ fud::FudStatus ZipArchive::populate()
     if (numEntriesResult < 0) {
         return FudStatus::Failure;
     }
-    size_t numEntries = static_cast<size_t>(numEntriesResult);
+    auto numEntries = static_cast<size_t>(numEntriesResult);
 
     struct NameIndex {
         std::string name;
@@ -91,7 +91,8 @@ fud::FudStatus ZipArchive::populate()
     m_pages.reserve(numEntries);
 
     for (size_t idx = 0; idx < numEntries; ++idx) {
-        struct zip_stat stats;
+        using zipStat = struct zip_stat;
+        zipStat stats{};
 
         auto status = zip_stat_index(m_archive, idx, 0, &stats);
         if (status < 0) {
diff --git a/src/archive.hpp b/src/archive.hpp
index e803bd2..66059c6 100644
--- a/src/archive.hpp
+++ b/src/archive.hpp
@@ -35,10 +35,10 @@ class ZipArchive : public Archive {
   public:
     explicit ZipArchive(const QString& filename);
     ZipArchive(const ZipArchive&) = delete;
-    ZipArchive(ZipArchive&&);
+    ZipArchive(ZipArchive&&) noexcept;
     virtual ~ZipArchive() override;
     ZipArchive& operator=(const ZipArchive&) = delete;
-    ZipArchive& operator=(ZipArchive&& rhs);
+    ZipArchive& operator=(ZipArchive&& rhs) noexcept;
 
     virtual ArchiveResult getPage(size_t page) override;
     virtual size_t numPages() const override;
diff --git a/src/config.cpp b/src/config.cpp
index ab7d3ca..78f402b 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -2,19 +2,28 @@
 
 #include "luacxx.hpp"
 
+#include <QKeySequence>
 #include <algorithm>
 #include <filesystem>
-#include <fud_c_file.hpp>
+#include <fud_file.hpp>
 #include <fud_status.hpp>
 #include <fud_string.hpp>
+#include <fud_vector.hpp>
 #include <vector>
 
 namespace getsuyomi {
 
+using fud::BufferedRegularFile;
+using fud::FudError;
 using fud::FudStatus;
+using fud::RegularFile;
+using fud::Vector;
+constexpr auto FlagAppend = fud::OpenFlagEnum::Append;
+constexpr auto FlagTruncate = fud::OpenFlagEnum::Truncate;
 
 constexpr auto PermNone = fud::PermissionType::None;
 constexpr auto defaultPermissions = fud::Permissions(fud::PermReadWrite, PermNone, PermNone);
+constexpr auto AccessReadWrite = fud::FileAccessMode::ReadWrite;
 
 std::optional<std::string> getEnvVar(const char* envVar)
 {
@@ -115,20 +124,18 @@ void setUserConfig(GetsuyomiConfig& config, const ShortcutMap& shortcutMap)
 
 GetConfigResult getUserConfig()
 {
-    auto configResult = getEnvironment();
-    if (configResult.isError()) {
-        return GetConfigResult::error(configResult.getError());
-    }
-    auto config = configResult.getOkay();
+    auto config{M_TakeOrReturn(GetConfigResult, getEnvironment())};
 
     auto dirStatus = createXdgDirectory(config.dataHome);
     if (dirStatus != FudStatus::Success) {
         return GetConfigResult::error(dirStatus);
     }
+
     dirStatus = createXdgDirectory(config.configHome);
     if (dirStatus != FudStatus::Success) {
         return GetConfigResult::error(dirStatus);
     }
+
     dirStatus = createXdgDirectory(config.stateHome);
     if (dirStatus != FudStatus::Success) {
         return GetConfigResult::error(dirStatus);
@@ -199,9 +206,8 @@ std::optional<ActionType> Shortcuts::action(QKeySequence keySequence) const
 {
     if (m_shortcutToAction.contains(keySequence)) {
         return m_shortcutToAction.at(keySequence);
-    } else {
-        return std::nullopt;
     }
+    return std::nullopt;
 }
 
 std::optional<ShortcutList> Shortcuts::shortcuts(ActionType action) const
@@ -214,10 +220,10 @@ std::optional<ShortcutList> Shortcuts::shortcuts(ActionType action) const
             shortcutList.push_back(shortcut);
         }
         return shortcutList;
-    } else {
-        qWarning("Action %s not found", actionTypeToString(action));
-        return std::nullopt;
     }
+
+    qWarning("Action %s not found", actionTypeToString(action));
+    return std::nullopt;
 }
 
 FudStatus Shortcuts::bind(ActionType action, QKeySequence keySequence)
@@ -322,7 +328,7 @@ Shortcuts Shortcuts::fromLuaConfig(const std::filesystem::path& configFileName)
             continue;
         }
 
-        auto actionBindings = result.getOkay();
+        auto actionBindings{result.takeOkay()};
 
         for (const auto& shortcut : actionBindings) {
             QKeySequence keySeq{shortcut.c_str()};
@@ -344,17 +350,32 @@ Shortcuts Shortcuts::fromLuaConfig(const std::filesystem::path& configFileName)
     return shortcuts;
 }
 
-ShortcutMap Shortcuts::fromUserConfig(const std::filesystem::path& configFileName)
+auto getConfigFile(const std::filesystem::path& configFileName, fud::FileAccessMode mode, fud::OpenFlags flags)
+    -> fud::Result<RegularFile, FudStatus>
 {
-    fud::String filepath(configFileName.c_str());
-    fud::CTextFile file{filepath, fud::CFileMode::ReadWriteAppend};
-    auto status = file.create(defaultPermissions);
-    if (status != FudStatus::Success) {
-        if (errno != EEXIST) {
-            qCritical("Could not load or create user config file %s.", configFileName.c_str());
-        }
+    auto nameViewLength = fud::cStringLength(configFileName.c_str());
+    if (nameViewLength < 0 || nameViewLength > std::numeric_limits<ssize_t>::max()) {
+        qCritical("Failed to get filename length");
+        return FudError{FudStatus::ArgumentInvalid};
+    }
+    fud::StringView nameView{static_cast<size_t>(nameViewLength), configFileName.c_str()};
+    return RegularFile::create(nameView, mode, flags, defaultPermissions, false, fud::NullOpt);
+}
+
+auto createConfigFile(const std::filesystem::path& configFileName, fud::OpenFlags flags)
+    -> fud::Result<RegularFile, FudStatus>
+{
+    auto fileResult = getConfigFile(configFileName, AccessReadWrite, flags);
+    if (fileResult.isError()) {
+        qCritical("Failed to construct file object: %s", FudStatusToString(fileResult.getError()));
+        return FudError{fileResult.getError()};
     }
+    return fileResult;
+}
 
+ShortcutMap Shortcuts::fromUserConfig(const std::filesystem::path& configFileName)
+{
+    static_cast<void>(createConfigFile(configFileName, FlagAppend));
     auto shortcuts = Shortcuts::fromLuaConfig(configFileName);
 
     auto binder = [&](ActionType action, QKeySequence keySeq) {
@@ -386,15 +407,27 @@ ShortcutMap Shortcuts::fromUserConfig(const std::filesystem::path& configFileNam
     return shortcuts.shortcutMap();
 }
 
-FudStatus backupFile(const fud::String& filepath)
+FudStatus backupFile(const std::filesystem::path& path)
 {
-    fud::CTextFile file{filepath, fud::CFileMode::ReadWriteAppend};
-    auto status = file.create(defaultPermissions);
-    if (status != FudStatus::Success) {
-        if (errno != EEXIST) {
-            qCritical("Could not load or create user config file %s.", filepath.c_str());
-        }
+    auto filepathResult{fud::String::makeFromCString(path.c_str())};
+    if (filepathResult.isError()) {
+        qCritical("Failed to construct filename string: %s", FudStatusToString(filepathResult.getError()));
+        return filepathResult.getError();
     }
+    auto filepath{filepathResult.takeOkay()};
+    auto backupFileNameResult{filepath.catenate(".bak")};
+    if (backupFileNameResult.isError()) {
+        qWarning("Could not create backup file name %s", FudStatusToString(backupFileNameResult.getError()));
+        return backupFileNameResult.getError();
+    }
+    auto backupFileName{backupFileNameResult.takeOkay()};
+
+    auto originalFileResult{createConfigFile(path, FlagAppend)};
+    if (originalFileResult.isError()) {
+        auto fileStatus = originalFileResult.getError();
+        return fileStatus;
+    }
+    auto file{originalFileResult.takeOkay()};
 
     size_t fileSize;
     auto fileSizeStatus = file.size();
@@ -409,20 +442,25 @@ FudStatus backupFile(const fud::String& filepath)
         return FudStatus::Empty;
     }
 
-    std::vector<char> backupFileData(fileSize);
+    auto vectorResult{Vector<std::byte>::withSize(fileSize)};
+    if (vectorResult.isError()) {
+        qCritical("Could not create vector for backup data");
+        return vectorResult.getError();
+    }
+    auto backupFileData(vectorResult.takeOkay());
     auto readStatus = file.read(backupFileData.data(), backupFileData.size(), fileSize);
     if (readStatus.status != FudStatus::Success) {
         qWarning("Could not read existing config");
         return readStatus.status;
     }
 
-    auto backupFileName{filepath.catenate(".bak")};
-    fud::CTextFile backupFile{backupFileName, fud::CFileMode::ReadWriteTruncate};
-    status = backupFile.create(defaultPermissions);
-    if (status != FudStatus::Success) {
-        qWarning("Could not create or open backup file %s", backupFileName.c_str());
-        return status;
+    auto backupFileResult{createConfigFile(path, FlagTruncate)};
+    if (backupFileResult.isError()) {
+        auto fileStatus = backupFileResult.getError();
+        qWarning("Could not create backup file object %s", FudStatusToString(backupFileNameResult.getError()));
+        return fileStatus;
     }
+    auto backupFile{backupFileResult.takeOkay()};
 
     auto writeResult = backupFile.write(backupFileData.data(), backupFileData.size());
     if (writeResult.status != FudStatus::Success) {
@@ -434,16 +472,14 @@ FudStatus backupFile(const fud::String& filepath)
 
 FudStatus Shortcuts::save(const std::filesystem::path& configFileName) const
 {
-    fud::String filepath(configFileName.c_str());
-    static_cast<void>(backupFile(filepath));
-
-    fud::CTextFile file{filepath, fud::CFileMode::ReadWriteTruncate};
-    auto status = file.create(defaultPermissions);
-    if (status != FudStatus::Success) {
-        if (errno != EEXIST) {
-            qCritical("Could not load or create user config file %s.", configFileName.c_str());
-        }
+    static_cast<void>(backupFile(configFileName));
+
+    auto fileResult = getConfigFile(configFileName, AccessReadWrite, FlagTruncate);
+    if (fileResult.isError()) {
+        qCritical("Failed to construct file object: %s", FudStatusToString(fileResult.getError()));
+        return fileResult.getError();
     }
+    auto file{fileResult.takeOkay()};
 
     fud::String output{};
     constexpr size_t defaultFileSize = 4096;
@@ -489,7 +525,7 @@ FudStatus Shortcuts::save(const std::filesystem::path& configFileName) const
         }
     }
 
-    auto writeStatus = file.write(output.data(), output.length());
+    auto writeStatus = file.write(reinterpret_cast<const std::byte*>(output.data()), output.length());
     if (writeStatus.status != FudStatus::Success) {
         qCritical("Could not save user config file %s.", configFileName.c_str());
     }
diff --git a/src/config.hpp b/src/config.hpp
index bc5698e..144efdc 100644
--- a/src/config.hpp
+++ b/src/config.hpp
@@ -1,15 +1,16 @@
 #ifndef GETSUYOMI_CONFIG_HPP
 #define GETSUYOMI_CONFIG_HPP
 
+#include <filesystem>
 #include <fud_result.hpp>
 #include <fud_status.hpp>
 #include <map>
 #include <optional>
-#include <qkeysequence.h>
-#include <qlist.h>
 #include <set>
 #include <string>
-#include <filesystem>
+#include <qlist.h>
+
+class QKeySequence;
 
 namespace getsuyomi {
 
@@ -94,7 +95,7 @@ constexpr const char* actionTypeToString(ActionType action)
 }
 
 ShortcutSet shortcutSetFromList(const ShortcutList& shortcutList);
-ShortcutList shortcutListFromSet(const ShortcutSet& shortcutList);
+ShortcutList shortcutListFromSet(const ShortcutSet& shortcutSet);
 
 class Shortcuts {
   public:
diff --git a/src/fud_mem.cpp b/src/fud_mem.cpp
index d8f6673..ffcf6fd 100644
--- a/src/fud_mem.cpp
+++ b/src/fud_mem.cpp
@@ -4,6 +4,8 @@
 
 namespace fud {
 
+// nolintbegin(cppcoreguidelines-no-malloc)
+
 void* fudAlloc(size_t size) {
     return malloc(size);
 }
@@ -12,8 +14,10 @@ void* fudRealloc(void* ptr, size_t size) {
     return realloc(ptr, size);
 }
 
-void fudFree(void* ptr) {
-    return free(ptr);
+void fudFree(std::byte* ptr) {
+    free(ptr);
 }
 
+// nolintend(cppcoreguidelines-no-malloc)
+
 } // namespace fud
diff --git a/src/luacxx.cpp b/src/luacxx.cpp
index 0a6db82..c76bddc 100644
--- a/src/luacxx.cpp
+++ b/src/luacxx.cpp
@@ -9,9 +9,8 @@ namespace getsuyomi {
 
 using fud::FudStatus;
 
-LuaContext::LuaContext()
+LuaContext::LuaContext() : m_state{luaL_newstate()}
 {
-    m_state = luaL_newstate();
     if (m_state != nullptr) {
         luaL_openlibs(m_state);
     }
@@ -25,12 +24,12 @@ LuaContext::~LuaContext()
     }
 }
 
-LuaContext::LuaContext(LuaContext&& rhs) : m_state{rhs.m_state}
+LuaContext::LuaContext(LuaContext&& rhs) noexcept : m_state{rhs.m_state}
 {
     rhs.m_state = nullptr;
 }
 
-LuaContext& LuaContext::operator=(LuaContext&& rhs)
+LuaContext& LuaContext::operator=(LuaContext&& rhs) noexcept
 {
     if (m_state != nullptr) {
         lua_close(m_state);
@@ -83,7 +82,7 @@ LuaResult<const char*> LuaContext::getGlobalString(const char* name)
         return LuaResult<const char*>::error(FudStatus::ObjectInvalid);
     }
 
-    size_t length;
+    size_t length{};
     const char* result{nullptr};
 
     auto luaType = lua_getglobal(m_state, name);
@@ -114,10 +113,10 @@ LuaResult<std::vector<std::string>> LuaContext::getGlobalStringArray(const char*
 
     if (luaType != LUA_TTABLE) {
         lua_pop(m_state, 1);
-        return RetType::error(FudStatus::InvalidInput);
+        return RetType::error(FudStatus::ArgumentInvalid);
     }
 
-    int64_t length;
+    int64_t length{};
     int isNumber{};
     lua_len(m_state, -1);
     length = lua_tointegerx(m_state, -1, &isNumber);
diff --git a/src/luacxx.hpp b/src/luacxx.hpp
index 388f640..fb7c1b9 100644
--- a/src/luacxx.hpp
+++ b/src/luacxx.hpp
@@ -23,9 +23,9 @@ public:
     LuaContext();
     ~LuaContext();
     LuaContext(const LuaContext&) = delete;
-    LuaContext(LuaContext&& rhs);
+    LuaContext(LuaContext&& rhs) noexcept;
     LuaContext& operator=(const LuaContext&) = delete;
-    LuaContext& operator=(LuaContext&& rhs);
+    LuaContext& operator=(LuaContext&& rhs) noexcept;
 
     fud::FudStatus loadFile(const char* filename);
 
diff --git a/src/settings.cpp b/src/settings.cpp
index abd53c1..9f643b7 100644
--- a/src/settings.cpp
+++ b/src/settings.cpp
@@ -142,7 +142,7 @@ void ShortcutCollector::removeBinding(QKeySequence binding)
     auto bindingHandle = m_bindings.extract(binding);
     if (bindingHandle) {
         m_layout->removeWidget(bindingHandle.mapped());
-        delete bindingHandle.mapped();
+        bindingHandle.mapped()->deleteLater();
     } else {
         qWarning("Could not remove widget!");
     }
diff --git a/warnings.cmake b/warnings.cmake
index 7228542..df3b23b 100644
--- a/warnings.cmake
+++ b/warnings.cmake
@@ -1,5 +1,8 @@
 set(GETSUYOMI_WARNINGS
     -Werror
+    # -pedantic
+    # -pedantic-errors
+    -Wno-pedantic
     -Wstack-usage=2048
     -Wno-long-long
     -Wno-error=long-long
@@ -7,9 +10,7 @@ set(GETSUYOMI_WARNINGS
     -Wno-error=mismatched-tags
     -Wall
     -Weffc++
-    -pedantic
     -Wsizeof-pointer-memaccess
-    -pedantic-errors
     -Wextra
     -Wpacked
     -Wshadow