Re: [3/4] msxml3: Add IDispatchEx support for IXMLDOMNamedNodeMap

2011-11-04 Thread Jacek Caban

Hi Nikolay,

On 11/04/11 16:46, Nikolay Sivov wrote:

+if(This-data-vtbl  This-data-vtbl-invoke) {
+hres = This-data-vtbl-invoke(This-outer, id, lcid, wFlags, pdp, 
pvarRes, pei);
+if (hres != E_NOTIMPL) return hres;
+}


Using E_NOTIMPL for special meaning here seems wrong to me. 
DISP_E_UNKNOWNNAME could be a better choice. However, if you'd move 
custom invoke implementation to be the last choice, after dynamic and 
typelib-based values, then you could simply return whatever invoke returns.


Jacek




Re: [3/4] msxml3: Add IDispatchEx support for IXMLDOMNamedNodeMap

2011-11-04 Thread Nikolay Sivov

On 11/4/2011 20:00, Jacek Caban wrote:

Hi Nikolay,

On 11/04/11 16:46, Nikolay Sivov wrote:

+if(This-data-vtbl  This-data-vtbl-invoke) {
+hres = This-data-vtbl-invoke(This-outer, id, lcid, 
wFlags, pdp, pvarRes, pei);

+if (hres != E_NOTIMPL) return hres;
+}


Using E_NOTIMPL for special meaning here seems wrong to me. 
DISP_E_UNKNOWNNAME could be a better choice. However, if you'd move 
custom invoke implementation to be the last choice, after dynamic and 
typelib-based values, then you could simply return whatever invoke 
returns.
I was thinking about additional call in vtable to use instead of 
is_custom to check if dispid is supposed to be handled with custom 
invoke. What do you think about it?


Jacek






Re: [3/4] msxml3: Add IDispatchEx support for IXMLDOMNamedNodeMap

2011-11-04 Thread Jacek Caban

On 11/04/11 17:11, Nikolay Sivov wrote:

On 11/4/2011 20:00, Jacek Caban wrote:

Hi Nikolay,

On 11/04/11 16:46, Nikolay Sivov wrote:

+if(This-data-vtbl  This-data-vtbl-invoke) {
+hres = This-data-vtbl-invoke(This-outer, id, lcid, 
wFlags, pdp, pvarRes, pei);

+if (hres != E_NOTIMPL) return hres;
+}


Using E_NOTIMPL for special meaning here seems wrong to me. 
DISP_E_UNKNOWNNAME could be a better choice. However, if you'd move 
custom invoke implementation to be the last choice, after dynamic and 
typelib-based values, then you could simply return whatever invoke 
returns.
I was thinking about additional call in vtable to use instead of 
is_custom to check if dispid is supposed to be handled with custom 
invoke. What do you think about it?


It really depends on how it's used in msxml3. The problem with this 
solution is that it means more code in every object using custom dipids. 
If we can avoid it, even at cost of more complicated code in dispex.c, 
that's probably better in terms of code maintaining IMO.


Jacek




Re: [3/4] msxml3: Add IDispatchEx support for IXMLDOMNamedNodeMap

2011-11-04 Thread Nikolay Sivov

On 11/4/2011 20:21, Jacek Caban wrote:

On 11/04/11 17:11, Nikolay Sivov wrote:

On 11/4/2011 20:00, Jacek Caban wrote:

Hi Nikolay,

On 11/04/11 16:46, Nikolay Sivov wrote:

+if(This-data-vtbl  This-data-vtbl-invoke) {
+hres = This-data-vtbl-invoke(This-outer, id, lcid, 
wFlags, pdp, pvarRes, pei);

+if (hres != E_NOTIMPL) return hres;
+}


Using E_NOTIMPL for special meaning here seems wrong to me. 
DISP_E_UNKNOWNNAME could be a better choice. However, if you'd move 
custom invoke implementation to be the last choice, after dynamic 
and typelib-based values, then you could simply return whatever 
invoke returns.
I was thinking about additional call in vtable to use instead of 
is_custom to check if dispid is supposed to be handled with custom 
invoke. What do you think about it?


It really depends on how it's used in msxml3.
The only case I'm aware of so far is index based access of collection 
elements, that's all. So I'll better go with DISP_E_UNKNOWNNAME for now 
cause use of this thing is really limited in msxml.
The problem with this solution is that it means more code in every 
object using custom dipids. If we can avoid it, even at cost of more 
complicated code in dispex.c, that's probably better in terms of code 
maintaining IMO.

Right, extra call is too much probably.


Jacek