O , 2011-10-18 22:40 +0100, Chris Morley rakstīja: > On 18/10/2011 20:18, My Th wrote: > > Hi! > > > > I have problems with your version of the patch. I'm using Gentoo Linux > > with gcc 4.6.1. > > > > Running the test program I get this: > > Try finding a forcefield in blank state.. > > terminate called after throwing an instance of > > '__gnu_cxx::recursive_init_error' > > what(): std::exception > > Aborted > > > It seems Visual C++ and gcc treat this differently, presumably Visual > C++ has set the internal 'initialized' flag on the variable n before > LoadAllPlugins() has returned, while gcc does it afterwards. > PluginMap() is called when there is any use of any plugin and I see the > first use is when OpGenAlias in the API code is made. The recursive > passes I suspect are all the other plugins being made when > LoadAllPlugins() runs.
I guess then I don't completely understand the meaning of static variable. In my understanding it is a variable that preserves its state in between function calls, but still its value can be changed during the execusion of the program.. so the compiler doesn't know actually what the value of n will be before LoadAllPlugins() returns, so IMHO gcc does the right thing. Or it might be that Visual C++ sees that n is not really used anywhere and decides that it doesn't really matter. GCC gives a warning that n is not used anywhere. I haven't stepped trough the code, because in debugger I couldn't see the exeption, it was just runaway process. But I think the recursive initialization comes from that when PluginMap() is accessed for the first time in LoadAllPlugins() OBDefine hook also accesses PluginMap() and LoadAllPlugins() is called again before the first has returned. > > > I think the reason for this exception is the same as I mention for why > > LoadAllPlugins() can't be called from PluginMap() - the hook to > > OBDefine. > > > > Also there is no point making the function LoadAllPlugins() to return > > int if its not used. As it is now LoadAllPlugins() is called every time > > the PluginMap() is accessed. In my version I had an if statement to > > ensure that it is called only once and the status AllPluginsLoaded was > > updated inside the function just before the hook for OBDefine (which > > also accesses PluginMap()). > In the line > static int n = LoadAllPlugins(); > in PluginMap(), LoadAllPlugins should be called only once when the local > static variable n is initialized on the first pass. This certainly > happens for me (using the debugger); does it not for you? Couldn't verify how it goes for me, I'll try setting some breakpoints and stepping trough. But as I said, I don't understand why would in this case the function be called only once? As I understand if, for example, the first pass assigns n=3, then on the second pass n would be 3 at the beginning, but the function could assign n=5 if LoadAllPlugins() now finds two more dynamic libraries to load. So the function has to be called each time to know what value n gets. > > > > > I can't comment right now on the issue with OBLocale since the patch in > > this form is clearly not working for me. I'm surprised it works on Win. > > > > I think the move of LoadAllPlugins() to private status is very > > appropriate, so I'm changing that also in my version. > > > > Another question I have is how to handle openbabel/dlhandler.h include? > > Is it really necessary to have it in both places - plugin.h and > > plugin.cpp? I think one of them is enough. Probably tough it should go > > in plugin.cpp since there is the definition of LoadAllPlugins() which > > uses it. Am I correct? > Yes, I meant to move it, but instead copied it. On second thoughts, I > think it is better in plugin.h otherwise it needs to be included in a > few other places in OB. Better to leave pluhin.h as the top level > inclusive header for this area. Ok, I don't know all the implications of this. I just was reading Code Standarts in OB wiki and it says: Headers must use forward declarations. For example, if you use OBAtom, you should #include <openbabel/atom.h> not mol.h. But I'm not sure how to interpret it in this case. > > > > In my test program I also add a check for plugin iterators. Attached > > updated versions of the patch and test program. > > > > With gcc not playing ball, I've committed your patch, which seems to > work fine for me, to trunk. Thanks :) Still I think it might be a good idea to make LoadAllPlugins() and AllPluginsLoaded private since those should not be directly accessible for derived classes. Reinis ------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2d-oct _______________________________________________ OpenBabel-Devel mailing list OpenBabel-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbabel-devel
