On Mon, Feb 12, 2007 at 10:18:03AM -0600, Ted Phelps wrote: > > Hi Daniel, > > As always, thank you for your speedy reply. > > Daniel Veillard writes: > > Okay, fixing the problem sounds fine of course, but I have a couple of > > issues with the patch: > > > > - all thread implementation specific code in libxml2 is contained in > > threads.c module, that patch breal that containment, I would really > > prefer if this code was kept in threads.c > > I thought you might. I initially chose not to because I didn't want to > add new functions to the shared object, but I'm happy to do so if you > are. Or do you have some mechanism to prevent functions from being > exported to the aplication?
see libxml.h in same directory as the C sources, prefix the function names by __ so even if it shows up in the shared object people are not tempted to call them and dont export from include/libxml/*.h > > - it uses the keyword 'volatile' which is not used anywhere else in > > libxml2, I would prefer to avoid it if possible to maximize portability > > (plus its semantic is IMHO rather fuzzy) > > I included the volatile keyword for the Windows code because the > declaration of InterlockedCompareExchangePointer indicates that the > first parameter should be of type 'PVOID volatile *'. I believe that we > can safely leave this out and I have removed it from the patch below. > > For the BeOS implementation, I have changed 'volatile' to vint32, > following the definition of atomic_add: > > http://www.beunited.org/bebook/The%20Support%20Kit/Functions.html > > You may want to consider changing run_once_init to be a vint32 too. Hum, if you know you're in a specific environment where volatile is expected then go ahead, just avoid it for the platform agnostic code :-) > > I'm not sure how to best achieve those improvements, as this code > > really mix the xmlInitParser with really thread-specific code. > > I have shifted the thread-related code into threads.c, leaving only > calls to lock and unlock a global mutex. Looks good except the __ renaming and include file. > > Also we had trouble in the past with the xmlInitGlobals()/ > > xmlInitThreads()/xmlInitMemory() ordering of initializers, the > > interraction are relatively complex to debug I hope the new code > > doesn't have side effect, that seems true as it uses direct memory > > allocations, it's just something to keep in mind. > > I believe this patch should be immmune to such interactions. yes I believe so too now :-) > No worries. Here's a second attempt. Please have a look and let me > know if you'd like me to try a different approach or make additional > changes. Would you be so kind to make the last changes for the function name and include, then I guess that should be commited, thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ _______________________________________________ xml mailing list, project page http://xmlsoft.org/ [email protected] http://mail.gnome.org/mailman/listinfo/xml
