[PATCH v4 0/2] MR9639: Fix the integration of PowerPoint add-in in MS Office 2016
Encountered an issue with PowerPoint add-in integration. Reviewing the logs revealed the problem was due to the lack of CLSID lookup via the CurVer section and incorrect handling of the PARAMFLAG_NONE flag. A test was conducted in Windows to verify the handling of PARAMFLAG_NONE, PARAMFLAG_FIN, and PARAMFLAG_FOUT. Test class: ``` interface ITest : IDispatch { [id(1)] HRESULT TestMethod(BSTR* value); [id(2)] HRESULT TestMethodOut([out] BSTR* retVal); [id(3)] HRESULT TestMethodIn([in] BSTR* inVal); }; ``` Results in Windows: ``` --------TestMethod-------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethod] Hello from VT_BSTR Invoke returned: 0x00000000 -------TestMethodIn------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethodIn] Hello from VT_BSTR Invoke returned: 0x00000000 -------TestMethodOut------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethodOut] (null) Invoke returned: 0x00000000 ``` Results in Wine: ``` --------TestMethod-------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethod] (null) Invoke returned: 0x00000000 -------TestMethodIn------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethodIn] Hello from VT_BSTR Invoke returned: 0x00000000 -------TestMethodOut------- Variant type: 0x0008 (VT_BSTR) [CTest::TestMethodOut] (null) ``` Therefore, parameters with the PARAMFLAG_NONE flag are implied to be input parameters in Windows and are handled accordingly as PARAMFLAG_FIN. Added tests for the implemented changes. In the last two tests (their code is provided below), the CLSID is nullified in the guid_from_string function, which is why this test is left with a todo_wine tag. ``` clsid = CLSID_StdFont; hr = CLSIDFromString(L"MyApp.DocumentTest.3", &clsid); 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); todo_wine ok(IsEqualCLSID(&clsid, &CLSID_StdFont), "Unexpected clsid %s.\n", wine_dbgstr_guid(&clsid)); ``` -- v4: combase: Implement CLSID lookup with CurVer fallback oleaut32: Fix parameter value loss in ITypeInfo_Invoke for byref parameters https://gitlab.winehq.org/wine/wine/-/merge_requests/9639
From: Ivan Lyugaev <valy@etersoft.ru> --- dlls/oleaut32/tests/test_reg.idl | 6 ++++ dlls/oleaut32/tests/typelib.c | 50 ++++++++++++++++++++++++++++++++ dlls/oleaut32/typelib.c | 2 +- 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/dlls/oleaut32/tests/test_reg.idl b/dlls/oleaut32/tests/test_reg.idl index 2ab7638f2fc..4b6c11d81bf 100644 --- a/dlls/oleaut32/tests/test_reg.idl +++ b/dlls/oleaut32/tests/test_reg.idl @@ -171,6 +171,12 @@ library register_test void testget8([out, retval] int* value); [propput, id(11)] void testput8([in] int value); + [id(12)] + HRESULT testbstrnone(BSTR* value, [out, retval] BSTR* retvalue); + [id(13)] + HRESULT testbstrin([in] BSTR* value, [out, retval] BSTR* retvalue); + [id(14)] + HRESULT testbstrout([out] BSTR* value, [out, retval] BSTR* retvalue); } /* uuid is same as for test_struct2 in test_tlb.idl, fields are different */ diff --git a/dlls/oleaut32/tests/typelib.c b/dlls/oleaut32/tests/typelib.c index 153964e17d5..b54e6be0806 100644 --- a/dlls/oleaut32/tests/typelib.c +++ b/dlls/oleaut32/tests/typelib.c @@ -316,6 +316,33 @@ static void WINAPI invoketest_get_testput8(IInvokeTest *iface, int value) { } +static HRESULT WINAPI invoketest_test_bstr_none(IInvokeTest *iface, BSTR* value, BSTR* retvalue) +{ + if (value != NULL && *value != NULL) + { + *retvalue = SysAllocString(*value); + } + return S_OK; +} + +static HRESULT WINAPI invoketest_test_bstr_in(IInvokeTest *iface, BSTR* value, BSTR* retvalue) +{ + if (value != NULL && *value != NULL) + { + *retvalue = SysAllocString(*value); + } + return S_OK; +} + +static HRESULT WINAPI invoketest_test_bstr_out(IInvokeTest *iface, BSTR* value, BSTR* retvalue) +{ + if (value != NULL && *value != NULL) + { + *retvalue = SysAllocString(*value); + } + return S_OK; +} + static const IInvokeTestVtbl invoketestvtbl = { invoketest_QueryInterface, invoketest_AddRef, @@ -337,6 +364,9 @@ static const IInvokeTestVtbl invoketestvtbl = { invoketest_get_testget7, invoketest_get_testget8, invoketest_get_testput8, + invoketest_test_bstr_none, + invoketest_test_bstr_in, + invoketest_test_bstr_out }; static IInvokeTest invoketest = { &invoketestvtbl }; @@ -868,6 +898,26 @@ static void test_invoke_func(ITypeInfo *typeinfo) dp.cArgs = 2; hres = ITypeInfo_Invoke(typeinfo, &invoketest, 3, DISPATCH_METHOD, &dp, &res, NULL, &i); ok(hres == DISP_E_BADPARAMCOUNT, "got 0x%08lx\n", hres); + + V_VT(args) = VT_BSTR; + V_BSTR(args) = SysAllocString(L"TestString"); + dp.cArgs = 1; + dp.cNamedArgs = 0; + + hres = ITypeInfo_Invoke(typeinfo, &invoketest, 12, DISPATCH_METHOD, &dp, &res, NULL, &i); + ok(hres == S_OK, "got 0x%08lx\n", hres); + ok(V_VT(&res) == VT_BSTR, "got %d\n", V_VT(&res)); + ok(!lstrcmpW(V_BSTR(&res), L"TestString"), "got %ls\n", !lstrcmpW(V_BSTR(&res), L"") ? L"(null)" : V_BSTR(&res)); + + hres = ITypeInfo_Invoke(typeinfo, &invoketest, 13, DISPATCH_METHOD, &dp, &res, NULL, &i); + ok(hres == S_OK, "got 0x%08lx\n", hres); + ok(V_VT(&res) == VT_BSTR, "got %d\n", V_VT(&res)); + ok(!lstrcmpW(V_BSTR(&res), L"TestString"), "got %ls\n", V_BSTR(&res)); + + hres = ITypeInfo_Invoke(typeinfo, &invoketest, 14, DISPATCH_METHOD, &dp, &res, NULL, &i); + ok(hres == S_OK, "got 0x%08lx\n", hres); + ok(V_VT(&res) == VT_BSTR, "got %d\n", V_VT(&res)); + ok(V_BSTR(&res) == NULL || !lstrcmpW(V_BSTR(&res), L""), "got %ls\n", V_BSTR(&res)); } static WCHAR *create_test_typelib(int res_no) diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c index c4f00ca5bea..d5129a2c5f9 100644 --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -7326,7 +7326,7 @@ static HRESULT WINAPI ITypeInfo_fnInvoke( } else if ((rgvt[i] & VT_BYREF) && !V_ISBYREF(src_arg)) { - if (wParamFlags & PARAMFLAG_FIN) + if ((wParamFlags & PARAMFLAG_FIN) || (wParamFlags == PARAMFLAG_NONE)) hres = VariantChangeType(&missing_arg[i], src_arg, 0, rgvt[i] & ~VT_BYREF); else V_VT(&missing_arg[i]) = rgvt[i] & ~VT_BYREF; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9639
From: Ivan Lyugaev <valy@etersoft.ru> Some COM applications register version-independent ProgIDs that point to version-specific classes through the CurVer registry key. This implements the correct logic for handling such cases --- dlls/combase/combase.c | 64 ++++++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 8 deletions(-) diff --git a/dlls/combase/combase.c b/dlls/combase/combase.c index 34a9a42dc2a..d1284be885c 100644 --- a/dlls/combase/combase.c +++ b/dlls/combase/combase.c @@ -1454,22 +1454,70 @@ static HRESULT clsid_from_string_reg(LPCOLESTR progid, CLSID *clsid) lstrcpyW(buf, progid); lstrcatW(buf, L"\\CLSID"); - if (open_classes_key(HKEY_CLASSES_ROOT, buf, MAXIMUM_ALLOWED, &xhkey)) + if (!open_classes_key(HKEY_CLASSES_ROOT, buf, MAXIMUM_ALLOWED, &xhkey)) { - free(buf); + if (!RegQueryValueW(xhkey, NULL, buf2, &buf2len)) + { + RegCloseKey(xhkey); + free(buf); + return guid_from_string(buf2, clsid) ? S_OK : CO_E_CLASSSTRING; + } WARN("couldn't open key for ProgID %s\n", debugstr_w(progid)); - return CO_E_CLASSSTRING; + RegCloseKey(xhkey); + xhkey = NULL; } free(buf); - if (RegQueryValueW(xhkey, NULL, buf2, &buf2len)) + buf = malloc((lstrlenW(progid) + 8) * sizeof(WCHAR)); + if (!buf) return E_OUTOFMEMORY; + lstrcpyW(buf, progid); + lstrcatW(buf, L"\\CurVer"); + + if (!open_classes_key(HKEY_CLASSES_ROOT, buf, MAXIMUM_ALLOWED, &xhkey)) { - RegCloseKey(xhkey); - WARN("couldn't query clsid value for ProgID %s\n", debugstr_w(progid)); - return CO_E_CLASSSTRING; + LONG curverlen = 0; + if (!RegQueryValueW(xhkey, NULL, NULL, &curverlen)) + { + WCHAR *curver = malloc(curverlen * sizeof(WCHAR)); + if (!curver) + { + free(buf); + RegCloseKey(xhkey); + return E_OUTOFMEMORY; + } + if (!RegQueryValueW(xhkey, NULL, curver, &curverlen)) + { + RegCloseKey(xhkey); + free(buf); + + buf = malloc((lstrlenW(curver) + 8) * sizeof(WCHAR)); + if (!buf) + { + free(curver); + return E_OUTOFMEMORY; + } + + lstrcpyW(buf, curver); + lstrcatW(buf, L"\\CLSID"); + if(!open_classes_key(HKEY_CLASSES_ROOT, buf, MAXIMUM_ALLOWED, &xhkey)) + { + if (RegQueryValueW(xhkey, NULL, buf2, &buf2len)) + { + RegCloseKey(xhkey); + WARN("couldn't query clsid value for ProgID %s\n", debugstr_w(progid)); + return CO_E_CLASSSTRING; + } + free(buf); + free(curver); + RegCloseKey(xhkey); + return guid_from_string(buf2, clsid) ? S_OK : CO_E_CLASSSTRING; + } + } + } } + if (buf) free(buf); RegCloseKey(xhkey); - return guid_from_string(buf2, clsid) ? S_OK : CO_E_CLASSSTRING; + return CO_E_CLASSSTRING; } /****************************************************************************** -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9639
On Tue Dec 2 13:37:38 2025 +0000, Ivan Lyugaev wrote:
Hi! I only implemented one level of CurVer search, and that turned out to be enough to solve my problem. I noticed an error in the Windows tests for typelib.c: "got (null)". I changed the check for 'res', and now the error is gone. Also, I would like to ask if there will be a further review of this change?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9639#note_125566
On Tue Dec 16 19:13:56 2025 +0000, Elizabeth Figura wrote:
It's not ideal how long review has taken thus far, but regardless, it probably won't be reviewed, and certainly won't be committed, until code freeze is over, probably a month or so from now. Hi, I'm assuming the code freeze is over now. Could you please resume the reviews?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9639#note_127636
Nikolay Sivov (@nsivov) commented about dlls/oleaut32/typelib.c:
} else if ((rgvt[i] & VT_BYREF) && !V_ISBYREF(src_arg)) { - if (wParamFlags & PARAMFLAG_FIN) + if ((wParamFlags & PARAMFLAG_FIN) || (wParamFlags == PARAMFLAG_NONE)) hres = VariantChangeType(&missing_arg[i], src_arg, 0, rgvt[i] & ~VT_BYREF);
Maybe it should be more like !(wParamFlags & (FIN|FOUT))? This needs some experimenting. Or maybe PARAMFLAG_FIN is reported by typelib API, in the absence of IN/OUT/RETVAL. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9639#note_127637
On Wed Jan 21 17:59:10 2026 +0000, Nikolay Sivov wrote:
Maybe it should be more like !(wParamFlags & (FIN|FOUT))? This needs some experimenting. Or maybe PARAMFLAG_FIN is reported by typelib API, in the absence of IN/OUT/RETVAL. I'm not sure if that condition will work correctly because I've only been able to test for the PARAMFLAG_NONE flag. I can provide a test COM class to confirm my point (if the Wine test suite isn't enough).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9639#note_127640
On Wed Jan 21 19:20:33 2026 +0000, Ivan Lyugaev wrote:
I'm not sure if that condition will work correctly because I've only been able to test for the PARAMFLAG_NONE flag. I can provide a test COM class to confirm my point (if the Wine test suite isn't enough). @nsivov, are you going to continue with the review of my merge request?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9639#note_128229
On Wed Jan 28 06:13:44 2026 +0000, Ivan Lyugaev wrote:
@nsivov, are you going to continue with the review of my merge request? Both commits need work in my opinion. I already suggested what else to check for the typelib change.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9639#note_128237
participants (3)
-
Ivan Lyugaev -
Ivan Lyugaev (＠valy)
-
Nikolay Sivov (＠nsivov)