wxActiveX.cpp
John Rem
hret returns other than S_OK the program will still continue to run.
i.e. show the activex control but have no event connection point. It
will still show an error at CHECK_HR(hret); if FindConnectionPoint()
fails and debug level is set appropriately.
1023 HRESULT hret =
cpContainer->FindConnectionPoint(ta->guid,
cp.GetRef());
CHECK_HR(hret);
if ( SUCCEEDED(hret)) // add
{ // add
IDispatch* disp;
m_frameSite->QueryInterface(IID_IDispatch, (void**)
&disp);
hret = cp->Advise(new wxActiveXEvents(this, ta-
>guid),
&adviseCookie);
CHECK_HR(hret);
} // add
At present if it will segfault if FindConnectionPoint() fails as cp
will be NULL when using cp->Advise()
Vadim Zeitlin
JR> In wxActiveX.cpp can the following (//add) lines be added so that if
JR> hret returns other than S_OK the program will still continue to run.
JR> i.e. show the activex control but have no event connection point. It
JR> will still show an error at CHECK_HR(hret); if FindConnectionPoint()
JR> fails and debug level is set appropriately.
JR>
JR> 1023 HRESULT hret =
JR> cpContainer->FindConnectionPoint(ta->guid,
JR> cp.GetRef());
JR> CHECK_HR(hret);
JR>
JR> if ( SUCCEEDED(hret)) // add
JR> { // add
JR> IDispatch* disp;
JR> m_frameSite->QueryInterface(IID_IDispatch, (void**)
JR> &disp);
JR>
JR> hret = cp->Advise(new wxActiveXEvents(this, ta-
JR> >guid),
JR> &adviseCookie);
JR> CHECK_HR(hret);
JR> } // add
JR>
JR> At present if it will segfault if FindConnectionPoint() fails as cp
JR> will be NULL when using cp->Advise()
The question I have is whether it's normal for FindConnectionPoint() to
fail. If it is, then we shouldn't use CHECK_HR() after calling it at all
but I'm not really sure about it.
In general, error handling in this code is really bad, any improvements to
it would be very welcome.
Regards,
VZ
--
TT-Solutions: wxWidgets consultancy and technical support
http://www.tt-solutions.com/
John Roberts
but I'm not really sure about it."
IConnectionPointContainer::FindConnectionPoint Method
E_NOTIMPL is not allowed as a return value for this method. Any
implementation of IConnectionPointContainer must implement this method for
the connectable object's outgoing interfaces.
So I don't think it is normal to fail and CHECK_HR() needs to be there.
(nice if it showed whether it was E_POINTER or CONNECT_E_NOCONNECTION) but I
am trying to connect to a 3rd party .net assembly and it does fail with
CONNECT_E_NOCONNECTION suggesting the REFIID is incorrect. With the above
code at least the ActiveX control shows and my program continues. There are
no user controls on this ActiveX control. It is for display only and I will
connect (hopefully) to the code via oleautomation. I have no means of
altering the 3rd party assembly except by feedback to the developers.
Vadim Zeitlin
JR> So I don't think it is normal to fail and CHECK_HR() needs to be there.
JR> (nice if it showed whether it was E_POINTER or CONNECT_E_NOCONNECTION) but I
JR> am trying to connect to a 3rd party .net assembly and it does fail with
JR> CONNECT_E_NOCONNECTION suggesting the REFIID is incorrect. With the above
JR> code at least the ActiveX control shows and my program continues.
I've added the check but someone should still go through this code and
carefully add proper error checking to it. Unfortunately I don't have
neither time nor motivation to do it myself.
However I couldn't prevent myself from noticing the weird
IDispatch* disp;
m_frameSite->QueryInterface(IID_IDispatch, (void**)&disp);
lines in this code. I don't understand what is their purpose ("disp" is
never used below) and I'm pretty sure we've got a memory leak here because
Release() is not called even if disp != NULL. Could we just remove these
lines? As you're using this class already, could you please test if your
code still works if you do this?
Thanks again,
John Roberts
----- Original Message -----
From: "Vadim Zeitlin" <va...@wxwidgets.org>
To: <wx-u...@googlegroups.com>
Subject: Re[2]: wxActiveX.cpp
VZ "However I couldn't prevent myself from noticing the weird
IDispatch* disp;
m_frameSite->QueryInterface(IID_IDispatch, (void**)&disp);
lines in this code. I don't understand what is their purpose ("disp" is
never used below) and I'm pretty sure we've got a memory leak here because
Release() is not called even if disp != NULL. Could we just remove these
lines? As you're using this class already, could you please test if your
code still works if you do this?"
at FindConnectionPoint in ConnectionPointContainer despite passing just
about every ID I found for the control and dispinterface in ms OLE viewer.
Could be the .net's control's problem. Unfortunately the developers have not
yet been very responsive as it works when imported into VBasic and C# .net
though not for C++ even with MS VS9/10.
I will take a look at it over the next week or two as I get time. I have
preferred to avoid ActiveX stuff so I am learning as I go.
Cheers, John
John Roberts
> From: "Vadim Zeitlin" <va...@wxwidgets.org>
> To: <wx-u...@googlegroups.com>
> Sent: Saturday, December 05, 2009 10:25 PM
> Subject: Re[2]: wxActiveX.cpp
>
> VZ "However I couldn't prevent myself from noticing the weird
>
> IDispatch* disp;
> m_frameSite->QueryInterface(IID_IDispatch, (void**)&disp);
>
> lines in this code. I don't understand what is their purpose ("disp" is
> never used below) and I'm pretty sure we've got a memory leak here because
> Release() is not called even if disp != NULL. Could we just remove these
> lines? As you're using this class already, could you please test if your
> code still works if you do this?"
>
> at FindConnectionPoint in ConnectionPointContainer despite passing just
> about every ID I found for the control and dispinterface in ms OLE viewer.
> Could be the .net's control's problem. Unfortunately the developers have
> not
> yet been very responsive as it works when imported into VBasic and C# .net
> though not for C++ even with MS VS9/10.
> I will take a look at it over the next week or two as I get time. I have
> preferred to avoid ActiveX stuff so I am learning as I go.
> Cheers, John
The ActiveX control that I am working with returns CONNECT_E_NOCONNECTION
from HRESULT hret = cpContainer->FindConnectionPoint(ta->guid,
cp.GetRef()); so will not go into the line approx 1040 "if(cp)". So there is
not connection point and I cannot test whether removing those lines is a
problem. It is a query directed at the ActiveX container's m_frameSite and
will presumably err if m_frameSite does not return a IDispatch pointer for
its own interface to the ActiveX control site. Odd that the returned pointer
disp is not stored anywhere. Perhaps it is not released because m_frameSite
is still in use but that doesn't make much sense either.
There are no user accessible controls on the activex control I am
interfacing with and it just displays a bitmap and some text that may change
when the control is accessed programmatically. The displayed text is changed
from the server end. What this means is that even though it works I will
still get a CHECK_HR(hret) log message after line
1034 HRESULT hret = cpContainer->FindConnectionPoint(ta->guid, cp.GetRef());
hret can be S_OK, CONNECT_E_NOCONNECTION or E_POINTER
It would be good not to get the log message as the control works. If
CHECK_HR(hret) is removed then E_POINTER probably still needs to be checked
for.
Any thoughts?
Cheers, John
Vadim Zeitlin
JR> There are no user accessible controls on the activex control I am
JR> interfacing with and it just displays a bitmap and some text that may change
JR> when the control is accessed programmatically. The displayed text is changed
JR> from the server end. What this means is that even though it works I will
JR> still get a CHECK_HR(hret) log message after line
JR> 1034 HRESULT hret = cpContainer->FindConnectionPoint(ta->guid, cp.GetRef());
JR> hret can be S_OK, CONNECT_E_NOCONNECTION or E_POINTER
JR> It would be good not to get the log message as the control works. If
JR> CHECK_HR(hret) is removed then E_POINTER probably still needs to be checked
JR> for.
JR> Any thoughts?
Yes, I guess failing to find any connection points shouldn't be fatal --
all it means is that no events will be generated but it's not always a
problem. If you can make a patch checking for CONNECT_E_NOCONNECTION (this
is better than checking for E_POINTER: we should ignore only the errors we
know about and not everything by default) and handling it specially, I'd be
glad to apply it.
Thanks,
John Roberts
JR> There are no user accessible controls on the activex control I am
JR> interfacing with and it just displays a bitmap and some text that may
change
JR> when the control is accessed programmatically. The displayed text is
changed
JR> from the server end. What this means is that even though it works I will
JR> still get a CHECK_HR(hret) log message after line
JR> 1034 HRESULT hret = cpContainer->FindConnectionPoint(ta->guid,
cp.GetRef());
JR> hret can be S_OK, CONNECT_E_NOCONNECTION or E_POINTER
JR> It would be good not to get the log message as the control works. If
JR> CHECK_HR(hret) is removed then E_POINTER probably still needs to be
checked
JR> for.
JR> Any thoughts?
VZ "Yes, I guess failing to find any connection points shouldn't be
fatal --
all it means is that no events will be generated but it's not always a
problem. If you can make a patch checking for CONNECT_E_NOCONNECTION (this
is better than checking for E_POINTER: we should ignore only the errors we
know about and not everything by default) and handling it specially, I'd be
glad to apply it."
I will do so then after testing.
Cheers, John