Signed-off-by: Mohamad Al-Jaf mohamadaljaf@gmail.com --- v2: - Support byref BSTR. - Support nested VARIANT including byref BSTR. --- dlls/propsys/propsys.spec | 2 +- dlls/propsys/propvar.c | 46 +++++++++++++++++++++++++++++++++++++++ include/propvarutil.h | 6 +++++ 3 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/dlls/propsys/propsys.spec b/dlls/propsys/propsys.spec index 15749952e95..7391cf4aaa2 100644 --- a/dlls/propsys/propsys.spec +++ b/dlls/propsys/propsys.spec @@ -194,7 +194,7 @@ @ stub VariantToStringAlloc @ stub VariantToStringArray @ stub VariantToStringArrayAlloc -@ stub VariantToStringWithDefault +@ stdcall VariantToStringWithDefault(ptr wstr) @ stub VariantToUInt16 @ stub VariantToUInt16Array @ stub VariantToUInt16ArrayAlloc diff --git a/dlls/propsys/propvar.c b/dlls/propsys/propvar.c index 8f6c52f7aa7..09f9a8aa892 100644 --- a/dlls/propsys/propvar.c +++ b/dlls/propsys/propvar.c @@ -416,6 +416,52 @@ PCWSTR WINAPI PropVariantToStringWithDefault(REFPROPVARIANT propvarIn, LPCWSTR p return pszDefault; }
+/****************************************************************** + * VariantToStringWithDefault (PROPSYS.@) + */ +PCWSTR WINAPI VariantToStringWithDefault(const VARIANT *pvar, LPCWSTR pszDefault) +{ + TRACE("(%p, %s)\n", pvar, debugstr_w(pszDefault)); + + switch(V_VT(pvar)) + { + case VT_BSTR: + { + if (V_BSTR(pvar) == NULL) + return L""; + + return V_BSTR(pvar); + } + case VT_BYREF | VT_BSTR: + { + if (*V_BSTRREF(pvar) == NULL) + return L""; + + return *V_BSTRREF(pvar); + } + case VT_BYREF | VT_VARIANT: + { + if (V_VT(V_VARIANTREF(pvar)) == VT_BSTR) + { + if (V_BSTR(V_VARIANTREF(pvar)) == NULL) + return L""; + + return V_BSTR(V_VARIANTREF(pvar)); + } + + if (V_VT(V_VARIANTREF(pvar)) == (VT_BYREF | VT_BSTR)) + { + if (*V_BSTRREF(V_VARIANTREF(pvar)) == NULL) + return L""; + + return *V_BSTRREF(V_VARIANTREF(pvar)); + } + } + } + + return pszDefault; +} +
/****************************************************************** * PropVariantChangeType (PROPSYS.@) diff --git a/include/propvarutil.h b/include/propvarutil.h index 36a670f56e6..2515d71b3b6 100644 --- a/include/propvarutil.h +++ b/include/propvarutil.h @@ -93,6 +93,7 @@ HRESULT WINAPI PropVariantToBoolean(REFPROPVARIANT propvarIn, BOOL *ret); HRESULT WINAPI PropVariantToBuffer(REFPROPVARIANT propvarIn, void *ret, UINT cb); HRESULT WINAPI PropVariantToString(REFPROPVARIANT propvarIn, PWSTR ret, UINT cch); PCWSTR WINAPI PropVariantToStringWithDefault(REFPROPVARIANT propvarIn, LPCWSTR pszDefault); +PCWSTR WINAPI VariantToStringWithDefault(const VARIANT *pvar, LPCWSTR pszDefault);
HRESULT WINAPI PropVariantToStringAlloc(REFPROPVARIANT propvarIn, WCHAR **ret);
@@ -201,6 +202,11 @@ inline BOOL IsPropVariantString(REFPROPVARIANT propvar) return (PropVariantToStringWithDefault(propvar, NULL) != NULL); }
+inline BOOL IsVariantString(const VARIANT *pvar) +{ + return (VariantToStringWithDefault(pvar, NULL) != NULL); +} + #endif /* NO_PROPVAR_INLINES */ #endif /* __cplusplus */
Signed-off-by: Mohamad Al-Jaf mohamadaljaf@gmail.com --- v2: - Add some more tests to see what variants are supported. - Test for BYREF. - Test nested BSTR and BYREF.
I added a nested nested VARIANT test and it showed that Windows returns the default value. However, it causes propsys to crash in 64-bit tests, namely propsys_test64.exe.
How do I compile 64-bit tests like propsys_test64.exe? It's different from propsys_test.exe in the 64-build of Wine. --- dlls/propsys/tests/propsys.c | 179 +++++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+)
diff --git a/dlls/propsys/tests/propsys.c b/dlls/propsys/tests/propsys.c index 124b3405bcb..776f70ae726 100644 --- a/dlls/propsys/tests/propsys.c +++ b/dlls/propsys/tests/propsys.c @@ -2064,6 +2064,184 @@ static void test_InitVariantFromFileTime(void) ok(V_DATE(&var) == d, "got wrong value: %f, expected %f\n", V_DATE(&var), d); }
+static void test_VariantToStringWithDefault(void) +{ + static WCHAR default_value[] = {'t', 'e', 's', 't', 0}; + static WCHAR wstr_test[] = {'t', 'e', 's', 't', '2', 0}; + static WCHAR wstr_empty[] = {0}; + static WCHAR wstr_space[] = {' ', 0}; + LPCWSTR result; + VARIANT var, nes; + BSTR b; + + V_VT(&var) = VT_EMPTY; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_NULL; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_BOOL; + V_BOOL(&var) = VARIANT_TRUE; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_CY; + V_CY(&var).int64 = 100000; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_DATE; + V_DATE(&var) = 42.0; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_BYREF; + V_BYREF(&var) = &wstr_test; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_ERROR; + V_ERROR(&var) = DISP_E_PARAMNOTFOUND; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_I4; + V_I4(&var) = 15; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_I1; + V_I1(&var) = 't'; + result = VariantToStringWithDefault(&var, default_value); + ok(result == default_value, "Unexpected value %s\n", wine_dbgstr_w(result)); + + /* V_BSTR */ + + V_VT(&var) = VT_BSTR; + V_BSTR(&var) = NULL; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_BSTR; + V_BSTR(&var) = SysAllocString(wstr_empty); + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(V_BSTR(&var)); + + V_VT(&var) = VT_BSTR; + V_BSTR(&var) = SysAllocString(wstr_space); + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_space), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(V_BSTR(&var)); + + V_VT(&var) = VT_BSTR; + V_BSTR(&var) = SysAllocString(wstr_test); + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_test), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(V_BSTR(&var)); + + /* V_BSTRREF */ + + V_VT(&var) = VT_BYREF | VT_BSTR; + b = NULL; + V_BSTRREF(&var) = &b; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_BYREF | VT_BSTR; + b = SysAllocString(wstr_empty); + V_BSTRREF(&var) = &b; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(b); + + V_VT(&var) = VT_BYREF | VT_BSTR; + b = SysAllocString(wstr_space); + V_BSTRREF(&var) = &b; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_space), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(b); + + V_VT(&var) = VT_BYREF | VT_BSTR; + b = SysAllocString(wstr_test); + V_BSTRREF(&var) = &b; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_test), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(b); + + /* Nested V_BSTR */ + + V_VT(&var) = VT_BYREF | VT_VARIANT; + V_VT(&nes) = VT_BSTR; + V_BSTR(&nes) = NULL; + V_VARIANTREF(&var) = &nes; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_BYREF | VT_VARIANT; + V_VT(&nes) = VT_BSTR; + V_BSTR(&nes) = SysAllocString(wstr_empty); + V_VARIANTREF(&var) = &nes; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(V_BSTR(&nes)); + + V_VT(&var) = VT_BYREF | VT_VARIANT; + V_VT(&nes) = VT_BSTR; + V_BSTR(&nes) = SysAllocString(wstr_space); + V_VARIANTREF(&var) = &nes; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_space), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(V_BSTR(&nes)); + + V_VT(&var) = VT_BYREF | VT_VARIANT; + V_VT(&nes) = VT_BSTR; + V_BSTR(&nes) = SysAllocString(wstr_test); + V_VARIANTREF(&var) = &nes; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_test), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(V_BSTR(&nes)); + + /* Nested V_BSTRREF */ + + V_VT(&var) = VT_BYREF | VT_VARIANT; + V_VT(&nes) = VT_BYREF | VT_BSTR; + b = NULL; + V_BSTRREF(&nes) = &b; + V_VARIANTREF(&var) = &nes; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result)); + + V_VT(&var) = VT_BYREF | VT_VARIANT; + V_VT(&nes) = VT_BYREF | VT_BSTR; + b = SysAllocString(wstr_empty); + V_BSTRREF(&nes) = &b; + V_VARIANTREF(&var) = &nes; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(b); + + V_VT(&var) = VT_BYREF | VT_VARIANT; + V_VT(&nes) = VT_BYREF | VT_BSTR; + b = SysAllocString(wstr_space); + V_BSTRREF(&nes) = &b; + V_VARIANTREF(&var) = &nes; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_space), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(b); + + V_VT(&var) = VT_BYREF | VT_VARIANT; + V_VT(&nes) = VT_BYREF | VT_BSTR; + b = SysAllocString(wstr_test); + V_BSTRREF(&nes) = &b; + V_VARIANTREF(&var) = &nes; + result = VariantToStringWithDefault(&var, default_value); + ok(!lstrcmpW(result, wstr_test), "Unexpected value %s\n", wine_dbgstr_w(result)); + SysFreeString(b); +} + START_TEST(propsys) { test_PSStringFromPropertyKey(); @@ -2088,4 +2266,5 @@ START_TEST(propsys) test_propertystore(); test_PSCreatePropertyStoreFromObject(); test_InitVariantFromFileTime(); + test_VariantToStringWithDefault(); }
On 3/28/22 03:16, Mohamad Al-Jaf wrote:
- V_VT(&var) = VT_BSTR;
- V_BSTR(&var) = SysAllocString(wstr_empty);
- result = VariantToStringWithDefault(&var, default_value);
- ok(!lstrcmpW(result, wstr_empty), "Unexpected value %s\n", wine_dbgstr_w(result));
- SysFreeString(V_BSTR(&var));
Same as with previous version you sent. The function returns pointer directly, so let's test for that, instead of string comparison.
Another useful test would be with default_value == NULL.
Same as with previous version you sent. The function returns pointer directly, so let's test for that, instead of string comparison.
Sorry, but I really have no idea what you mean by that. I can't find a test that has an example of what you're talking about. Can you please provide an example of what you mean? Also, does this mean the test for PropVariantToStringWithDefault is wrong?
Another useful test would be with default_value == NULL.
Indeed, thanks. -- Kind regards, Mohamad
On 3/28/22 08:46, Mohamad Al-Jaf wrote:
Same as with previous version you sent. The function returns pointer directly, so let's test for that, instead of string comparison.
Sorry, but I really have no idea what you mean by that.
Same thing you do when testing that default string is returned.
On Mon, Mar 28, 2022 at 3:07 AM Nikolay Sivov nsivov@codeweavers.com wrote:
On 3/28/22 08:46, Mohamad Al-Jaf wrote:
Same as with previous version you sent. The function returns pointer directly, so let's test for that, instead of string comparison.
Sorry, but I really have no idea what you mean by that.
Same thing you do when testing that default string is returned.
I was getting confused by this because I thought that you have to do string comparison when testing strings in C.
Would just like to know why it's better to test the returned pointer value instead of string comparison? I'm guessing to also test if the size is the same?
-- Kind regards, Mohamad
Would this be better? It avoids repetitive if and return statements.
PCWSTR WINAPI VariantToStringWithDefault(const VARIANT *pvar, LPCWSTR pszDefault) { PCWSTR ret = pszDefault;
TRACE("(%p, %s)\n", pvar, debugstr_w(pszDefault));
switch(V_VT(pvar)) { case VT_BSTR: ret = V_BSTR(pvar); break; case VT_BYREF | VT_BSTR: ret = *V_BSTRREF(pvar); break; case VT_BYREF | VT_VARIANT: { if (V_VT(V_VARIANTREF(pvar)) == VT_BSTR) { ret = V_BSTR(V_VARIANTREF(pvar); break; } if (V_VT(V_VARIANTREF(pvar)) == (VT_BYREF | VT_BSTR)) { ret = *V_BSTRREF(V_VARIANTREF(pvar)); break; } } }
if (ret == NULL) return L"";
return ret; } -- Kind regards, Mohamad
On 3/28/22 03:16, Mohamad Al-Jaf wrote:
+/******************************************************************
- VariantToStringWithDefault (PROPSYS.@)
- */
+PCWSTR WINAPI VariantToStringWithDefault(const VARIANT *pvar, LPCWSTR pszDefault) +{
- TRACE("(%p, %s)\n", pvar, debugstr_w(pszDefault));
- switch(V_VT(pvar))
- {
case VT_BSTR:{if (V_BSTR(pvar) == NULL)return L"";return V_BSTR(pvar);}case VT_BYREF | VT_BSTR:{if (*V_BSTRREF(pvar) == NULL)return L"";return *V_BSTRREF(pvar);}case VT_BYREF | VT_VARIANT:{if (V_VT(V_VARIANTREF(pvar)) == VT_BSTR){if (V_BSTR(V_VARIANTREF(pvar)) == NULL)return L"";return V_BSTR(V_VARIANTREF(pvar));}if (V_VT(V_VARIANTREF(pvar)) == (VT_BYREF | VT_BSTR)){if (*V_BSTRREF(V_VARIANTREF(pvar)) == NULL)return L"";return *V_BSTRREF(V_VARIANTREF(pvar));}}- }
- return pszDefault;
+}
There is a lot of duplication you don't need. You can first dereference a variant if needed, and then use same path for both plain variant and variant reference. Similar for BSTR references.
On Mon, Mar 28, 2022 at 1:13 AM Nikolay Sivov nsivov@codeweavers.com wrote:
There is a lot of duplication you don't need. You can first dereference a variant if needed, and then use same path for both plain variant and variant reference. Similar for BSTR references.
Not sure if you also meant to dereference the BSTR, but it's a different member of the structure and I don't know how to do that.
-- Kind regards, Mohamad
-
Mohamad Al-Jaf -
Nikolay Sivov