oleaut32: Conformance test and patch for VarCat
me at benjaminarai.com
me at benjaminarai.com
Sat Jul 15 02:58:10 CDT 2006
Hi,
I have attached a patch that should address the issue you brought up. Let
me know if the patch works for you. Thanks for the input.
Benjamin Arai
me at benjaminarai.com
>
> --- qingdoa daoo <qingdao33122 at yahoo.com> wrote:
>
>> --- Benjamin Arai <me at benjaminarai.com> wrote:
>>
>> > See http://bugs.winehq.com/show_bug.cgi?id=5545
>> > License: LGPL
>> >
>> > Changelog:
>> > - oleaut32: Add conformance test for VarCat
>> > - oleaut32: Update VarCat function to address all conformance test
>> > failures
>> > - Tests all pass on Windows XP SP2 and Wine
>> >
>> > > >From 61bbc58d652c97b70309c0fd37107e60cbaad86a Mon Sep 17 00:00:00
>> 2001
>> > From: Benjamin Arai <barai at barai.smo.corp.google.com>
>> > Date: Mon, 10 Jul 2006 09:12:46 -0700
>> > Subject: [PATCH] oleaut32:VarCat - Adds conformance test and updates
>> VarCat to pass
>> all
>> > tests
>> > ---
>> > dlls/oleaut32/tests/vartest.c | 319
>> +++++++++++++++++++++++++++++++++++++++++
>> > dlls/oleaut32/variant.c | 150 +++++++++++++++----
>> > 2 files changed, 439 insertions(+), 30 deletions(-)
>> >
>> > diff --git a/dlls/oleaut32/tests/vartest.c
>> b/dlls/oleaut32/tests/vartest.c
>> > index e8e4654..ba7ba84 100644
>> > --- a/dlls/oleaut32/tests/vartest.c
>> > +++ b/dlls/oleaut32/tests/vartest.c
>> === message truncated ===>
>> >
>>
>> Be careful when you call VariantClear. Calling VariantClear on a random
>> value is
>> dangerous.
>>
>> Specifically,
>> Before a local variable is initialzed its content is undefined.
>> When a function call fails the content of the output buffer is usually
>> undefined.
>>
>
> Another problem.
>
> Your test for VarCat won't catch many cases in which native returns S_OK
> while our code
> fails.
>
> hres = VarCat(&left, &right, &result);
> ......
> if (hres != S_OK)
> {
> HRESULT expected_error_num;
> ......
>
> The test may pass on Windows because the hres is S_OK and on Wine because
> hres is the
> expected error num according to our test code.
>
>
>
>
> ___________________________________________________________
> ÇÀ×¢ÑÅ»¢Ãâ·ÑÓÊÏä-3.5GÈÝÁ¿£¬20M¸½¼þ£¡
> http://cn.mail.yahoo.com
>
>
>
-------------- next part --------------
>From 1fec7a54fb5f368310df3963491d91a05d84bdbd Mon Sep 17 00:00:00 2001
From: Benjamin Arai <me at benjaminarai.com>
Date: Sat, 15 Jul 2006 00:56:08 -0700
Subject: [PATCH] oleaut32:VarCat Updates conformance test and VarCat function
Changelog:
- Helps address http://bugs.winehq.com/show_bug.cgi?id=3628
- Updates VarCat conformance test
- Updates VarCat function to address conformance test failures
The current conformance contains a logic error allowing certain tests to pass when they should not. A test may pass on Windows because it returned without any errors (S_OK) but on Wine that same test can technically fail but be misinterpeted as a correct result in Wine if the expected error code is returned, allowing for error codes to be misintepeted as the correct result in specific cases. This patch addresses these issues. This patch does not fix bug 3628 but it does advance the application farther before getting new errors.
---
dlls/oleaut32/tests/vartest.c | 154 ++++++++++++++++++-----------------------
dlls/oleaut32/variant.c | 117 ++++++++++++++++++++++---------
2 files changed, 147 insertions(+), 124 deletions(-)
diff --git a/dlls/oleaut32/tests/vartest.c b/dlls/oleaut32/tests/vartest.c
index 5b1941e..da806d3 100644
--- a/dlls/oleaut32/tests/vartest.c
+++ b/dlls/oleaut32/tests/vartest.c
@@ -5109,6 +5109,7 @@ static void test_VarCat(void)
TCHAR orig_date_format[128];
VARTYPE leftvt, rightvt, resultvt;
HRESULT hres;
+ HRESULT expected_error_num;
/* Set date format for testing */
lcid = LOCALE_USER_DEFAULT;
@@ -5128,52 +5129,70 @@ static void test_VarCat(void)
for (rightvt = 0; rightvt <= VT_BSTR_BLOB; rightvt++)
{
- BOOL bFail = FALSE;
+
SKIPTESTS(rightvt);
+ expected_error_num = S_OK;
+ resultvt = VT_EMPTY;
if (leftvt == VT_DISPATCH || rightvt == VT_DISPATCH ||
- leftvt == VT_UNKNOWN || rightvt == VT_UNKNOWN ||
- leftvt == VT_RECORD || rightvt == VT_RECORD ||
- leftvt == VT_CLSID || rightvt == VT_CLSID ||
- leftvt == VT_BSTR_BLOB || rightvt == VT_BSTR_BLOB
- )
- continue;
+ leftvt == VT_UNKNOWN || rightvt == VT_UNKNOWN ||
+ leftvt == VT_RECORD || rightvt == VT_RECORD ||
+ leftvt == 15 || rightvt == 15 /* Undefined type */)
+ continue;
- if (leftvt == VT_ERROR || rightvt == VT_ERROR)
- resultvt = VT_EMPTY;
- else if (leftvt == VT_NULL && rightvt == VT_NULL)
+ if (leftvt == VT_NULL && rightvt == VT_NULL)
resultvt = VT_NULL;
+ else if (leftvt == VT_VARIANT && (rightvt == VT_ERROR ||
+ rightvt == VT_DATE || rightvt == VT_DECIMAL))
+ expected_error_num = DISP_E_TYPEMISMATCH;
else if ((leftvt == VT_I2 || leftvt == VT_I4 ||
- leftvt == VT_R4 || leftvt == VT_R8 ||
- leftvt == VT_CY || leftvt == VT_BOOL ||
- leftvt == VT_BSTR || leftvt == VT_I1 ||
- leftvt == VT_UI1 || leftvt == VT_UI2 ||
- leftvt == VT_UI4 || leftvt == VT_I8 ||
- leftvt == VT_UI8 || leftvt == VT_INT ||
- leftvt == VT_UINT || leftvt == VT_EMPTY ||
- leftvt == VT_NULL || leftvt == VT_DECIMAL ||
- leftvt == VT_DATE)
- &&
- (rightvt == VT_I2 || rightvt == VT_I4 ||
- rightvt == VT_R4 || rightvt == VT_R8 ||
- rightvt == VT_CY || rightvt == VT_BOOL ||
- rightvt == VT_BSTR || rightvt == VT_I1 ||
- rightvt == VT_UI1 || rightvt == VT_UI2 ||
- rightvt == VT_UI4 || rightvt == VT_I8 ||
- rightvt == VT_UI8 || rightvt == VT_INT ||
- rightvt == VT_UINT || rightvt == VT_EMPTY ||
- rightvt == VT_NULL || rightvt == VT_DECIMAL ||
- rightvt == VT_DATE))
- {
+ leftvt == VT_R4 || leftvt == VT_R8 ||
+ leftvt == VT_CY || leftvt == VT_BOOL ||
+ leftvt == VT_BSTR || leftvt == VT_I1 ||
+ leftvt == VT_UI1 || leftvt == VT_UI2 ||
+ leftvt == VT_UI4 || leftvt == VT_I8 ||
+ leftvt == VT_UI8 || leftvt == VT_INT ||
+ leftvt == VT_UINT || leftvt == VT_EMPTY ||
+ leftvt == VT_NULL || leftvt == VT_DECIMAL ||
+ leftvt == VT_DATE)
+ &&
+ (rightvt == VT_I2 || rightvt == VT_I4 ||
+ rightvt == VT_R4 || rightvt == VT_R8 ||
+ rightvt == VT_CY || rightvt == VT_BOOL ||
+ rightvt == VT_BSTR || rightvt == VT_I1 ||
+ rightvt == VT_UI1 || rightvt == VT_UI2 ||
+ rightvt == VT_UI4 || rightvt == VT_I8 ||
+ rightvt == VT_UI8 || rightvt == VT_INT ||
+ rightvt == VT_UINT || rightvt == VT_EMPTY ||
+ rightvt == VT_NULL || rightvt == VT_DECIMAL ||
+ rightvt == VT_DATE))
resultvt = VT_BSTR;
- }
+ else if (rightvt == VT_ERROR && leftvt < VT_VOID)
+ expected_error_num = DISP_E_TYPEMISMATCH;
+ else if (leftvt == VT_ERROR && (rightvt == VT_DATE ||
+ rightvt == VT_ERROR || rightvt == VT_DECIMAL))
+ expected_error_num = DISP_E_TYPEMISMATCH;
+ else if (rightvt == VT_DATE || rightvt == VT_ERROR ||
+ rightvt == VT_DECIMAL)
+ expected_error_num = DISP_E_BADVARTYPE;
+ else if (leftvt == VT_ERROR || rightvt == VT_ERROR)
+ expected_error_num = DISP_E_TYPEMISMATCH;
+ else if (leftvt == VT_VARIANT)
+ expected_error_num = DISP_E_TYPEMISMATCH;
+ else if (rightvt == VT_VARIANT && (leftvt == VT_EMPTY ||
+ leftvt == VT_NULL || leftvt == VT_I2 ||
+ leftvt == VT_I4 || leftvt == VT_R4 ||
+ leftvt == VT_R8 || leftvt == VT_CY ||
+ leftvt == VT_DATE || leftvt == VT_BSTR ||
+ leftvt == VT_BOOL || leftvt == VT_DECIMAL ||
+ leftvt == VT_I1 || leftvt == VT_UI1 ||
+ leftvt == VT_UI2 || leftvt == VT_UI4 ||
+ leftvt == VT_I8 || leftvt == VT_UI8 ||
+ leftvt == VT_INT || leftvt == VT_UINT
+ ))
+ expected_error_num = DISP_E_TYPEMISMATCH;
else
- resultvt = VT_EMPTY;
-
- if (leftvt == VT_ERROR || rightvt == VT_ERROR)
- {
- bFail = TRUE;
- }
+ expected_error_num = DISP_E_BADVARTYPE;
V_VT(&left) = leftvt;
V_VT(&right) = rightvt;
@@ -5192,57 +5211,16 @@ static void test_VarCat(void)
VarDecFromR8(0.0, &V_DECIMAL(&right));
hres = VarCat(&left, &right, &result);
- if (bFail) {
- /* Determine the error code for the vt combination */
- HRESULT expected_error_num = S_OK;
- if (rightvt == VT_ERROR && leftvt <= VT_VOID)
- expected_error_num = DISP_E_TYPEMISMATCH;
- else if (leftvt == VT_ERROR && (rightvt == VT_DATE ||
- rightvt == VT_ERROR || rightvt == VT_DECIMAL))
- expected_error_num = DISP_E_TYPEMISMATCH;
- else if (rightvt == VT_DATE || rightvt == VT_ERROR ||
- rightvt == VT_DECIMAL)
- expected_error_num = DISP_E_BADVARTYPE;
- else if (leftvt == VT_ERROR || rightvt == VT_ERROR)
- expected_error_num = DISP_E_TYPEMISMATCH;
-
- ok(hres == DISP_E_BADVARTYPE || hres == E_INVALIDARG ||
- hres == DISP_E_TYPEMISMATCH,
- "VarCat: %d, %d: Expected failure 0x%lX, got 0x%lX\n",
- leftvt, rightvt, expected_error_num, hres);
- }
- else
- {
- ok(V_VT(&result) == resultvt,
- "VarCat: %d, %d: expected vt %d, got vt %d\n",
- leftvt, rightvt, resultvt, V_VT(&result));
- if (hres != S_OK)
- {
- HRESULT expected_error_num;
- if (leftvt == VT_VARIANT && rightvt == VT_ERROR)
- expected_error_num = DISP_E_BADVARTYPE;
- else if (leftvt == VT_VARIANT)
- expected_error_num = DISP_E_TYPEMISMATCH;
- else if (rightvt == VT_VARIANT && (leftvt == VT_EMPTY ||
- leftvt == VT_NULL || leftvt == VT_I2 ||
- leftvt == VT_I4 || leftvt == VT_R4 ||
- leftvt == VT_R8 || leftvt == VT_CY ||
- leftvt == VT_DATE || leftvt == VT_BSTR ||
- leftvt == VT_BOOL || leftvt == VT_DECIMAL ||
- leftvt == VT_I1 || leftvt == VT_UI1 ||
- leftvt == VT_UI2 || leftvt == VT_UI4 ||
- leftvt == VT_I8 || leftvt == VT_UI8 ||
- leftvt == VT_INT || leftvt == VT_UINT
- ))
- expected_error_num = DISP_E_TYPEMISMATCH;
- else
- expected_error_num = DISP_E_BADVARTYPE;
- ok(hres == expected_error_num,
- "VarCat: %d, %d: Expected failure 0x%lX, got 0x%lX\n",
- leftvt, rightvt, expected_error_num, hres);
- }
- }
+ /* Determine the error code for the vt combination */
+ ok(hres == expected_error_num,
+ "VarCat: %d, %d returned error, 0x%lX expected 0x%lX.\n",
+ leftvt, rightvt, expected_error_num, hres);
+
+ /* Check types are correct */
+ ok(V_VT(&result) == resultvt,
+ "VarCat: %d, %d: expected vt %d, got vt %d\n",
+ leftvt, rightvt, resultvt, V_VT(&result));
VariantClear(&left);
VariantClear(&right);
diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c
index 3080e53..dc1c26a 100644
--- a/dlls/oleaut32/variant.c
+++ b/dlls/oleaut32/variant.c
@@ -2444,52 +2444,87 @@ VarNumFromParseNum_DecOverflow:
*/
HRESULT WINAPI VarCat(LPVARIANT left, LPVARIANT right, LPVARIANT out)
{
- VARTYPE leftvt,rightvt;
+ VARTYPE leftvt,rightvt,resultvt;
HRESULT hres;
static const WCHAR str_true[] = {'T','r','u','e','\0'};
static const WCHAR str_false[] = {'F','a','l','s','e','\0'};
+ static const WCHAR sz_empty[] = {'\0'};
leftvt = V_VT(left);
rightvt = V_VT(right);
TRACE("(%p->(%s%s),%p->(%s%s),%p)\n", left, debugstr_VT(left),
debugstr_VF(left), right, debugstr_VT(right), debugstr_VF(right), out);
- /* Null and Null simply return Null */
+ /* when both left and right are NULL the result is NULL */
if (leftvt == VT_NULL && rightvt == VT_NULL)
{
V_VT(out) = VT_NULL;
return S_OK;
}
- /* VT_ERROR with any other value should return VT_NULL */
- else if (V_VT(left) == VT_ERROR || V_VT(right) == VT_ERROR)
+ hres = S_OK;
+ resultvt = VT_EMPTY;
+
+ /* There are many special case for errors and return types */
+ if (leftvt == VT_VARIANT && (rightvt == VT_ERROR ||
+ rightvt == VT_DATE || rightvt == VT_DECIMAL))
+ hres = DISP_E_TYPEMISMATCH;
+ else if ((leftvt == VT_I2 || leftvt == VT_I4 ||
+ leftvt == VT_R4 || leftvt == VT_R8 ||
+ leftvt == VT_CY || leftvt == VT_BOOL ||
+ leftvt == VT_BSTR || leftvt == VT_I1 ||
+ leftvt == VT_UI1 || leftvt == VT_UI2 ||
+ leftvt == VT_UI4 || leftvt == VT_I8 ||
+ leftvt == VT_UI8 || leftvt == VT_INT ||
+ leftvt == VT_UINT || leftvt == VT_EMPTY ||
+ leftvt == VT_NULL || leftvt == VT_DECIMAL ||
+ leftvt == VT_DATE)
+ &&
+ (rightvt == VT_I2 || rightvt == VT_I4 ||
+ rightvt == VT_R4 || rightvt == VT_R8 ||
+ rightvt == VT_CY || rightvt == VT_BOOL ||
+ rightvt == VT_BSTR || rightvt == VT_I1 ||
+ rightvt == VT_UI1 || rightvt == VT_UI2 ||
+ rightvt == VT_UI4 || rightvt == VT_I8 ||
+ rightvt == VT_UI8 || rightvt == VT_INT ||
+ rightvt == VT_UINT || rightvt == VT_EMPTY ||
+ rightvt == VT_NULL || rightvt == VT_DECIMAL ||
+ rightvt == VT_DATE))
+ resultvt = VT_BSTR;
+ else if (rightvt == VT_ERROR && leftvt < VT_VOID)
+ hres = DISP_E_TYPEMISMATCH;
+ else if (leftvt == VT_ERROR && (rightvt == VT_DATE ||
+ rightvt == VT_ERROR || rightvt == VT_DECIMAL))
+ hres = DISP_E_TYPEMISMATCH;
+ else if (rightvt == VT_DATE || rightvt == VT_ERROR ||
+ rightvt == VT_DECIMAL)
+ hres = DISP_E_BADVARTYPE;
+ else if (leftvt == VT_ERROR || rightvt == VT_ERROR)
+ hres = DISP_E_TYPEMISMATCH;
+ else if (leftvt == VT_VARIANT)
+ hres = DISP_E_TYPEMISMATCH;
+ else if (rightvt == VT_VARIANT && (leftvt == VT_EMPTY ||
+ leftvt == VT_NULL || leftvt == VT_I2 ||
+ leftvt == VT_I4 || leftvt == VT_R4 ||
+ leftvt == VT_R8 || leftvt == VT_CY ||
+ leftvt == VT_DATE || leftvt == VT_BSTR ||
+ leftvt == VT_BOOL || leftvt == VT_DECIMAL ||
+ leftvt == VT_I1 || leftvt == VT_UI1 ||
+ leftvt == VT_UI2 || leftvt == VT_UI4 ||
+ leftvt == VT_I8 || leftvt == VT_UI8 ||
+ leftvt == VT_INT || leftvt == VT_UINT))
+ hres = DISP_E_TYPEMISMATCH;
+ else
+ hres = DISP_E_BADVARTYPE;
+
+ /* if resutl type is not S_OK, then no need to go further */
+ if (hres != S_OK)
{
- V_VT(out) = VT_EMPTY;
- return DISP_E_BADVARTYPE;
+ V_VT(out) = resultvt;
+ return hres;
}
-
- /* Concat all type that match conformance test */
- if ((leftvt == VT_I2 || leftvt == VT_I4 ||
- leftvt == VT_R4 || leftvt == VT_R8 ||
- leftvt == VT_CY || leftvt == VT_BOOL ||
- leftvt == VT_BSTR || leftvt == VT_I1 ||
- leftvt == VT_UI1 || leftvt == VT_UI2 ||
- leftvt == VT_UI4 || leftvt == VT_I8 ||
- leftvt == VT_UI8 || leftvt == VT_INT ||
- leftvt == VT_UINT || leftvt == VT_EMPTY ||
- leftvt == VT_NULL || leftvt == VT_DATE ||
- leftvt == VT_DECIMAL)
- &&
- (rightvt == VT_I2 || rightvt == VT_I4 ||
- rightvt == VT_R4 || rightvt == VT_R8 ||
- rightvt == VT_CY || rightvt == VT_BOOL ||
- rightvt == VT_BSTR || rightvt == VT_I1 ||
- rightvt == VT_UI1 || rightvt == VT_UI2 ||
- rightvt == VT_UI4 || rightvt == VT_I8 ||
- rightvt == VT_UI8 || rightvt == VT_INT ||
- rightvt == VT_UINT || rightvt == VT_EMPTY ||
- rightvt == VT_NULL || rightvt == VT_DATE ||
- rightvt == VT_DECIMAL))
+ /* Else proceed with formatting inputs to strings */
+ else
{
VARIANT bstrvar_left, bstrvar_right;
V_VT(out) = VT_BSTR;
@@ -2507,6 +2542,12 @@ HRESULT WINAPI VarCat(LPVARIANT left, LP
else
V_BSTR(&bstrvar_left) = SysAllocString(str_false);
}
+ /* Fill with empty string for later concat with right side */
+ else if (leftvt == VT_NULL)
+ {
+ V_VT(&bstrvar_left) = VT_BSTR;
+ V_BSTR(&bstrvar_left) = SysAllocString(sz_empty);
+ }
else
{
hres = VariantChangeTypeEx(&bstrvar_left,left,0,0,VT_BSTR);
@@ -2523,8 +2564,8 @@ HRESULT WINAPI VarCat(LPVARIANT left, LP
rightvt == VT_UI2 || rightvt == VT_UI4 ||
rightvt == VT_I8 || rightvt == VT_UI8 ||
rightvt == VT_INT || rightvt == VT_UINT))
- return DISP_E_BADVARTYPE;
- return hres;
+ return DISP_E_TYPEMISMATCH;
+ return DISP_E_TYPEMISMATCH;
}
}
}
@@ -2542,6 +2583,12 @@ HRESULT WINAPI VarCat(LPVARIANT left, LP
else
V_BSTR(&bstrvar_right) = SysAllocString(str_false);
}
+ /* Fill with empty string for later concat with right side */
+ else if (rightvt == VT_NULL)
+ {
+ V_VT(&bstrvar_right) = VT_BSTR;
+ V_BSTR(&bstrvar_right) = SysAllocString(sz_empty);
+ }
else
{
hres = VariantChangeTypeEx(&bstrvar_right,right,0,0,VT_BSTR);
@@ -2558,8 +2605,8 @@ HRESULT WINAPI VarCat(LPVARIANT left, LP
leftvt == VT_UI2 || leftvt == VT_UI4 ||
leftvt == VT_I8 || leftvt == VT_UI8 ||
leftvt == VT_INT || leftvt == VT_UINT))
- return DISP_E_BADVARTYPE;
- return hres;
+ return DISP_E_TYPEMISMATCH;
+ return DISP_E_TYPEMISMATCH;
}
}
}
@@ -2578,11 +2625,9 @@ HRESULT WINAPI VarCat(LPVARIANT left, LP
VariantClear(&bstrvar_right);
return S_OK;
}
-
- V_VT(out) = VT_EMPTY;
- return S_OK;
}
+
/* Wrapper around VariantChangeTypeEx() which permits changing a
variant with VT_RESERVED flag set. Needed by VarCmp. */
static HRESULT _VarChangeTypeExWrap (VARIANTARG* pvargDest,
--
1.4.0
More information about the wine-devel
mailing list