Re: [PATCH 2/3] msscript.ocx: Implement IScriptControl::Run.

[Jacek Caban]

Hi Gabriel,

On 9/26/19 4:43 PM, Gabriel Ivăncescu wrote:
> static HRESULT WINAPI ScriptControl_Run(IScriptControl *iface, BSTR procedure_name, SAFEARRAY **parameters, VARIANT *res)
>   {
>       ScriptControl *This = impl_from_IScriptControl(iface);
> - FIXME("(%p)->(%s %p %p)\n", This, debugstr_w(procedure_name), parameters, res);
> - return E_NOTIMPL;
> - IDispatchEx *dispex;
> - IDispatch *disp;
> - SAFEARRAY *sa;
> - DISPPARAMS dp;
> - DISPID dispid;
> - HRESULT hr;
> - UINT i = 0;
> -
> - TRACE("(%p)->(%s %p %p)\n", This, debugstr_w(procedure_name), parameters, res);
> -
> - if (!parameters || !res) return E_POINTER;
> - if (!(sa = *parameters)) return E_POINTER;
> -
> - if (sa->cDims == 0) return DISP_E_BADINDEX;

Shouldn't it be '!= 1'? Are cDims > 1 expected to work?
> - if (!(sa->fFeatures & FADF_VARIANT)) return DISP_E_BADVARTYPE;
> -
> - hr = start_script(This);
> - if (FAILED(hr)) return hr;
> -
> - dp.cArgs = sa->rgsabound[0].cElements;
> - dp.rgdispidNamedArgs = NULL;
> - dp.cNamedArgs = 0;
> -
> - dp.rgvarg = heap_alloc(dp.cArgs * sizeof(*dp.rgvarg));
> - if (!dp.rgvarg) return E_OUTOFMEMORY;
> -
> - hr = SafeArrayLock(sa);
> - if (FAILED(hr)) goto err;

Error case will try to clean array elements that are not yet initialized.
> -
> - for (i = 0; i < dp.cArgs; i++)
> - {
> -
>    /* The DISPPARAMS are stored in reverse order */
> -
>    VARIANT *src = (VARIANT*)((char*)(sa->pvData) + (dp.cArgs - i - 1) * sa->cbElements);
> -
> -
>    V_VT(&dp.rgvarg[i]) = VT_EMPTY;
> -
>    hr = VariantCopy(&dp.rgvarg[i], src);

Do we really need VariantCopy here? I think that simple assignment 
should be enough in this case would would simplify the code.

Thanks,

Jacek