[ros-diffs] [reactos] 01/01: [DBGHELP] Fix default search path handling. CORE-17073

[Thomas Faber]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=4601d948013712974d9fd…

commit 4601d948013712974d9fd6990800c0ecfb36ed36
Author:     Thomas Faber <thomas.faber(a)reactos.org>
AuthorDate: Mon Nov 15 20:11:42 2021 -0500
Commit:     Thomas Faber <thomas.faber(a)reactos.org>
CommitDate: Mon Nov 15 20:14:34 2021 -0500

    [DBGHELP] Fix default search path handling. CORE-17073
    
    * Allow NULL search path in SymSetSearchPath
    * Use . instead of concrete current directory
    * Use _NT_ALT_SYMBOL_PATH variable
    * Add some tests
---
 dll/win32/dbghelp/dbghelp.c                  | 98 ++++++++++++++++++----------
 modules/rostests/winetests/dbghelp/dbghelp.c | 67 ++++++++++++++++++-
 2 files changed, 128 insertions(+), 37 deletions(-)

diff --git a/dll/win32/dbghelp/dbghelp.c b/dll/win32/dbghelp/dbghelp.c
index c67f9730125..62c249c411b 100644
--- a/dll/win32/dbghelp/dbghelp.c
+++ b/dll/win32/dbghelp/dbghelp.c
@@ -191,6 +191,43 @@ struct cpu* cpu_find(DWORD machine)
     return NULL;
 }
 
+static WCHAR* make_default_search_path(void)
+{
+    WCHAR*      search_path;
+    WCHAR*      p;
+    unsigned    sym_path_len;
+    unsigned    alt_sym_path_len;
+
+    sym_path_len = GetEnvironmentVariableW(L"_NT_SYMBOL_PATH", NULL, 0);
+    alt_sym_path_len = GetEnvironmentVariableW(L"_NT_ALT_SYMBOL_PATH", NULL,
0);
+
+    /* The default symbol path is
".[;%_NT_SYMBOL_PATH%][;%_NT_ALT_SYMBOL_PATH%]".
+     * If the variables exist, the lengths include a null-terminator. We use that
+     * space for the semicolons, and only add the initial dot and the final null. */
+    search_path = HeapAlloc(GetProcessHeap(), 0,
+                            (1 + sym_path_len + alt_sym_path_len + 1) * sizeof(WCHAR));
+    if (!search_path) return NULL;
+
+    p = search_path;
+    *p++ = L'.';
+    if (sym_path_len)
+    {
+        *p++ = L';';
+        GetEnvironmentVariableW(L"_NT_SYMBOL_PATH", p, sym_path_len);
+        p += sym_path_len - 1;
+    }
+
+    if (alt_sym_path_len)
+    {
+        *p++ = L';';
+        GetEnvironmentVariableW(L"_NT_ALT_SYMBOL_PATH", p, alt_sym_path_len);
+        p += alt_sym_path_len - 1;
+    }
+    *p = L'\0';
+
+    return search_path;
+}
+
 /******************************************************************
  *		SymSetSearchPathW (DBGHELP.@)
  *
@@ -198,14 +235,24 @@ struct cpu* cpu_find(DWORD machine)
 BOOL WINAPI SymSetSearchPathW(HANDLE hProcess, PCWSTR searchPath)
 {
     struct process* pcs = process_find_by_handle(hProcess);
+    WCHAR*          search_path_buffer;
 
     if (!pcs) return FALSE;
-    if (!searchPath) return FALSE;
 
+    if (searchPath)
+    {
+        search_path_buffer = HeapAlloc(GetProcessHeap(), 0,
+                                       (lstrlenW(searchPath) + 1) * sizeof(WCHAR));
+        if (!search_path_buffer) return FALSE;
+        lstrcpyW(search_path_buffer, searchPath);
+    }
+    else
+    {
+        search_path_buffer = make_default_search_path();
+        if (!search_path_buffer) return FALSE;
+    }
     HeapFree(GetProcessHeap(), 0, pcs->search_path);
-    pcs->search_path = lstrcpyW(HeapAlloc(GetProcessHeap(), 0,
-                                          (lstrlenW(searchPath) + 1) * sizeof(WCHAR)),
-                                searchPath);
+    pcs->search_path = search_path_buffer;
     return TRUE;
 }
 
@@ -217,16 +264,19 @@ BOOL WINAPI SymSetSearchPath(HANDLE hProcess, PCSTR searchPath)
 {
     BOOL        ret = FALSE;
     unsigned    len;
-    WCHAR*      sp;
+    WCHAR*      sp = NULL;
 
-    len = MultiByteToWideChar(CP_ACP, 0, searchPath, -1, NULL, 0);
-    if ((sp = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR))))
+    if (searchPath)
     {
+        len = MultiByteToWideChar(CP_ACP, 0, searchPath, -1, NULL, 0);
+        sp = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
+        if (!sp) return FALSE;
         MultiByteToWideChar(CP_ACP, 0, searchPath, -1, sp, len);
-
-        ret = SymSetSearchPathW(hProcess, sp);
-        HeapFree(GetProcessHeap(), 0, sp);
     }
+
+    ret = SymSetSearchPathW(hProcess, sp);
+
+    HeapFree(GetProcessHeap(), 0, sp);
     return ret;
 }
 
@@ -442,37 +492,13 @@ BOOL WINAPI SymInitializeW(HANDLE hProcess, PCWSTR UserSearchPath,
BOOL fInvadeP
 
     if (UserSearchPath)
     {
-        pcs->search_path = lstrcpyW(HeapAlloc(GetProcessHeap(), 0,
+        pcs->search_path = lstrcpyW(HeapAlloc(GetProcessHeap(), 0,      
                                               (lstrlenW(UserSearchPath) + 1) *
sizeof(WCHAR)),
                                     UserSearchPath);
     }
     else
     {
-        unsigned        size;
-        unsigned        len;
-        static const WCHAR      sym_path[] =
{'_','N','T','_','S','Y','M','B','O','L','_','P','A','T','H',0};
-        static const WCHAR      alt_sym_path[] =
{'_','N','T','_','A','L','T','E','R','N','A','T','E','_','S','Y','M','B','O','L','_','P','A','T','H',0};
-
-        pcs->search_path = HeapAlloc(GetProcessHeap(), 0, (len = MAX_PATH) *
sizeof(WCHAR));
-        while ((size = GetCurrentDirectoryW(len, pcs->search_path)) >= len)
-            pcs->search_path = HeapReAlloc(GetProcessHeap(), 0, pcs->search_path,
(len *= 2) * sizeof(WCHAR));
-        pcs->search_path = HeapReAlloc(GetProcessHeap(), 0, pcs->search_path, (size
+ 1) * sizeof(WCHAR));
-
-        len = GetEnvironmentVariableW(sym_path, NULL, 0);
-        if (len)
-        {
-            pcs->search_path = HeapReAlloc(GetProcessHeap(), 0, pcs->search_path,
(size + 1 + len + 1) * sizeof(WCHAR));
-            pcs->search_path[size] = ';';
-            GetEnvironmentVariableW(sym_path, pcs->search_path + size + 1, len);
-            size += 1 + len;
-        }
-        len = GetEnvironmentVariableW(alt_sym_path, NULL, 0);
-        if (len)
-        {
-            pcs->search_path = HeapReAlloc(GetProcessHeap(), 0, pcs->search_path,
(size + 1 + len + 1) * sizeof(WCHAR));
-            pcs->search_path[size] = ';';
-            GetEnvironmentVariableW(alt_sym_path, pcs->search_path + size + 1, len);
-        }
+        pcs->search_path = make_default_search_path();
     }
 
     pcs->lmodules = NULL;
diff --git a/modules/rostests/winetests/dbghelp/dbghelp.c
b/modules/rostests/winetests/dbghelp/dbghelp.c
index 214adb7b16b..f062d647ed9 100644
--- a/modules/rostests/winetests/dbghelp/dbghelp.c
+++ b/modules/rostests/winetests/dbghelp/dbghelp.c
@@ -132,12 +132,77 @@ static void test_stack_walk(void)
 
 #endif /* __i386__ || __x86_64__ */
 
+static void test_search_path(void)
+{
+    char search_path[128];
+    BOOL ret;
+
+    /* The default symbol path is
".[;%_NT_SYMBOL_PATH%][;%_NT_ALT_SYMBOL_PATH%]".
+     * We unset both variables earlier so should simply get "." */
+    ret = SymGetSearchPath(GetCurrentProcess(), search_path, ARRAY_SIZE(search_path));
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ok(!strcmp(search_path, "."), "Got search path '%s', expected
'.'\n", search_path);
+
+    /* Set an arbitrary search path */
+    ret = SymSetSearchPath(GetCurrentProcess(), "W:\\");
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ret = SymGetSearchPath(GetCurrentProcess(), search_path, ARRAY_SIZE(search_path));
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ok(!strcmp(search_path, "W:\\"), "Got search path '%s',
expected 'W:\\'\n", search_path);
+
+    /* Setting to NULL resets to the default */
+    ret = SymSetSearchPath(GetCurrentProcess(), NULL);
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ret = SymGetSearchPath(GetCurrentProcess(), search_path, ARRAY_SIZE(search_path));
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ok(!strcmp(search_path, "."), "Got search path '%s', expected
'.'\n", search_path);
+
+    /* With _NT_SYMBOL_PATH */
+    SetEnvironmentVariableA("_NT_SYMBOL_PATH", "X:\\");
+    ret = SymSetSearchPath(GetCurrentProcess(), NULL);
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ret = SymGetSearchPath(GetCurrentProcess(), search_path, ARRAY_SIZE(search_path));
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ok(!strcmp(search_path, ".;X:\\"), "Got search path '%s',
expected '.;X:\\'\n", search_path);
+
+    /* With both _NT_SYMBOL_PATH and _NT_ALT_SYMBOL_PATH */
+    SetEnvironmentVariableA("_NT_ALT_SYMBOL_PATH", "Y:\\");
+    ret = SymSetSearchPath(GetCurrentProcess(), NULL);
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ret = SymGetSearchPath(GetCurrentProcess(), search_path, ARRAY_SIZE(search_path));
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ok(!strcmp(search_path, ".;X:\\;Y:\\"), "Got search path '%s',
expected '.;X:\\;Y:\\'\n", search_path);
+
+    /* With just _NT_ALT_SYMBOL_PATH */
+    SetEnvironmentVariableA("_NT_SYMBOL_PATH", NULL);
+    ret = SymSetSearchPath(GetCurrentProcess(), NULL);
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ret = SymGetSearchPath(GetCurrentProcess(), search_path, ARRAY_SIZE(search_path));
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ok(!strcmp(search_path, ".;Y:\\"), "Got search path '%s',
expected '.;Y:\\'\n", search_path);
+
+    /* Restore original search path */
+    SetEnvironmentVariableA("_NT_ALT_SYMBOL_PATH", NULL);
+    ret = SymSetSearchPath(GetCurrentProcess(), NULL);
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ret = SymGetSearchPath(GetCurrentProcess(), search_path, ARRAY_SIZE(search_path));
+    ok(ret == TRUE, "ret = %d\n", ret);
+    ok(!strcmp(search_path, "."), "Got search path '%s', expected
'.'\n", search_path);
+}
+
 START_TEST(dbghelp)
 {
-    BOOL ret = SymInitialize(GetCurrentProcess(), NULL, TRUE);
+    BOOL ret;
+
+    /* Don't let the user's environment influence our symbol path */
+    SetEnvironmentVariableA("_NT_SYMBOL_PATH", NULL);
+    SetEnvironmentVariableA("_NT_ALT_SYMBOL_PATH", NULL);
+
+    ret = SymInitialize(GetCurrentProcess(), NULL, TRUE);
     ok(ret, "got error %u\n", GetLastError());
 
     test_stack_walk();
+    test_search_path();
 
     ret = SymCleanup(GetCurrentProcess());
     ok(ret, "got error %u\n", GetLastError());