[PATCH v12 0/3] MR7539: combase : clsid_from_string_reg() add read of CurVer key value.
If the read ProgID only has a CurVer key and no CLSID key, it will directly return CO_E_CCLASSSTRING, causing the program to fail to continue running. Add the key value for finding CurVer. -- v12: combase:clsid_from_string_reg() add read of CurVer key value. ole32:Clear GUID on failed registry lookup for non-GUID strings. ole32/tests:Remove now succeeding todo_wine https://gitlab.winehq.org/wine/wine/-/merge_requests/7539
From: Maotong Zhang <zmtong1988(a)gmail.com> --- dlls/ole32/tests/compobj.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/dlls/ole32/tests/compobj.c b/dlls/ole32/tests/compobj.c index 3f358a8ef8e..b5a4e88b132 100644 --- a/dlls/ole32/tests/compobj.c +++ b/dlls/ole32/tests/compobj.c @@ -526,15 +526,11 @@ static void test_CLSIDFromProgID(void) ok(!ret, "Failed to create a test key.\n"); hr = CLSIDFromProgID(L"MyApp.DocumentTest", &clsid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_non_existent), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromProgID(L"MyApp.DocumentTest.1", &clsid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_non_existent), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromProgID(L"MyApp.DocumentTest.2", &clsid); @@ -552,15 +548,11 @@ static void test_CLSIDFromProgID(void) ok(IsEqualCLSID(&clsid, &CLSID_NULL), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromString(L"MyApp.DocumentTest", &clsid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_non_existent), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromString(L"MyApp.DocumentTest.1", &clsid); - todo_wine ok(hr == S_OK, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_non_existent), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); hr = CLSIDFromString(L"MyApp.DocumentTest.2", &clsid); @@ -569,15 +561,12 @@ static void test_CLSIDFromProgID(void) clsid = CLSID_StdFont; hr = CLSIDFromString(L"MyApp.DocumentTest.3", &clsid); - todo_wine ok(hr == REGDB_E_INVALIDVALUE, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_StdFont), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); clsid = CLSID_StdFont; hr = CLSIDFromString(L"MyApp.DocumentTest.5", &clsid); ok(hr == CO_E_CLASSSTRING, "Unexpected hr %#lx.\n", hr); - todo_wine ok(IsEqualCLSID(&clsid, &CLSID_StdFont), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); RegDeleteTreeW(HKEY_CLASSES_ROOT, L"MyApp.DocumentTest"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7539
From: Maotong Zhang <zmtong1988(a)gmail.com> guid_from_string() now clears the GUID if open_classes_key() fails. --- dlls/combase/combase.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dlls/combase/combase.c b/dlls/combase/combase.c index b3a1c9bd8fc..89cbd720f9c 100644 --- a/dlls/combase/combase.c +++ b/dlls/combase/combase.c @@ -1330,11 +1330,17 @@ static const BYTE guid_conv_table[256] = static BOOL guid_from_string(LPCWSTR s, GUID *id) { int i; + HKEY xhkey = NULL; if (!s || s[0] != '{') { - memset(id, 0, sizeof(*id)); - if (!s) return TRUE; + if (!s) + { + memset(id, 0, sizeof(*id)); + return TRUE; + } + if (open_classes_key(HKEY_CLASSES_ROOT, s, KEY_READ, &xhkey) != ERROR_SUCCESS) + memset(id, 0, sizeof(*id)); return FALSE; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7539
From: Maotong Zhang <zmtong1988(a)gmail.com> --- dlls/combase/combase.c | 89 +++++++++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 23 deletions(-) diff --git a/dlls/combase/combase.c b/dlls/combase/combase.c index 89cbd720f9c..87e6b2e48d5 100644 --- a/dlls/combase/combase.c +++ b/dlls/combase/combase.c @@ -1389,35 +1389,78 @@ static BOOL guid_from_string(LPCWSTR s, GUID *id) return FALSE; } -static HRESULT clsid_from_string_reg(LPCOLESTR progid, CLSID *clsid) +static HRESULT clsid_from_string_reg(LPCOLESTR progid, CLSID *clsid, BOOL forceassign) { - WCHAR buf2[CHARS_IN_GUID]; - LONG buf2len = sizeof(buf2); - HKEY xhkey; - WCHAR *buf; + HRESULT ret = CO_E_CLASSSTRING; + HKEY xhkey = NULL; + WCHAR szclsid[256] = {0}; + LONG cbclsid = sizeof(szclsid); + CLSID origclsid = *clsid; + static LPCOLESTR visited[16]; + static int visitedcount = 0; + BOOL hitlimit; + + if (progid == NULL) + return E_INVALIDARG; - memset(clsid, 0, sizeof(*clsid)); - buf = malloc((lstrlenW(progid) + 8) * sizeof(WCHAR)); - if (!buf) return E_OUTOFMEMORY; + if (*progid == 0) + return CO_E_CLASSSTRING; - lstrcpyW(buf, progid); - lstrcatW(buf, L"\\CLSID"); - if (open_classes_key(HKEY_CLASSES_ROOT, buf, MAXIMUM_ALLOWED, &xhkey)) + /* Avoid infinite recursion: limit to 16 levels of ProgID redirection */ + hitlimit = (visitedcount >= 16); + for (int i = 0; i < visitedcount && !hitlimit; i++) { - free(buf); - WARN("couldn't open key for ProgID %s\n", debugstr_w(progid)); - return CO_E_CLASSSTRING; + if (lstrcmpiW(progid, visited[i]) == 0) + hitlimit = TRUE; } - free(buf); - if (RegQueryValueW(xhkey, NULL, buf2, &buf2len)) + if (hitlimit) { - RegCloseKey(xhkey); - WARN("couldn't query clsid value for ProgID %s\n", debugstr_w(progid)); - return CO_E_CLASSSTRING; + *clsid = forceassign ? GUID_NULL : origclsid; + return forceassign ? CO_E_CLASSSTRING : REGDB_E_INVALIDVALUE; } - RegCloseKey(xhkey); - return guid_from_string(buf2, clsid) ? S_OK : CO_E_CLASSSTRING; + + visited[visitedcount++] = progid; + + if (SUCCEEDED(open_classes_key(HKEY_CLASSES_ROOT, progid, MAXIMUM_ALLOWED, &xhkey))) + { + if (RegQueryValueW(xhkey, L"CLSID", szclsid, &cbclsid) != ERROR_SUCCESS) + { + LONG cbcurver = 0; + if (RegQueryValueW(xhkey, L"CurVer", NULL, &cbcurver) == ERROR_SUCCESS) + { + if(cbcurver > 0) + { + WCHAR *szcurver = (WCHAR *)LocalAlloc(LMEM_FIXED, cbcurver); + if (szcurver && RegQueryValueW(xhkey, L"CurVer", szcurver, &cbcurver) == ERROR_SUCCESS) + { + if (cbcurver >= sizeof(WCHAR)) + szcurver[(cbcurver / sizeof(WCHAR)) - 1] = 0; + + if (szcurver[0] !=0 && lstrcmpiW(szcurver, progid) != 0) + { + RegCloseKey(xhkey); + ret = clsid_from_string_reg(szcurver, clsid, forceassign); + LocalFree(szcurver); + goto cleanup; + } + } + if (szcurver) LocalFree(szcurver); + } + } + } + else + ret = guid_from_string(szclsid, clsid) ? S_OK : REGDB_E_INVALIDVALUE; + } + +cleanup: + if (xhkey) + RegCloseKey(xhkey); + if (FAILED(ret)) + *clsid = forceassign ? GUID_NULL : origclsid; + visitedcount--; + + return ret; } /****************************************************************************** @@ -1440,7 +1483,7 @@ HRESULT WINAPI DECLSPEC_HOTPATCH CLSIDFromProgID(LPCOLESTR progid, CLSID *clsid) return S_OK; } - return clsid_from_string_reg(progid, clsid); + return clsid_from_string_reg(progid, clsid, TRUE); } /****************************************************************************** @@ -1468,7 +1511,7 @@ HRESULT WINAPI CLSIDFromString(LPCOLESTR str, LPCLSID clsid) return S_OK; /* It appears a ProgID is also valid */ - hr = clsid_from_string_reg(str, &tmp_id); + hr = clsid_from_string_reg(str, &tmp_id, FALSE); if (SUCCEEDED(hr)) *clsid = tmp_id; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7539
On Wed Sep 24 10:11:44 2025 +0000, Nikolay Sivov wrote:
Submitted tests separately !9021. Thanks, the issue’s fixed. Anything else that still needs optimizing ?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7539#note_116815
Nikolay Sivov (@nsivov) commented about dlls/combase/combase.c:
+ } + } + if (szcurver) LocalFree(szcurver); + } + } + } + else + ret = guid_from_string(szclsid, clsid) ? S_OK : REGDB_E_INVALIDVALUE; + } + +cleanup: + if (xhkey) + RegCloseKey(xhkey); + if (FAILED(ret)) + *clsid = forceassign ? GUID_NULL : origclsid; + visitedcount--; Why do you need this line?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7539#note_116879
Nikolay Sivov (@nsivov) commented about dlls/combase/combase.c:
+ hitlimit = (visitedcount >= 16); + for (int i = 0; i < visitedcount && !hitlimit; i++) + { + if (lstrcmpiW(progid, visited[i]) == 0) + hitlimit = TRUE; } - free(buf);
- if (RegQueryValueW(xhkey, NULL, buf2, &buf2len)) + if (hitlimit) { - RegCloseKey(xhkey); - WARN("couldn't query clsid value for ProgID %s\n", debugstr_w(progid)); - return CO_E_CLASSSTRING; + *clsid = forceassign ? GUID_NULL : origclsid; + return forceassign ? CO_E_CLASSSTRING : REGDB_E_INVALIDVALUE; It's better to handle such differences at the caller.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7539#note_116878
Nikolay Sivov (@nsivov) commented about dlls/combase/combase.c:
int i; + HKEY xhkey = NULL;
if (!s || s[0] != '{') { - memset(id, 0, sizeof(*id)); - if (!s) return TRUE; + if (!s) + { + memset(id, 0, sizeof(*id)); + return TRUE; + } + if (open_classes_key(HKEY_CLASSES_ROOT, s, KEY_READ, &xhkey) != ERROR_SUCCESS) + memset(id, 0, sizeof(*id)); return FALSE; }
This function has nothing to do with registry keys. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7539#note_116880
Nikolay Sivov (@nsivov) commented about dlls/combase/combase.c:
}
-static HRESULT clsid_from_string_reg(LPCOLESTR progid, CLSID *clsid) +static HRESULT clsid_from_string_reg(LPCOLESTR progid, CLSID *clsid, BOOL forceassign) { - WCHAR buf2[CHARS_IN_GUID]; - LONG buf2len = sizeof(buf2); - HKEY xhkey; - WCHAR *buf; + HRESULT ret = CO_E_CLASSSTRING; + HKEY xhkey = NULL; + WCHAR szclsid[256] = {0}; + LONG cbclsid = sizeof(szclsid); + CLSID origclsid = *clsid; + static LPCOLESTR visited[16]; + static int visitedcount = 0; Why is this static and why are you using specifically 16 elements?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7539#note_116877
On Fri Sep 26 14:16:05 2025 +0000, Nikolay Sivov wrote:
This function has nothing to do with registry keys. Sorry, I think I messed it up. I’ll restore it.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7539#note_116926
participants (3)
-
Maotong Zhang -
Maotong Zhang (＠xiaotong)
-
Nikolay Sivov (＠nsivov)