Re: [PATCH 2/2] oledb32: GetConversionSize shouldn't fail for null variants.
On Tue, Dec 22, 2015 at 06:32:04PM +1100, Alistair Leslie-Hughes wrote:
diff --git a/dlls/oledb32/tests/convert.c b/dlls/oledb32/tests/convert.c index a785457..55539f9 100644 --- a/dlls/oledb32/tests/convert.c +++ b/dlls/oledb32/tests/convert.c @@ -2627,6 +2627,33 @@ static void test_getconversionsize(void) ok(hr == S_OK, "got 0x%08x\n", hr); VariantClear(&var);
+ dst_len = 78; + V_VT(&var) = VT_NULL; + hr = IDataConvert_GetConversionSize(convert, DBTYPE_VARIANT, DBTYPE_WSTR, NULL, &dst_len, &var); + ok(hr == S_OK, "got 0x%08x\n", hr);
You should test dst_len after return. Likewise below. Huw.
Hi Huw, On 22/12/15 20:03, Huw Davies wrote:
On Tue, Dec 22, 2015 at 06:32:04PM +1100, Alistair Leslie-Hughes wrote:
diff --git a/dlls/oledb32/tests/convert.c b/dlls/oledb32/tests/convert.c index a785457..55539f9 100644 --- a/dlls/oledb32/tests/convert.c +++ b/dlls/oledb32/tests/convert.c @@ -2627,6 +2627,33 @@ static void test_getconversionsize(void) ok(hr == S_OK, "got 0x%08x\n", hr); VariantClear(&var);
+ dst_len = 78; + V_VT(&var) = VT_NULL; + hr = IDataConvert_GetConversionSize(convert, DBTYPE_VARIANT, DBTYPE_WSTR, NULL, &dst_len, &var); + ok(hr == S_OK, "got 0x%08x\n", hr);
You should test dst_len after return. Likewise below.
No, there is no point. On windows it returns a magic number of 110, and that is why previous patches were rejected, since I had those checks in there. The return value doesn't matter in the case of a NULL Variant. Best Regards Alistair Leslie-Hughes
Hi Huw, On 22/12/15 20:03, Huw Davies wrote:
On Tue, Dec 22, 2015 at 06:32:04PM +1100, Alistair Leslie-Hughes wrote:
diff --git a/dlls/oledb32/tests/convert.c b/dlls/oledb32/tests/convert.c index a785457..55539f9 100644 --- a/dlls/oledb32/tests/convert.c +++ b/dlls/oledb32/tests/convert.c @@ -2627,6 +2627,33 @@ static void test_getconversionsize(void) ok(hr == S_OK, "got 0x%08x\n", hr); VariantClear(&var);
+ dst_len = 78; + V_VT(&var) = VT_NULL; + hr = IDataConvert_GetConversionSize(convert, DBTYPE_VARIANT, DBTYPE_WSTR, NULL, &dst_len, &var); + ok(hr == S_OK, "got 0x%08x\n", hr);
You should test dst_len after return. Likewise below.
No, there is no point. On windows it returns a magic number of 110, and that is why previous patches were rejected, since I had those checks in there. The return value doesn't matter in the case of a NULL Variant. Best Regards Alistair Leslie-Hughes
On 22.12.2015 12:07, Alistair Leslie-Hughes wrote:
Hi Huw,
On 22/12/15 20:03, Huw Davies wrote:
On Tue, Dec 22, 2015 at 06:32:04PM +1100, Alistair Leslie-Hughes wrote:
diff --git a/dlls/oledb32/tests/convert.c b/dlls/oledb32/tests/convert.c index a785457..55539f9 100644 --- a/dlls/oledb32/tests/convert.c +++ b/dlls/oledb32/tests/convert.c @@ -2627,6 +2627,33 @@ static void test_getconversionsize(void) ok(hr == S_OK, "got 0x%08x\n", hr); VariantClear(&var);
+ dst_len = 78; + V_VT(&var) = VT_NULL; + hr = IDataConvert_GetConversionSize(convert, DBTYPE_VARIANT, DBTYPE_WSTR, NULL, &dst_len, &var); + ok(hr == S_OK, "got 0x%08x\n", hr);
You should test dst_len after return. Likewise below.
No, there is no point. On windows it returns a magic number of 110, and that is why previous patches were rejected, since I had those checks in there. The return value doesn't matter in the case of a NULL Variant.
With previous patches my question was to test actual string this variant is converted too, maybe it's not that magic after all.
Best Regards Alistair Leslie-Hughes
On 22 Dec 2015, at 09:28, Nikolay Sivov wrote:
With previous patches my question was to test actual string this variant is converted too, maybe it's not that magic after all.
Looks like there's already a test for this (see the end of test_converttostr). It doesn't return a string, but returns status DBSTATUS_S_ISNULL. I think, given that's the case, you should add a comment above the new test, stating that Windows returns 110, but we're not testing it. Also remove the initialization of dst_len before the call, since if we're not testing it there's no need to initialize it. Huw.
Hi Huw, On 22/12/15 20:56, Huw Davies wrote:
On 22 Dec 2015, at 09:28, Nikolay Sivov wrote:
With previous patches my question was to test actual string this variant is converted too, maybe it's not that magic after all.
Looks like there's already a test for this (see the end of test_converttostr). It doesn't return a string, but returns status DBSTATUS_S_ISNULL.
I think, given that's the case, you should add a comment above the new test, stating that Windows returns 110, but we're not testing it. Also remove the initialization of dst_len before the call, since if we're not testing it there's no need to initialize it.
Sure I can live with that. Alistair
Hi Huw, On 22/12/15 20:56, Huw Davies wrote:
On 22 Dec 2015, at 09:28, Nikolay Sivov wrote:
With previous patches my question was to test actual string this variant is converted too, maybe it's not that magic after all.
Looks like there's already a test for this (see the end of test_converttostr). It doesn't return a string, but returns status DBSTATUS_S_ISNULL.
I think, given that's the case, you should add a comment above the new test, stating that Windows returns 110, but we're not testing it. Also remove the initialization of dst_len before the call, since if we're not testing it there's no need to initialize it.
Sure I can live with that. Alistair
On 22.12.2015 12:56, Huw Davies wrote:
On 22 Dec 2015, at 09:28, Nikolay Sivov wrote:
With previous patches my question was to test actual string this variant is converted too, maybe it's not that magic after all.
Looks like there's already a test for this (see the end of test_converttostr). It doesn't return a string, but returns status DBSTATUS_S_ISNULL.
Ah, ok. Missed that.
I think, given that's the case, you should add a comment above the new test, stating that Windows returns 110, but we're not testing it. Also remove the initialization of dst_len before the call, since if we're not testing it there's no need to initialize it.
Huw.
participants (4)
-
Alistair Leslie-Hughes -
Huw Davies -
Huw Davies -
Nikolay Sivov