S , 2011-10-08 21:13 +0100, Chris Morley rakstīja: > On 08/10/2011 20:02, My Th wrote: > > It might be that the changes could be implemented in other place which > > would shrink the patch size, but I don't know the OB code that well, so > > I went for the most straightforward solution. > > Could a single line > static int n = LoadAllPlugins(); > in the function OBPlugin::PluginMap() replace all the other calls? > The local static variable n would be initialized once on first use, but > not subsequently used. > > I'm impressed that you took the trouble to understand the OBPlugin code. > In the hybrid systems Cygwin and Mingw32 the plugins were originally not > being loaded. Tim Vandermeersch made some changes which fixed this, but > it would be helpful, if you are interested, in explaining how they work. > (The OBPlugin code is a bit obscure; I can understand why macros are > considered evil, http://www.parashift.com/c++-faq-lite )
The biggest problem with the macros in this case is that the code in them is so badly formatted that reading it makes head hurt. And on top of that one has to follow the subclassing of OBPlugin -> plugin types -> plugin sub-types. OBPlugin::PluginMap() is the key point here. It has to be fully populated before any lookup in that is done. Plugin map is a two level construct: - First is a list of plugin types - Second is a list of sub-types for each plugin type As I understand, the plugin map is populated by making instances of specific plugin sub-types. Loading plugins makes also all the instances of those plugin sub-types -> plugin type map gets populated. So the function OBPlugin::LoadAllPlugins() has to be called before the lookup in plugin map is done. Besides loading the dynamic libraries, OBPlugin::LoadAllPlugins() also have a hook for making instances of plugin classes using information in a text file plugindefines.txt. The problem with putting OBPlugin::LoadAllPlugins() inside of OBPlugin::PluginMap() is that OBDefine class, which makes instances of plugin classes given in the text file, itself is a plugin and it fails to make those instances if plugin loading is put there. It is required to make an instance of OBDefine after all the plugins have been loaded to ensure that it can make instances of all the classes the data file is referring to. OB knows about all available plugins only after all those instances are made. The code in my patch ensures that all of the plugins are properly loaded and it doesn't touch platform specific code for loading dynamic libraries (that is done by DLHandler). It also doesn't change any exposed interface, so ABI should be fine AFAIK. Only issue is that I can't see a single entry point where to put OBPlugin::LoadAllPlugins(), at least not for the current OB plugin system. Maybe the solution is to make wrapper function for OBPlugin::PluginMap() and OBPlugin::LoadAllPlugins(), and make other functions call the wrapper, letting only sub-class constructors to call OBPlugin::PluginMap() directly. Does it sound reasonable? I can make such version of the patch if needed. Reinis ------------------------------------------------------------------------------ All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2dcopy2 _______________________________________________ OpenBabel-Devel mailing list OpenBabel-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbabel-devel
