Hi Nikolay,
On 11/21/11 08:24, Nikolay Sivov wrote:
Added common ISupportErrorInfo implementation
+/* common ISupportErrorInfo implementation */ +typedef struct { + ISupportErrorInfo ISupportErrorInfo_iface; + LONG ref; + + const tid_t* iids; +} SupportErrorInfo; + +static inline SupportErrorInfo *impl_from_ISupportErrorInfo(ISupportErrorInfo *iface) +{ + return CONTAINING_RECORD(iface, SupportErrorInfo, ISupportErrorInfo_iface); +} + +static HRESULT WINAPI SupportErrorInfo_QueryInterface(ISupportErrorInfo *iface, REFIID riid, void **obj) +{ + SupportErrorInfo *This = impl_from_ISupportErrorInfo(iface); + TRACE("(%p)->(%s %p)\n", This, debugstr_guid(riid), obj); + + *obj = NULL; + + if (IsEqualGUID(riid,&IID_IUnknown) || IsEqualGUID(riid,&IID_ISupportErrorInfo)) { + *obj = iface; + ISupportErrorInfo_AddRef(iface); + return S_OK; + } + + return E_NOINTERFACE; +}
This doesn't seem to be the right approach. You need to return here interfaces of an object that inherits your ISupportErrorInfo implementation - that's the COM rule. Also you probably don't want to create new SupportErrorInfo instance on each QI call. I'd suggest DispatchEx-like approach in this case (or even abuse DispatchEx object and add ISupportErrorInfo there).
Jacek
On 11/21/2011 16:09, Jacek Caban wrote:
Hi Nikolay,
On 11/21/11 08:24, Nikolay Sivov wrote:
Added common ISupportErrorInfo implementation
+/* common ISupportErrorInfo implementation */ +typedef struct {
- ISupportErrorInfo ISupportErrorInfo_iface;
- LONG ref;
- const tid_t* iids;
+} SupportErrorInfo;
+static inline SupportErrorInfo *impl_from_ISupportErrorInfo(ISupportErrorInfo *iface) +{
- return CONTAINING_RECORD(iface, SupportErrorInfo,
ISupportErrorInfo_iface); +}
+static HRESULT WINAPI SupportErrorInfo_QueryInterface(ISupportErrorInfo *iface, REFIID riid, void **obj) +{
- SupportErrorInfo *This = impl_from_ISupportErrorInfo(iface);
- TRACE("(%p)->(%s %p)\n", This, debugstr_guid(riid), obj);
- *obj = NULL;
- if (IsEqualGUID(riid,&IID_IUnknown) ||
IsEqualGUID(riid,&IID_ISupportErrorInfo)) {
*obj = iface;ISupportErrorInfo_AddRef(iface);return S_OK;- }
- return E_NOINTERFACE;
+}
This doesn't seem to be the right approach. You need to return here interfaces of an object that inherits your ISupportErrorInfo implementation - that's the COM rule.
I know but that's not how native works. ISupportErrorInfo reference counting is completely separated from object your queried it from.
Also you probably don't want to create new SupportErrorInfo instance on each QI call.
I do. Tests shows that two consecutive QI calls return different pointers.
I'd suggest DispatchEx-like approach in this case (or even abuse DispatchEx object and add ISupportErrorInfo there).
Jacek
On 11/21/11 15:16, Nikolay Sivov wrote:
On 11/21/2011 16:09, Jacek Caban wrote:
Hi Nikolay,
On 11/21/11 08:24, Nikolay Sivov wrote:
Added common ISupportErrorInfo implementation
+/* common ISupportErrorInfo implementation */ +typedef struct {
- ISupportErrorInfo ISupportErrorInfo_iface;
- LONG ref;
- const tid_t* iids;
+} SupportErrorInfo;
+static inline SupportErrorInfo *impl_from_ISupportErrorInfo(ISupportErrorInfo *iface) +{
- return CONTAINING_RECORD(iface, SupportErrorInfo,
ISupportErrorInfo_iface); +}
+static HRESULT WINAPI SupportErrorInfo_QueryInterface(ISupportErrorInfo *iface, REFIID riid, void **obj) +{
- SupportErrorInfo *This = impl_from_ISupportErrorInfo(iface);
- TRACE("(%p)->(%s %p)\n", This, debugstr_guid(riid), obj);
- *obj = NULL;
- if (IsEqualGUID(riid,&IID_IUnknown) ||
IsEqualGUID(riid,&IID_ISupportErrorInfo)) {
*obj = iface;ISupportErrorInfo_AddRef(iface);return S_OK;- }
- return E_NOINTERFACE;
+}
This doesn't seem to be the right approach. You need to return here interfaces of an object that inherits your ISupportErrorInfo implementation - that's the COM rule.
I know but that's not how native works. ISupportErrorInfo reference counting is completely separated from object your queried it from.
Still, QI doesn't look right (unless you also have tests for this). See something like this:
IDOMXMLNode_QueryInterface(node, &IID_ISupportErrorInfo, (void**)&sei); ISupportErrorInfo_QueryInterface(sei, &IID_IDOMXMLNode, (void**)&node2);
The second QueryInterface call should succeeded, but it won't with your implementation.
Also you probably don't want to create new SupportErrorInfo instance on each QI call.
I do. Tests shows that two consecutive QI calls return different pointers.
That's fine then.
Jacek
On 11/21/2011 16:29, Jacek Caban wrote:
On 11/21/11 15:16, Nikolay Sivov wrote:
On 11/21/2011 16:09, Jacek Caban wrote:
Hi Nikolay,
On 11/21/11 08:24, Nikolay Sivov wrote:
Added common ISupportErrorInfo implementation
+/* common ISupportErrorInfo implementation */ +typedef struct {
- ISupportErrorInfo ISupportErrorInfo_iface;
- LONG ref;
- const tid_t* iids;
+} SupportErrorInfo;
+static inline SupportErrorInfo *impl_from_ISupportErrorInfo(ISupportErrorInfo *iface) +{
- return CONTAINING_RECORD(iface, SupportErrorInfo,
ISupportErrorInfo_iface); +}
+static HRESULT WINAPI SupportErrorInfo_QueryInterface(ISupportErrorInfo *iface, REFIID riid, void **obj) +{
- SupportErrorInfo *This = impl_from_ISupportErrorInfo(iface);
- TRACE("(%p)->(%s %p)\n", This, debugstr_guid(riid), obj);
- *obj = NULL;
- if (IsEqualGUID(riid,&IID_IUnknown) ||
IsEqualGUID(riid,&IID_ISupportErrorInfo)) {
*obj = iface;ISupportErrorInfo_AddRef(iface);return S_OK;- }
- return E_NOINTERFACE;
+}
This doesn't seem to be the right approach. You need to return here interfaces of an object that inherits your ISupportErrorInfo implementation - that's the COM rule.
I know but that's not how native works. ISupportErrorInfo reference counting is completely separated from object your queried it from.
Still, QI doesn't look right (unless you also have tests for this). See something like this:
IDOMXMLNode_QueryInterface(node, &IID_ISupportErrorInfo, (void**)&sei); ISupportErrorInfo_QueryInterface(sei, &IID_IDOMXMLNode, (void**)&node2);
The second QueryInterface call should succeeded, but it won't with your implementation.
Yeah, I thought about that when I added this implementation, and maybe it should work this way but it doesn't on native :) I have a test for that that I'll submit as a separate patch. This interface looks more like meta thing that doesn't belong to any DOM object at all.
On 11/21/11 15:33, Nikolay Sivov wrote:
On 11/21/2011 16:29, Jacek Caban wrote:
On 11/21/11 15:16, Nikolay Sivov wrote:
On 11/21/2011 16:09, Jacek Caban wrote:
This doesn't seem to be the right approach. You need to return here interfaces of an object that inherits your ISupportErrorInfo implementation - that's the COM rule.
I know but that's not how native works. ISupportErrorInfo reference counting is completely separated from object your queried it from.
Still, QI doesn't look right (unless you also have tests for this). See something like this:
IDOMXMLNode_QueryInterface(node, &IID_ISupportErrorInfo, (void**)&sei); ISupportErrorInfo_QueryInterface(sei, &IID_IDOMXMLNode, (void**)&node2);
The second QueryInterface call should succeeded, but it won't with your implementation.
Yeah, I thought about that when I added this implementation, and maybe it should work this way but it doesn't on native :) I have a test for that that I'll submit as a separate patch. This interface looks more like meta thing that doesn't belong to any DOM object at all.
OK, the patch is fine then. Thanks for checking it.
Jacek
On 11/21/2011 16:39, Jacek Caban wrote:
On 11/21/11 15:33, Nikolay Sivov wrote:
On 11/21/2011 16:29, Jacek Caban wrote:
On 11/21/11 15:16, Nikolay Sivov wrote:
On 11/21/2011 16:09, Jacek Caban wrote:
This doesn't seem to be the right approach. You need to return here interfaces of an object that inherits your ISupportErrorInfo implementation - that's the COM rule.
I know but that's not how native works. ISupportErrorInfo reference counting is completely separated from object your queried it from.
Still, QI doesn't look right (unless you also have tests for this). See something like this:
IDOMXMLNode_QueryInterface(node, &IID_ISupportErrorInfo, (void**)&sei); ISupportErrorInfo_QueryInterface(sei, &IID_IDOMXMLNode, (void**)&node2);
The second QueryInterface call should succeeded, but it won't with your implementation.
Yeah, I thought about that when I added this implementation, and maybe it should work this way but it doesn't on native :) I have a test for that that I'll submit as a separate patch. This interface looks more like meta thing that doesn't belong to any DOM object at all.
OK, the patch is fine then. Thanks for checking it.
Thanks for a review.
Jacek
-
Jacek Caban -
Nikolay Sivov