[RAPPS] settings read/write refactor (#3101)

* [RAPPS] move settings-related stuff to a separate file
* [RAPPS] refactor reg read/write
* [RAPPS] add params for Load/SaveSettings, no longer use global vars

base/applications/rapps/CMakeLists.txt

@@ -17,6 +17,7 @@ list(APPEND SOURCE
     integrity.cpp
     loaddlg.cpp
     misc.cpp
+    settings.cpp
     settingsdlg.cpp
     winmain.cpp
     unattended.cpp
@@ -30,6 +31,7 @@ list(APPEND SOURCE
     include/crichedit.h
     include/defines.h
     include/misc.h
+    include/settings.h
     include/resource.h
     include/rosui.h
     include/winmain.h

base/applications/rapps/gui.cpp

@@ -316,7 +316,7 @@ BOOL CMainWindow::ProcessWindowMessage(HWND hwnd, UINT Msg, WPARAM wParam, LPARA
     case WM_DESTROY:
     {
         ShowWindow(SW_HIDE);
-        SaveSettings(hwnd);
+        SaveSettings(hwnd, &SettingsInfo);
 
         FreeLogs();
         m_AvailableApps.FreeCachedEntries();

base/applications/rapps/include/settings.h

@@ -0,0 +1,33 @@
+#pragma once
+#include <windef.h>
+#include <wininet.h>
+
+struct SETTINGS_INFO
+{
+    BOOL bSaveWndPos;
+    BOOL bUpdateAtStart;
+    BOOL bLogEnabled;
+    WCHAR szDownloadDir[MAX_PATH];
+    BOOL bDelInstaller;
+    /* Window Pos */
+    BOOL Maximized;
+    INT Left;
+    INT Top;
+    INT Width;
+    INT Height;
+    /* Proxy settings */
+    INT Proxy;
+    WCHAR szProxyServer[MAX_PATH];
+    WCHAR szNoProxyFor[MAX_PATH];
+    /* Software source settings */
+    BOOL bUseSource;
+    WCHAR szSourceURL[INTERNET_MAX_URL_LENGTH];
+};
+
+typedef SETTINGS_INFO *PSETTINGS_INFO;
+
+BOOL LoadSettings(PSETTINGS_INFO pSettingsInfo);
+BOOL SaveSettings(HWND hwnd, PSETTINGS_INFO pSettingsInfo);
+VOID FillDefaultSettings(PSETTINGS_INFO pSettingsInfo);
+
+extern SETTINGS_INFO SettingsInfo;

base/applications/rapps/include/winmain.h

@@ -1,40 +1,12 @@
 #pragma once
 #include <windef.h>
 #include <wininet.h>
+#include "settings.h"
 
 extern LPCWSTR szWindowClass;
 
-//TODO: Separate main and settings related definitions
-struct SETTINGS_INFO
-{
-    BOOL bSaveWndPos;
-    BOOL bUpdateAtStart;
-    BOOL bLogEnabled;
-    WCHAR szDownloadDir[MAX_PATH];
-    BOOL bDelInstaller;
-    /* Window Pos */
-    BOOL Maximized;
-    INT Left;
-    INT Top;
-    INT Width;
-    INT Height;
-    /* Proxy settings */
-    INT Proxy;
-    WCHAR szProxyServer[MAX_PATH];
-    WCHAR szNoProxyFor[MAX_PATH];
-    /* Software source settings */
-    BOOL bUseSource;
-    WCHAR szSourceURL[INTERNET_MAX_URL_LENGTH];
-};
-
-typedef SETTINGS_INFO *PSETTINGS_INFO;
-
 extern HWND hMainWnd;
 extern HINSTANCE hInst;
-extern SETTINGS_INFO SettingsInfo;
-
-VOID SaveSettings(HWND hwnd);
-VOID FillDefaultSettings(PSETTINGS_INFO pSettingsInfo);
 
 // integrity.cpp
 BOOL VerifyInteg(LPCWSTR lpSHA1Hash, LPCWSTR lpFileName);

base/applications/rapps/settings.cpp

@@ -0,0 +1,243 @@
+/*
+ * PROJECT:     ReactOS Applications Manager
+ * LICENSE:     GPL-2.0-or-later (https://spdx.org/licenses/GPL-2.0-or-later)
+ * PURPOSE:     Functions to load / save settings from reg.
+ * COPYRIGHT:   Copyright 2020 He Yang           (1160386205@qq.com)
+ */
+
+#include "rapps.h"
+#include "settings.h"
+
+
+class SettingsField
+{
+public:
+    virtual ~SettingsField() { ; }
+    virtual BOOL Save(CRegKey &key) = 0;
+    virtual BOOL Load(CRegKey &key) = 0;
+};
+
+class SettingsFieldBool : public SettingsField
+{
+public:
+    SettingsFieldBool(BOOL *pValue, LPCWSTR szRegName)
+        : m_pValueStore(pValue), m_RegName(szRegName)
+    {
+    }
+
+    virtual BOOL Save(CRegKey &key) override
+    {
+        return key.SetDWORDValue(m_RegName, (DWORD)(*m_pValueStore)) == ERROR_SUCCESS;
+    }
+    virtual BOOL Load(CRegKey &key) override
+    {
+        DWORD dwField;
+        LONG lResult = key.QueryDWORDValue(m_RegName, dwField);
+        if (lResult != ERROR_SUCCESS)
+        {
+            return FALSE;
+        }
+        *m_pValueStore = (BOOL)dwField;
+        return TRUE;
+    }
+
+private:
+    BOOL *m_pValueStore;     // where to read/store the value
+    LPCWSTR m_RegName;       // key name in registery
+};
+
+class SettingsFieldInt : public SettingsField
+{
+public:
+    SettingsFieldInt(INT *pValue, LPCWSTR szRegName)
+        : m_pValueStore(pValue), m_RegName(szRegName)
+    {
+    }
+
+    virtual BOOL Save(CRegKey &key) override
+    {
+        return key.SetDWORDValue(m_RegName, (DWORD)(*m_pValueStore)) == ERROR_SUCCESS;
+    }
+    virtual BOOL Load(CRegKey &key) override
+    {
+        DWORD dwField;
+        LONG lResult = key.QueryDWORDValue(m_RegName, dwField);
+        if (lResult != ERROR_SUCCESS)
+        {
+            return FALSE;
+        }
+        *m_pValueStore = (INT)dwField;
+        return TRUE;
+    }
+
+private:
+    INT *m_pValueStore;      // where to read/store the value
+    LPCWSTR m_RegName;       // key name in registery
+};
+
+class SettingsFieldString : public SettingsField
+{
+public:
+    SettingsFieldString(WCHAR *pString, ULONG cchLen, LPCWSTR szRegName)
+        : m_pStringStore(pString), m_StringLen(cchLen), m_RegName(szRegName)
+    {
+    }
+
+    virtual BOOL Save(CRegKey &key) override
+    {
+        return key.SetStringValue(m_RegName, m_pStringStore) == ERROR_SUCCESS;
+    }
+    virtual BOOL Load(CRegKey &key) override
+    {
+        ULONG nChar = m_StringLen - 1; // make sure the terminating L'\0'
+        LONG lResult = key.QueryStringValue(m_RegName, m_pStringStore, &nChar);
+        return lResult == ERROR_SUCCESS;
+    }
+
+private:
+    WCHAR *m_pStringStore;        // where to read/store the value
+    ULONG m_StringLen;            // string length, in chars
+    LPCWSTR m_RegName;            // key name in registery
+};
+
+
+void AddInfoFields(ATL::CAtlList<SettingsField *> &infoFields, SETTINGS_INFO &settings)
+{
+    infoFields.AddTail(new SettingsFieldBool(&(settings.bSaveWndPos), L"bSaveWndPos"));
+    infoFields.AddTail(new SettingsFieldBool(&(settings.bUpdateAtStart), L"bUpdateAtStart"));
+    infoFields.AddTail(new SettingsFieldBool(&(settings.bLogEnabled), L"bLogEnabled"));
+    infoFields.AddTail(new SettingsFieldString(settings.szDownloadDir, MAX_PATH, L"szDownloadDir"));
+    infoFields.AddTail(new SettingsFieldBool(&(settings.bDelInstaller), L"bDelInstaller"));
+    infoFields.AddTail(new SettingsFieldBool(&(settings.Maximized), L"WindowPosMaximized"));
+    infoFields.AddTail(new SettingsFieldInt(&(settings.Left), L"WindowPosLeft"));
+    infoFields.AddTail(new SettingsFieldInt(&(settings.Top), L"WindowPosTop"));
+    infoFields.AddTail(new SettingsFieldInt(&(settings.Width), L"WindowPosWidth"));
+    infoFields.AddTail(new SettingsFieldInt(&(settings.Height), L"WindowPosHeight"));
+    infoFields.AddTail(new SettingsFieldInt(&(settings.Proxy), L"ProxyMode"));
+    infoFields.AddTail(new SettingsFieldString((settings.szProxyServer), MAX_PATH, L"ProxyServer"));
+    infoFields.AddTail(new SettingsFieldString((settings.szNoProxyFor), MAX_PATH, L"NoProxyFor"));
+    infoFields.AddTail(new SettingsFieldBool(&(settings.bUseSource), L"bUseSource"));
+    infoFields.AddTail(new SettingsFieldString((settings.szSourceURL), INTERNET_MAX_URL_LENGTH, L"SourceURL"));
+
+    return;
+}
+
+BOOL SaveAllSettings(CRegKey &key, SETTINGS_INFO &settings)
+{
+    BOOL bAllSuccess = TRUE;
+    ATL::CAtlList<SettingsField *> infoFields;
+
+    AddInfoFields(infoFields, settings);
+
+    POSITION InfoListPosition = infoFields.GetHeadPosition();
+    while (InfoListPosition)
+    {
+        SettingsField *Info = infoFields.GetNext(InfoListPosition);
+        if (!Info->Save(key))
+        {
+            bAllSuccess = FALSE;
+            // TODO: error log
+        }
+        delete Info;
+    }
+    return bAllSuccess;
+}
+
+BOOL LoadAllSettings(CRegKey &key, SETTINGS_INFO &settings)
+{
+    BOOL bAllSuccess = TRUE;
+    ATL::CAtlList<SettingsField *> infoFields;
+
+    AddInfoFields(infoFields, settings);
+
+    POSITION InfoListPosition = infoFields.GetHeadPosition();
+    while (InfoListPosition)
+    {
+        SettingsField *Info = infoFields.GetNext(InfoListPosition);
+        if (!Info->Load(key))
+        {
+            bAllSuccess = FALSE;
+            // TODO: error log
+        }
+        delete Info;
+    }
+    return bAllSuccess;
+}
+
+VOID FillDefaultSettings(PSETTINGS_INFO pSettingsInfo)
+{
+    ATL::CStringW szDownloadDir;
+    ZeroMemory(pSettingsInfo, sizeof(SETTINGS_INFO));
+
+    pSettingsInfo->bSaveWndPos = TRUE;
+    pSettingsInfo->bUpdateAtStart = FALSE;
+    pSettingsInfo->bLogEnabled = TRUE;
+    pSettingsInfo->bUseSource = FALSE;
+
+    if (FAILED(SHGetFolderPathW(NULL, CSIDL_PERSONAL, NULL, SHGFP_TYPE_CURRENT, szDownloadDir.GetBuffer(MAX_PATH))))
+    {
+        szDownloadDir.ReleaseBuffer();
+        if (!szDownloadDir.GetEnvironmentVariableW(L"SystemDrive"))
+        {
+            szDownloadDir = L"C:";
+        }
+    }
+    else
+    {
+        szDownloadDir.ReleaseBuffer();
+    }
+
+    PathAppendW(szDownloadDir.GetBuffer(MAX_PATH), L"\\RAPPS Downloads");
+    szDownloadDir.ReleaseBuffer();
+
+    ATL::CStringW::CopyChars(pSettingsInfo->szDownloadDir,
+        _countof(pSettingsInfo->szDownloadDir),
+        szDownloadDir.GetString(),
+        szDownloadDir.GetLength() + 1);
+
+    pSettingsInfo->bDelInstaller = FALSE;
+    pSettingsInfo->Maximized = FALSE;
+    pSettingsInfo->Left = CW_USEDEFAULT;
+    pSettingsInfo->Top = CW_USEDEFAULT;
+    pSettingsInfo->Width = 680;
+    pSettingsInfo->Height = 450;
+}
+
+BOOL LoadSettings(PSETTINGS_INFO pSettingsInfo)
+{
+    ATL::CRegKey RegKey;
+    if (RegKey.Open(HKEY_CURRENT_USER, L"Software\\ReactOS\\rapps", KEY_READ) != ERROR_SUCCESS)
+    {
+        return FALSE;
+    }
+
+    return LoadAllSettings(RegKey, *pSettingsInfo);
+}
+
+BOOL SaveSettings(HWND hwnd, PSETTINGS_INFO pSettingsInfo)
+{
+    WINDOWPLACEMENT wp;
+    ATL::CRegKey RegKey;
+
+    if (pSettingsInfo->bSaveWndPos)
+    {
+        wp.length = sizeof(wp);
+        GetWindowPlacement(hwnd, &wp);
+
+        pSettingsInfo->Left = wp.rcNormalPosition.left;
+        pSettingsInfo->Top = wp.rcNormalPosition.top;
+        pSettingsInfo->Width = wp.rcNormalPosition.right - wp.rcNormalPosition.left;
+        pSettingsInfo->Height = wp.rcNormalPosition.bottom - wp.rcNormalPosition.top;
+        pSettingsInfo->Maximized = (wp.showCmd == SW_MAXIMIZE
+            || (wp.showCmd == SW_SHOWMINIMIZED
+                && (wp.flags & WPF_RESTORETOMAXIMIZED)));
+    }
+
+    if (RegKey.Create(HKEY_CURRENT_USER, L"Software\\ReactOS\\rapps", NULL,
+        REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, NULL) != ERROR_SUCCESS)
+    {
+        return FALSE;
+    }
+
+    return SaveAllSettings(RegKey, *pSettingsInfo);
+}

base/applications/rapps/settingsdlg.cpp

@@ -260,7 +260,7 @@ namespace
                 }
 
                 SettingsInfo = NewSettingsInfo;
-                SaveSettings(GetParent(hDlg));
+                SaveSettings(GetParent(hDlg), &SettingsInfo);
                 EndDialog(hDlg, LOWORD(wParam));
             }
             break;

base/applications/rapps/winmain.cpp

@@ -10,6 +10,8 @@
 
 #include "unattended.h"
 
+#include "winmain.h"
+
 #include <atlcom.h>
 
 #include <gdiplus.h>
@@ -61,88 +63,6 @@ VOID InitializeGDIPlus(BOOL bInitialize)
     }
 }
 
-VOID FillDefaultSettings(PSETTINGS_INFO pSettingsInfo)
-{
-    ATL::CStringW szDownloadDir;
-    ZeroMemory(pSettingsInfo, sizeof(SETTINGS_INFO));
-
-    pSettingsInfo->bSaveWndPos = TRUE;
-    pSettingsInfo->bUpdateAtStart = FALSE;
-    pSettingsInfo->bLogEnabled = TRUE;
-    pSettingsInfo->bUseSource = FALSE;
-    
-    if (FAILED(SHGetFolderPathW(NULL, CSIDL_PERSONAL, NULL, SHGFP_TYPE_CURRENT, szDownloadDir.GetBuffer(MAX_PATH))))
-    {
-        szDownloadDir.ReleaseBuffer();
-        if (!szDownloadDir.GetEnvironmentVariableW(L"SystemDrive"))
-        {
-            szDownloadDir = L"C:";
-        }
-    }
-    else
-    {
-        szDownloadDir.ReleaseBuffer();
-    }
-
-    PathAppendW(szDownloadDir.GetBuffer(MAX_PATH), L"\\RAPPS Downloads");
-    szDownloadDir.ReleaseBuffer();
-
-    ATL::CStringW::CopyChars(pSettingsInfo->szDownloadDir,
-                             _countof(pSettingsInfo->szDownloadDir),
-                             szDownloadDir.GetString(),
-                             szDownloadDir.GetLength() + 1);
-
-    pSettingsInfo->bDelInstaller = FALSE;
-    pSettingsInfo->Maximized = FALSE;
-    pSettingsInfo->Left = CW_USEDEFAULT;
-    pSettingsInfo->Top = CW_USEDEFAULT;
-    pSettingsInfo->Width = 680;
-    pSettingsInfo->Height = 450;
-}
-
-static BOOL LoadSettings()
-{
-    ATL::CRegKey RegKey;
-    DWORD dwSize;
-    BOOL bResult = FALSE;
-    if (RegKey.Open(HKEY_CURRENT_USER, L"Software\\ReactOS\\rapps", KEY_READ) == ERROR_SUCCESS)
-    {
-        dwSize = sizeof(SettingsInfo);
-        bResult = (RegKey.QueryBinaryValue(L"Settings", (PVOID) &SettingsInfo, &dwSize) == ERROR_SUCCESS);
-
-        RegKey.Close();
-    }
-
-    return bResult;
-}
-
-VOID SaveSettings(HWND hwnd)
-{
-    WINDOWPLACEMENT wp;
-    ATL::CRegKey RegKey;
-
-    if (SettingsInfo.bSaveWndPos)
-    {
-        wp.length = sizeof(wp);
-        GetWindowPlacement(hwnd, &wp);
-
-        SettingsInfo.Left = wp.rcNormalPosition.left;
-        SettingsInfo.Top = wp.rcNormalPosition.top;
-        SettingsInfo.Width = wp.rcNormalPosition.right - wp.rcNormalPosition.left;
-        SettingsInfo.Height = wp.rcNormalPosition.bottom - wp.rcNormalPosition.top;
-        SettingsInfo.Maximized = (wp.showCmd == SW_MAXIMIZE
-                                  || (wp.showCmd == SW_SHOWMINIMIZED
-                                      && (wp.flags & WPF_RESTORETOMAXIMIZED)));
-    }
-
-    if (RegKey.Create(HKEY_CURRENT_USER, L"Software\\ReactOS\\rapps", NULL,
-                      REG_OPTION_NON_VOLATILE, KEY_WRITE, NULL, NULL) == ERROR_SUCCESS)
-    {
-        RegKey.SetBinaryValue(L"Settings", (const PVOID) &SettingsInfo, sizeof(SettingsInfo));
-        RegKey.Close();
-    }
-}
-
 int wmain(int argc, wchar_t *argv[])
 {
     BOOL bIsFirstLaunch;
@@ -157,7 +77,7 @@ int wmain(int argc, wchar_t *argv[])
 
     hInst = GetModuleHandle(NULL);
 
-    bIsFirstLaunch = !LoadSettings();
+    bIsFirstLaunch = !LoadSettings(&SettingsInfo);
     if (bIsFirstLaunch)
     {
         FillDefaultSettings(&SettingsInfo);