automtn.cpp code

[John Roberts]

In wxAutomationObject::Invoke(...) excep is passed to
195 hr = ((IDispatch*)m_dispatchPtr)->Invoke(dispIds[0], IID_NULL,
LOCALE_SYSTEM_DEFAULT, (WORD)action, &dispparams,
vReturnPtr, &excep, &uiArgErr);

189 excep.pfnDeferredFillIn is set to NULL which means that excep will
not be changed or filled in the call. excep.bstrSource etc are
uninitialized. If hr fails and returns a bad parameter etc error then
excep members are freed as per below resulting in a segfault.

if (FAILED(hr))
{
218 SysFreeString(excep.bstrSource);
SysFreeString(excep.bstrDescription);
SysFreeString(excep.bstrHelpFile);
}

we need
189 excep.pfnDeferredFillIn = NULL;
// initialize the following because they will be freed later if Invoke
(...) fails
excep.bstrSource = NULL;
excep.bstrDescription = NULL;
excep.bstrHelpFile = NULL;

Should I submit a patch for this sort of thing.
Thanks, John

[Vadim Zeitlin]

On Thu, 17 Dec 2009 01:54:51 -0800 (PST) John Roberts <jo...@iinet.net.au> wrote:

JR> In wxAutomationObject::Invoke(...) excep is passed to
JR> 195 hr = ((IDispatch*)m_dispatchPtr)->Invoke(dispIds[0], IID_NULL,
JR> LOCALE_SYSTEM_DEFAULT, (WORD)action, &dispparams,
JR> vReturnPtr, &excep, &uiArgErr);
JR>
JR> 189 excep.pfnDeferredFillIn is set to NULL which means that excep will
JR> not be changed or filled in the call.

Are you sure about this? I didn't test it but MSDN documentation seems to
say something different, namely that if it is NULL the bstr fields should
be filled immediately rather than deferred until later. This also seems to
make more sense considering the name of the field..

JR> excep.bstrSource etc are
JR> uninitialized. If hr fails and returns a bad parameter etc error then
JR> excep members are freed as per below resulting in a segfault.
JR>
JR> if (FAILED(hr))
JR> {
JR> 218 SysFreeString(excep.bstrSource);
JR> SysFreeString(excep.bstrDescription);
JR> SysFreeString(excep.bstrHelpFile);
JR> }
JR>
JR> we need
JR> 189 excep.pfnDeferredFillIn = NULL;
JR> // initialize the following because they will be freed later if Invoke
JR> (...) fails
JR> excep.bstrSource = NULL;
JR> excep.bstrDescription = NULL;
JR> excep.bstrHelpFile = NULL;
JR>
JR> Should I submit a patch for this sort of thing.

I fixed this blindly in r62908 but patches are still preferred as, for
one, they ensure the message doesn't get lost/forgotten and they also
ensure that we apply exactly the change which was tested by you.

Thanks,
VZ

--
TT-Solutions: wxWidgets consultancy and technical support
http://www.tt-solutions.com/

[John Roberts]

On Thu, 17 Dec 2009 01:54:51 -0800 (PST) John Roberts <jo...@iinet.net.au>
wrote:

JR> In wxAutomationObject::Invoke(...) excep is passed to
JR> 195 hr = ((IDispatch*)m_dispatchPtr)->Invoke(dispIds[0], IID_NULL,
JR> LOCALE_SYSTEM_DEFAULT, (WORD)action, &dispparams,
JR> vReturnPtr, &excep, &uiArgErr);
JR>
JR> 189 excep.pfnDeferredFillIn is set to NULL which means that excep will
JR> not be changed or filled in the call.

VZ "Are you sure about this? I didn't test it but MSDN documentation seems

to
say something different, namely that if it is NULL the bstr fields should
be filled immediately rather than deferred until later. This also seems to
make more sense considering the name of the field.."

No I am not sure. I have had a brief look at the following.

How to: Map HRESULTs and Exceptions
COM methods report errors by returning HRESULTs; .NET methods report them by
throwing exceptions. The runtime handles the transition between the two.
Each exception class in the .NET Framework maps to an HRESULT.

So I am not sure it is appropriate in a non .net environment.

There were several occasions where there was nothing in excep members when
hr != S_OK.
Docs suggest
pExcepInfo
[out] Pointer to a structure that contains exception information. This
structure should be filled in if DISP_E_EXCEPTION is returned. Can be NULL.

Seems the test should not be
if (FAILED(hr))
but test for
DISP_E_EXCEPTION

JR> excep.bstrSource etc are
JR> uninitialized. If hr fails and returns a bad parameter etc error then
JR> excep members are freed as per below resulting in a segfault.
JR>
JR> if (FAILED(hr))
JR> {
JR> 218 SysFreeString(excep.bstrSource);
JR> SysFreeString(excep.bstrDescription);
JR> SysFreeString(excep.bstrHelpFile);
JR> }
JR>
JR> we need
JR> 189 excep.pfnDeferredFillIn = NULL;
JR> // initialize the following because they will be freed later if Invoke
JR> (...) fails
JR> excep.bstrSource = NULL;
JR> excep.bstrDescription = NULL;
JR> excep.bstrHelpFile = NULL;
JR>
JR> Should I submit a patch for this sort of thing.

VZ "I fixed this blindly in r62908 but patches are still preferred as, for

one, they ensure the message doesn't get lost/forgotten and they also
ensure that we apply exactly the change which was tested by you."

Thanks for that, JR