Hi Alistair,
On 17.04.2017 05:43, Alistair Leslie-Hughes wrote:
> diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c
> index 17f753af70..6f695c3f37 100644
> --- a/dlls/oleaut32/variant.c
> +++ b/dlls/oleaut32/variant.c
> @@ -2611,6 +2611,13 @@ HRESULT WINAPI VarCat(LPVARIANT left, LPVARIANT right, LPVARIANT out)
> V_VT(&bstrvar_left) = VT_BSTR;
> V_BSTR(&bstrvar_left) = SysAllocString(sz_empty);
> }
> + else if (rightvt == VT_DISPATCH)
> + {
> + /* The returned Dispatch Value, may not be able to be coerced, ie. VT_NULL */
> + hres = VariantChangeTypeEx(&bstrvar_left,left,0,0,VT_BSTR);
> + if (hres != S_OK && hres != DISP_E_TYPEMISMATCH)
> + return hres;
> + }
> else
> {
> hres = VariantChangeTypeEx(&bstrvar_left,left,0,0,VT_BSTR);
> @@ -2651,6 +2658,13 @@ HRESULT WINAPI VarCat(LPVARIANT left, LPVARIANT right, LPVARIANT out)
> V_VT(&bstrvar_right) = VT_BSTR;
> V_BSTR(&bstrvar_right) = SysAllocString(sz_empty);
> }
> + else if (rightvt == VT_DISPATCH)
> + {
> + /* The returned Dispatch Value, may not be able to be coerced, ie. VT_NULL */
> + hres = VariantChangeTypeEx(&bstrvar_right,right,0,0,VT_BSTR);
> + if (hres != S_OK && hres != DISP_E_TYPEMISMATCH)
> + return hres;
> + }
> else
> {
> hres = VariantChangeTypeEx(&bstrvar_right,right,0,0,VT_BSTR);
While this change may do the right thing in the case you're fixing, the
whole surrounding code looks just wrong and I don't like the idea of
making it even more complicated to work around problem in this single
case. For example take a look at if() statement in line 2620. It's never
executed, because it requires leftvt to be VT_NULL, which is handled
elsewhere (line 2609). If you remove that if(), this call is almost
identical to what you add here as a special case. The only difference is
DISP_E_TYPEMISMATCH handling. Did you investigate what happens in such
case? Maybe we should handle VariantChangeTypeEx failures like you do in
the patch?
Thanks,
Jacek