[PATCH v12 0/3] MR7539: combase : clsid_from_string_reg() add read of CurVer key value.

[Maotong Zhang (＠xiaotong)]

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

[PATCH v12 1/3] ole32/tests:Remove now succeeding todo_wine

[Maotong Zhang]

From: Maotong Zhang

---
 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");

[PATCH v12 2/3] ole32:Clear GUID on failed registry lookup for non-GUID strings.

[Maotong Zhang]

From: Maotong Zhang

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;
     }

[PATCH v12 3/3] combase:clsid_from_string_reg() add read of CurVer key value.

[Maotong Zhang]

From: Maotong Zhang

---
 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;

[Maotong Zhang (＠xiaotong)]

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 ?

[Nikolay Sivov (＠nsivov)]

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?

[Nikolay Sivov (＠nsivov)]

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.

[Nikolay Sivov (＠nsivov)]

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?

[Nikolay Sivov (＠nsivov)]

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.

[Maotong Zhang (＠xiaotong)]

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.