Hi Jacek,
Thanks for the review.
On 9/27/19 5:15 PM, Jacek Caban wrote:
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?
Yeah, they're supposed to "work", Windows seems to just ignore the other dimensions and only read the first one, without giving errors (I did add tests for it, so no problem there).
+ 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.
It counts down from 'i' (wherever it stopped at), not the entire array, so it should be safe, unless I'm missing something?
+ for (i = 0; i < dp.cArgs; i++) + { + /* The DISPPARAMS are stored in reverse order */ + VARIANT *src = (VARIANT*)((char*)(sa->pvData) + (dp.cArgs - i
- 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.
Actually, I'm not sure. :-)
I'll do some more tests with this and see what happens.