Re: [1/4] msxml3: Added common ISupportErrorInfo implementation

[Jacek Caban]

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