[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