desktop/win32/source/loader.cxx |  307 ++++++++++++++++++----------------------
 desktop/win32/source/loader.hxx |   54 -------
 2 files changed, 142 insertions(+), 219 deletions(-)

New commits:
commit 41356475d08f28b8fba0830b4a89bd0c6b58e2b9
Author:     Mike Kaganski <[email protected]>
AuthorDate: Tue Jan 23 20:06:20 2024 +0600
Commit:     Mike Kaganski <[email protected]>
CommitDate: Wed Jan 24 04:48:18 2024 +0100

    Some refactor of Win32 loader
    
    Change-Id: If019677d24a56c46f06b31a15a18a615b62a6806
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/162445
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <[email protected]>

diff --git a/desktop/win32/source/loader.cxx b/desktop/win32/source/loader.cxx
index d30f0ef90896..b730d4119695 100644
--- a/desktop/win32/source/loader.cxx
+++ b/desktop/win32/source/loader.cxx
@@ -18,18 +18,24 @@
  */
 
 #include "loader.hxx"
+#include <algorithm>
 #include <cassert>
-#include <systools/win32/uwinapi.h>
+#include <numeric>
 #include <stdlib.h>
 #include <string>
+#include <string_view>
 #include <vector>
 #include <desktop/exithelper.h>
+#include <systools/win32/uwinapi.h>
 #include <tools/pathutils.hxx>
 
 #include <fstream>
 #include <boost/property_tree/ptree.hpp>
 #include <boost/property_tree/ini_parser.hpp>
 
+#define MY_LENGTH(s) (std::size(s) - 1)
+#define MY_STRING(s) (s), MY_LENGTH(s)
+
 namespace {
 
 void fail()
@@ -43,63 +49,97 @@ void fail()
     TerminateProcess(GetCurrentProcess(), 255);
 }
 
-LPWSTR* GetCommandArgs(int* pArgc) { return 
CommandLineToArgvW(GetCommandLineW(), pArgc); }
+struct CommandArgs
+{
+    LPWSTR* argv;
+    int argc;
+    CommandArgs() { argv = CommandLineToArgvW(GetCommandLineW(), &argc); }
+    ~CommandArgs() { LocalFree(argv); }
+    auto begin() const { return argv; }
+    auto end() const { return begin() + argc; }
+};
 
 // tdf#120249: quotes in arguments need to be escaped; backslashes before 
quotes need doubling. See
 // 
https://docs.microsoft.com/en-us/windows/desktop/api/shellapi/nf-shellapi-commandlinetoargvw
-std::wstring EscapeArg(LPCWSTR sArg)
+std::wstring EscapeArg(std::wstring_view sArg)
 {
-    const size_t nOrigSize = wcslen(sArg);
-    LPCWSTR const end = sArg + nOrigSize;
     std::wstring sResult(L"\"");
-
-    LPCWSTR lastPosQuote = sArg;
-    LPCWSTR posQuote;
-    while ((posQuote = std::find(lastPosQuote, end, L'"')) != end)
+    for (size_t lastPosQuote = 0; lastPosQuote <= sArg.size();)
     {
-        LPCWSTR posBackslash = posQuote;
-        while (posBackslash != lastPosQuote && *(posBackslash - 1) == L'\')
+        const size_t posQuote = std::min(sArg.find(L'"', lastPosQuote), 
sArg.size());
+        size_t posBackslash = posQuote;
+        while (posBackslash != lastPosQuote && sArg[posBackslash - 1] == L'\')
             --posBackslash;
+        // 2n+1 '\' to escape internal '"'; 2n '\' before closing '"'
+        const size_t nEscapes = (posQuote - posBackslash) * 2 + (posQuote < 
sArg.size() ? 1 : 0);
 
-        sResult.append(lastPosQuote, posBackslash);
-        sResult.append((posQuote - posBackslash) * 2 + 1, L'\'); // 2n+1 '\' 
to escape the '"'
+        sResult.append(sArg.begin() + lastPosQuote, sArg.begin() + 
posBackslash);
+        sResult.append(nEscapes, L'\');
         sResult.append(1, L'"');
         lastPosQuote = posQuote + 1;
     }
-
-    LPCWSTR posTrailingBackslashSeq = end;
-    while (posTrailingBackslashSeq != lastPosQuote && 
*(posTrailingBackslashSeq - 1) == L'\')
-        --posTrailingBackslashSeq;
-    sResult.append(lastPosQuote, posTrailingBackslashSeq);
-    sResult.append((end - posTrailingBackslashSeq) * 2, L'\'); // 2n '\' 
before closing '"'
-    sResult.append(1, L'"');
-
     return sResult;
 }
 
-void AddEscapedArg(LPCWSTR sArg, std::vector<std::wstring>& aEscapedArgs,
-                   std::size_t& iLengthAccumulator)
+std::wstring getCWDarg()
 {
-    std::wstring sEscapedArg = EscapeArg(sArg);
-    aEscapedArgs.push_back(sEscapedArg);
-    iLengthAccumulator += sEscapedArg.length() + 1; // a space between args
-}
+    std::wstring s(L" \"-env:OOO_CWD=");
 
-bool HasWildCard(LPCWSTR sArg)
-{
-    while (*sArg != L'
+    WCHAR cwd[MAX_PATH];
+    DWORD cwdLen = GetCurrentDirectoryW(MAX_PATH, cwd);
+    if (cwdLen == 0 || cwdLen >= MAX_PATH)
     {
-        if (*sArg == L'*' || *sArg == L'?')
-            return true;
-        sArg++;
+        s += L'0';
     }
-    return false;
-}
+    else
+    {
+        s += L'2';
 
+        size_t n = 0; // number of trailing backslashes
+        for (auto* p = cwd; *p; ++p)
+        {
+            WCHAR c = *p;
+            if (c == L'$')
+            {
+                s += L"\$";
+                n = 0;
+            }
+            else if (c == L'\')
+            {
+                s += L"\\";
+                n += 2;
+            }
+            else
+            {
+                s += c;
+                n = 0;
+            }
+        }
+        // The command line will continue with a double quote, so double any
+        // preceding backslashes as required by Windows:
+        s.append(n, L'\');
+    }
+    s += L'"';
+    return s;
 }
 
-namespace desktop_win32 {
+WCHAR* commandLineAppend(WCHAR* buffer, std::wstring_view text)
+{
+    auto ret = std::copy_n(text.begin(), text.size(), buffer);
+    *ret = 0; // trailing null
+    return ret;
+}
 
+// Set the PATH environment variable in the current (loader) process, so that a
+// following CreateProcess has the necessary environment:
+// @param binPath
+// Must point to an array of size at least MAX_PATH.  Is filled with the null
+// terminated full path to the "bin" file corresponding to the current
+// executable.
+// @param iniDirectory
+// Must point to an array of size at least MAX_PATH.  Is filled with the null
+// terminated full directory path (ending in "\") to the "ini" file
+// corresponding to the current executable.
 void extendLoaderEnvironment(WCHAR * binPath, WCHAR * iniDirectory) {
     if (!GetModuleFileNameW(nullptr, iniDirectory, MAX_PATH)) {
         fail();
@@ -135,19 +175,15 @@ void extendLoaderEnvironment(WCHAR * binPath, WCHAR * 
iniDirectory) {
     }
     // must be first in PATH to override other entries
     assert(*(iniDirEnd - 1) == L'\'); // hence -1 below
-    if (wcsncmp(env, iniDirectory, iniDirEnd - iniDirectory - 1) != 0
-        || env[iniDirEnd - iniDirectory - 1] != L';')
+    std::wstring_view iniDirView(iniDirectory, iniDirEnd - iniDirectory - 1);
+    if (!std::wstring_view(env, n).starts_with(iniDirView) || 
env[iniDirView.size()] != L';')
     {
         WCHAR pad[MAX_PATH + maxEnv];
             // hopefully std::size_t is large enough to not overflow
-        WCHAR * p = commandLineAppend(pad, iniDirectory, iniDirEnd - 
iniDirectory - 1);
+        WCHAR* p = commandLineAppend(pad, iniDirView);
         if (n != 0) {
             *p++ = L';';
-            for (DWORD i = 0; i <= n; ++i) {
-                *p++ = env[i];
-            }
-        } else {
-            *p++ = L'
+            commandLineAppend(p, std::wstring_view(env, n));
         }
         if (!SetEnvironmentVariableW(L"PATH", pad)) {
             fail();
@@ -155,14 +191,18 @@ void extendLoaderEnvironment(WCHAR * binPath, WCHAR * 
iniDirectory) {
     }
 }
 
+}
+
+namespace desktop_win32 {
+
 int officeloader_impl(bool bAllowConsole)
 {
     WCHAR szTargetFileName[MAX_PATH] = {};
     WCHAR szIniDirectory[MAX_PATH];
-    STARTUPINFOW aStartupInfo;
 
-    desktop_win32::extendLoaderEnvironment(szTargetFileName, szIniDirectory);
+    extendLoaderEnvironment(szTargetFileName, szIniDirectory);
 
+    STARTUPINFOW aStartupInfo;
     ZeroMemory(&aStartupInfo, sizeof(aStartupInfo));
     aStartupInfo.cb = sizeof(aStartupInfo);
 
@@ -173,15 +213,7 @@ int officeloader_impl(bool bAllowConsole)
     DWORD dwExitCode = DWORD(-1);
 
     bool fSuccess = false;
-    LPWSTR lpCommandLine = nullptr;
     bool bFirst = true;
-    WCHAR cwd[MAX_PATH];
-    DWORD cwdLen = GetCurrentDirectoryW(MAX_PATH, cwd);
-    if (cwdLen >= MAX_PATH)
-    {
-        cwdLen = 0;
-    }
-    std::vector<std::wstring> aEscapedArgs;
 
     // read limit values from bootstrap.ini
     unsigned int nMaxMemoryInMB = 0;
@@ -224,111 +256,69 @@ int officeloader_impl(bool bAllowConsole)
                                     
sizeof(JOBOBJECT_EXTENDED_LIMIT_INFORMATION));
     }
 
-    do
+    std::vector<std::wstring> aEscapedArgs;
+    bool bHeadlessMode = false;
+    for (std::wstring_view arg : CommandArgs())
     {
-        if (bFirst)
+        // Check command line arguments for "--headless" parameter. We only 
set the environment
+        // variable "ATTACHED_PARENT_PROCESSID" for the headless mode as 
self-destruction of the
+        // soffice.bin process can lead to certain side-effects (log-off can 
result in data-loss,
+        // ".lock" is not deleted). See 138244 for more information.
+        if (arg == L"-headless" || arg == L"--headless")
+            bHeadlessMode = true;
+        // check for wildcards in arguments - Windows does not expand 
automatically
+        else if (arg.find_first_of(L"*?") != std::wstring_view::npos)
         {
-            int argc = 0;
-            LPWSTR* argv = GetCommandArgs(&argc);
-            std::size_t n = 0;
-            for (int i = 0; i < argc; ++i)
+            WIN32_FIND_DATAW aFindData;
+            HANDLE h = FindFirstFileW(arg.data(), &aFindData);
+            if (h != INVALID_HANDLE_VALUE)
             {
-                // check for wildCards in arguments- windows does not expand 
automatically
-                if (HasWildCard(argv[i]))
-                {
-                    WIN32_FIND_DATAW aFindData;
-                    HANDLE h = FindFirstFileW(argv[i], &aFindData);
-                    if (h == INVALID_HANDLE_VALUE)
-                    {
-                        AddEscapedArg(argv[i], aEscapedArgs, n);
-                    }
-                    else
-                    {
-                        const int nPathSize = 32 * 1024;
-                        wchar_t drive[nPathSize];
-                        wchar_t dir[nPathSize];
-                        wchar_t path[nPathSize];
-                        _wsplitpath_s(argv[i], drive, nPathSize, dir, 
nPathSize, nullptr, 0,
-                                      nullptr, 0);
-                        _wmakepath_s(path, nPathSize, drive, dir, 
aFindData.cFileName, nullptr);
-                        AddEscapedArg(path, aEscapedArgs, n);
-
-                        while (FindNextFileW(h, &aFindData))
-                        {
-                            _wmakepath_s(path, nPathSize, drive, dir, 
aFindData.cFileName, nullptr);
-                            AddEscapedArg(path, aEscapedArgs, n);
-                        }
-                        FindClose(h);
-                    }
-                }
-                else
+                const int nPathSize = 32 * 1024;
+                wchar_t drive[3];
+                wchar_t dir[nPathSize];
+                wchar_t path[nPathSize];
+                _wsplitpath_s(arg.data(), drive, std::size(drive), dir, 
std::size(dir), nullptr, 0,
+                              nullptr, 0);
+                do
                 {
-                    AddEscapedArg(argv[i], aEscapedArgs, n);
-                }
-            }
-            LocalFree(argv);
-            n += MY_LENGTH(L" \"-env:OOO_CWD=2") + 4 * cwdLen + 
MY_LENGTH(L"\"") + 1;
-            // 4 * cwdLen: each char preceded by backslash, each trailing
-            // backslash doubled
-            lpCommandLine = new WCHAR[n];
-        }
-        WCHAR* p = desktop_win32::commandLineAppend(lpCommandLine, 
aEscapedArgs[0].c_str(),
-                                                    aEscapedArgs[0].length());
-        for (size_t i = 1; i < aEscapedArgs.size(); ++i)
-        {
-            const std::wstring& rArg = aEscapedArgs[i];
-            if (bFirst || EXITHELPER_NORMAL_RESTART == dwExitCode
-                || wcsncmp(rArg.c_str(), MY_STRING(L"\"-env:")) == 0)
-            {
-                p = desktop_win32::commandLineAppend(p, MY_STRING(L" "));
-                p = desktop_win32::commandLineAppend(p, rArg.c_str(), 
rArg.length());
+                    _wmakepath_s(path, std::size(path), drive, dir, 
aFindData.cFileName, nullptr);
+                    aEscapedArgs.push_back(EscapeArg(path));
+                } while (FindNextFileW(h, &aFindData));
+                FindClose(h);
+                continue;
             }
         }
 
-        p = desktop_win32::commandLineAppend(p, MY_STRING(L" 
\"-env:OOO_CWD="));
-        if (cwdLen == 0)
-        {
-            p = desktop_win32::commandLineAppend(p, MY_STRING(L"0"));
-        }
-        else
-        {
-            p = desktop_win32::commandLineAppend(p, MY_STRING(L"2"));
-            p = desktop_win32::commandLineAppendEncoded(p, cwd);
-        }
-        desktop_win32::commandLineAppend(p, MY_STRING(L"\""));
-        bFirst = false;
+        aEscapedArgs.push_back(EscapeArg(arg));
+    }
+    size_t n = std::accumulate(aEscapedArgs.begin(), aEscapedArgs.end(), 
aEscapedArgs.size(),
+                               [](size_t a, const std::wstring& s) { return a 
+ s.size(); });
+    std::wstring sCWDarg = getCWDarg();
+    n += sCWDarg.size() + 1;
+    LPWSTR lpCommandLine = new WCHAR[n];
 
+    if (bHeadlessMode)
+    {
         WCHAR szParentProcessId[64]; // This is more than large enough for a 
128 bit decimal value
-        bool bHeadlessMode(false);
+        if (_ltow(static_cast<long>(GetCurrentProcessId()), szParentProcessId, 
10))
+            SetEnvironmentVariableW(L"ATTACHED_PARENT_PROCESSID", 
szParentProcessId);
+    }
 
+    do
+    {
+        WCHAR* p = commandLineAppend(lpCommandLine, aEscapedArgs[0]);
+        for (size_t i = 1; i < aEscapedArgs.size(); ++i)
         {
-            // Check command line arguments for "--headless" parameter. We only
-            // set the environment variable "ATTACHED_PARENT_PROCESSID" for 
the headless
-            // mode as self-destruction of the soffice.bin process can lead to
-            // certain side-effects (log-off can result in data-loss, ".lock" 
is not deleted.
-            // See 138244 for more information.
-            int argc2;
-            LPWSTR* argv2 = GetCommandArgs(&argc2);
-
-            if (argc2 > 1)
+            const std::wstring& rArg = aEscapedArgs[i];
+            if (bFirst || EXITHELPER_NORMAL_RESTART == dwExitCode || 
rArg.starts_with(L"\"-env:"))
             {
-                int n;
-
-                for (n = 1; n < argc2; n++)
-                {
-                    if (0 == wcsnicmp(argv2[n], L"-headless", 9)
-                        || 0 == wcsnicmp(argv2[n], L"--headless", 10))
-                    {
-                        bHeadlessMode = true;
-                    }
-                }
+                p = commandLineAppend(p, L" ");
+                p = commandLineAppend(p, rArg);
             }
-
-            LocalFree(argv2);
         }
 
-        if (_ltow(static_cast<long>(GetCurrentProcessId()), szParentProcessId, 
10) && bHeadlessMode)
-            SetEnvironmentVariableW(L"ATTACHED_PARENT_PROCESSID", 
szParentProcessId);
+        commandLineAppend(p, sCWDarg);
+        bFirst = false;
 
         PROCESS_INFORMATION aProcessInfo;
 
@@ -381,7 +371,7 @@ int unopkgloader_impl(bool bAllowConsole)
 {
     WCHAR        szTargetFileName[MAX_PATH];
     WCHAR        szIniDirectory[MAX_PATH];
-    desktop_win32::extendLoaderEnvironment(szTargetFileName, szIniDirectory);
+    extendLoaderEnvironment(szTargetFileName, szIniDirectory);
 
     STARTUPINFOW aStartupInfo{};
     aStartupInfo.cb = sizeof(aStartupInfo);
@@ -390,11 +380,7 @@ int unopkgloader_impl(bool bAllowConsole)
     DWORD   dwExitCode = DWORD(-1);
 
     size_t iniDirLen = wcslen(szIniDirectory);
-    WCHAR cwd[MAX_PATH];
-    DWORD cwdLen = GetCurrentDirectoryW(MAX_PATH, cwd);
-    if (cwdLen >= MAX_PATH) {
-        cwdLen = 0;
-    }
+    std::wstring sCWDarg = getCWDarg();
     WCHAR redirect[MAX_PATH];
     DWORD dummy;
     bool hasRedirect =
@@ -410,25 +396,16 @@ int unopkgloader_impl(bool bAllowConsole)
                 ? (MY_LENGTH(L" \"-env:INIFILENAME=vnd.sun.star.pathname:") +
                     iniDirLen + MY_LENGTH(L"redirect.ini\""))
                 : 0) +
-            MY_LENGTH(L" \"-env:OOO_CWD=2") + 4 * cwdLen + MY_LENGTH(L"\"") + 
1];
+            sCWDarg.size() + 1];
     // 4 * cwdLen: each char preceded by backslash, each trailing backslash
     // doubled
-    WCHAR* p = desktop_win32::commandLineAppend(cl2, cl1);
+    WCHAR* p = commandLineAppend(cl2, cl1);
     if (hasRedirect) {
-        p = desktop_win32::commandLineAppend(
-            p, MY_STRING(L" \"-env:INIFILENAME=vnd.sun.star.pathname:"));
-        p = desktop_win32::commandLineAppend(p, szIniDirectory);
-        p = desktop_win32::commandLineAppend(p, MY_STRING(L"redirect.ini\""));
-    }
-    p = desktop_win32::commandLineAppend(p, MY_STRING(L" \"-env:OOO_CWD="));
-    if (cwdLen == 0) {
-        p = desktop_win32::commandLineAppend(p, MY_STRING(L"0"));
-    }
-    else {
-        p = desktop_win32::commandLineAppend(p, MY_STRING(L"2"));
-        p = desktop_win32::commandLineAppendEncoded(p, cwd);
+        p = commandLineAppend(p, L" 
\"-env:INIFILENAME=vnd.sun.star.pathname:");
+        p = commandLineAppend(p, szIniDirectory);
+        p = commandLineAppend(p, L"redirect.ini\"");
     }
-    desktop_win32::commandLineAppend(p, MY_STRING(L"\""));
+    commandLineAppend(p, sCWDarg);
 
     PROCESS_INFORMATION aProcessInfo;
 
diff --git a/desktop/win32/source/loader.hxx b/desktop/win32/source/loader.hxx
index 66b0ed8e53a1..784926a78080 100644
--- a/desktop/win32/source/loader.hxx
+++ b/desktop/win32/source/loader.hxx
@@ -19,65 +19,11 @@
 
 #pragma once
 
-#include <cstddef>
 #define WIN32_LEAN_AND_MEAN
 #include <windows.h>
-#include <string.h>
-
-#define MY_LENGTH(s) (sizeof (s) / sizeof *(s) - 1)
-#define MY_STRING(s) (s), MY_LENGTH(s)
 
 namespace desktop_win32 {
 
-inline WCHAR * commandLineAppend(
-    WCHAR * buffer, WCHAR const * text, std::size_t length)
-{
-    wcsncpy(buffer, text, length + 1); // trailing null
-    return buffer + length;
-}
-
-inline WCHAR * commandLineAppend(WCHAR * buffer, WCHAR const * text) {
-    return commandLineAppend(buffer, text, wcslen(text));
-}
-
-inline WCHAR * commandLineAppendEncoded(WCHAR * buffer, WCHAR const * text) {
-    std::size_t n = 0;
-    for (;;) {
-        WCHAR c = *text++;
-        if (c == L'
-            break;
-        } else if (c == L'$') {
-            buffer = commandLineAppend(buffer, MY_STRING(L"\$"));
-            n = 0;
-        } else if (c == L'\') {
-            buffer = commandLineAppend(buffer, MY_STRING(L"\\"));
-            n += 2;
-        } else {
-            *buffer++ = c;
-            n = 0;
-        }
-    }
-    // The command line will continue with a double quote, so double any
-    // preceding backslashes as required by Windows:
-    for (std::size_t i = 0; i < n; ++i) {
-        *buffer++ = L'\';
-    }
-    *buffer = L'
-    return buffer;
-}
-
-// Set the PATH environment variable in the current (loader) process, so that a
-// following CreateProcess has the necessary environment:
-// @param binPath
-// Must point to an array of size at least MAX_PATH.  Is filled with the null
-// terminated full path to the "bin" file corresponding to the current
-// executable.
-// @param iniDirectory
-// Must point to an array of size at least MAX_PATH.  Is filled with the null
-// terminated full directory path (ending in "\") to the "ini" file
-// corresponding to the current executable.
-void extendLoaderEnvironment(WCHAR * binPath, WCHAR * iniDirectory);
-
 // Implementation of the process guarding soffice.bin
 int officeloader_impl(bool bAllowConsole);
 

Reply via email to