[PATCH] oleaut32: Improve support for IDispatch in VarCat

[Nikolay Sivov]

On 17.04.2017 6:43, Alistair Leslie-Hughes wrote:
> +            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);

This is suspiciously similar to default case, in last else block. Could
it mean VariantChangeTypeEx() should be fixed instead?

> +    if (This->bFailInvoke)
> +        return E_OUTOFMEMORY;

I think it's better to use some non-specific error code that triggers
FAILED() but is not generic as E_OUTOFMEMORY, anything above 0x8000000 I
suppose. Also maybe it's better to use exact code instead of
bFailInvoke, and return if (FAILED(This->hr))?

> +static inline DummyDispatch *impl_from_IDispatch(IDispatch *iface)
> +{
> +    return CONTAINING_RECORD(iface, DummyDispatch, IDispatch_iface);
> +}
> +
> +static ULONG WINAPI DummyDispatch_AddRef(IDispatch *iface)
> +{
> +    DummyDispatch *This = impl_from_IDispatch(iface);
> +    return InterlockedIncrement(&This->ref);
> +}

If static object is enough, you can remove reference counter.

> +    if (pVarCmp)
> +        ok(pVarCmp(&result,&expected,lcid,0) == VARCMP_EQ,
> +           "VarCat: EMPTY concat with EMPTY did not return empty VT_BSTR\n");

This is not a problem this patch introduces, but there's no reason to
continue to use pVarCmp, as it's always available on platforms we're
testing on.