Hello developers,
the msxml3 code currently uses aggregation to implement IXMLDOMNode on all of the different DOM tree elements. As the interfaces for these elements are all derived from IXMLDOMNode, a lot of forwarding thunks have to be implemented to get the calls into the inner object. The attached patch series modifies the source code to derive the objects instead, which seems like the more natural approach to get derived interfaces implemented.
The attached patches still have to be cleaned up: They contain lines ending with whitespace, incosistent spacing around function call parenthesis, incosistent style of using a cast helper function or direct casting and other small nits.
A more important problem with these patches that make me write this mail is that they generate lots of compiler warnings like | domdoc.c:1556: warning: initialization from incompatible pointer type which is of no surprise, as the vtable for domdoc expects a function taking an IXMLDOMDocument as "this" pointer while I provide a function taking an interface pointer to an IXMLDOMNode as "this" pointer. Both of these pointers point to the same address (as usual for derived objects), so the conversion is safe, but the simple-minded C compiler can't know that.
The question that arises is how to handle these warnings. a) ignore them (most probably not an option) b) add lots of casts (only way to make the idea of these patches fly) c) write lots of little forwarding functions that convert the this pointer. d) ditch the idea of using derivation instead of aggregation
Any oppions on that? I (obviously) think that b is the right solution, but as a quick grep over the Wine source code revealed, this solution seems to be used nowhere. Is that because this is the first place where deriving makes sense or is that because I'm completely off-track?
Kind regards, Michael Karcher
PS: The munged "From " lines in the attached patches are intentional. They keep my email program from choosing "application/mbox" as MIME type.
Hi Michael,
Michael Karcher wrote:
Any oppions on that?
I think other solution would be better. There is no reason for xmlnode object to implement any interface. All its child objects already have IXMLDOMNode interface. xmlnode could be a common struct stored by child objects and there could be set of function with common method implementation. This way we could also avoid ugly tests for node type in node.c, because we can handle the difference outside node.c.
I (obviously) think that b is the right solution, but as a quick grep over the Wine source code revealed, this solution seems to be used nowhere. Is that because this is the first place where deriving makes sense or is that because I'm completely off-track?
Casting function in vtbl is not an acceptable solution. If you have to do that, it means that the design is bad.
Jacek
Hi Jacek.
Hi Michael,
Casting function in vtbl is not an acceptable solution. If you have to do that, it means that the design is bad.
So in short that means that using deriving for objects to implement derived interfaces is not possible in Wine, as it is not cleanly implementable in plain C. Too bad, but I also don't like casts in the vtbl, as they are definitely not typesafe. I think one could come up with a preprocessor macro hack that enforces typesafety and results in plain casts, but macro-magic is rightly frowned upon in wine. I don't think anyone would like to maintain the macro-mess that would be needed (including myself).
Any oppions on that?
I think other solution would be better. There is no reason for xmlnode object to implement any interface.
xmlnode could provide a default implementation for most of the functions, so one only has to override the functions that need specific handling. If derivation could be used, one does not have to write tons of separate stubs that just do the same, namely forward calls to the xmlnode default handlers (I would call that poor man's derivation), which would kill a lot of boilerplate source code, and thus reduce executable size (who needs 300[1] functions containing [if gcc optimizes correctly, not verified] just one jump instruction if the address of the target could have been put instead in the vtable) and improve performance a bit. On the other hand, 300 jumps instructions (if the optimizer works...) are just 1.5 kilobytes, and that shouldn't hurt that much on modern computers.
[1] order-of-magnitude estimate.
All its child objects already have IXMLDOMNode interface.
Yes, of course, as the child objects implement interfaces that are derived from IXMLDOMNode (either directly or indirectly).
xmlnode could be a common struct stored by child objects and there could be set of function with common method implementation.
Right. That is quite like the direction I was also heading to. The extra memory allocation and use of COM aggregation for the xmlnode object definitely seems like overkill, and is also avoided by the containment approach you suggest.
This way we could also avoid ugly tests for node type in node.c, because we can handle the difference outside node.c.
Very good point, didn't take a look at them. These tests should get expunged and replaced by different implementation chosen by the vtable of the IXMLDOMFoo imlementing objects.
Thanks for your input, Michael Karcher
-
Jacek Caban -
Michael Karcher