On Mon, Jul 27, 2009 at 08:05:37PM -0700, Wang Lam wrote: > > Hi! I came across some (unfortunate) behavior in libxml2 (2.7.3 > and head) error handling that I want to run by you for your comments. > I have a patch that may help one aspect of it, if you are interested.
Yes, I am, sorry for the late reply, I was really busy lately. [...] > // 1 > Based on the API description and mailing-list messages-- > > [Re: [xml] libxml error redirection] > http://mail.gnome.org/archives/xml/2006-April/msg00090.html > > [Re: [xml] Error handling] > http://mail.gnome.org/archives/xml/2005-May/msg00134.html > > --this function (xmlSetStructuredErrorFunc) seems to be the preferable > way to capture errors. yes > (a) xmlGenericError still gets called, which seems surprising and > unfortunate in light of the xmlSetStructuredErrorFunc call. yes as you foudn out there is still many many calls internally to xmlGenericError(), that should be cleaned up in some ways ! [...] > > (b) Worse than being invoked, though, the default xmlGenericError > crashed (instead of, say, printing to stderr). > > From what I see, xmlGenericErrorFunc and xmlStructuredErrorFunc > share a single xmlGenericErrorContext; setting one function > overwrites the other function's context. In the above example > code, the default xmlGenericErrorDefaultFunc expects its context > to be a FILE * (or NULL, to default to stderr), and my custom > xmlStructuredErrorFunc expects a pointer to a writable error-string > buffer (for example). > > Locally, I work around the problem by writing both error-handling > functions (and setting them both together) so that they can share > one context. It's also possible, though, to create a separate > void * xmlStructuredErrorContext so that the two functions don't > have to fight over the same state. yes, I agree with the diagnostic, I came up to the same conclusion, this need fixing ! > With a patch to create xmlStructuredErrorContext, I get a different > result from the above example code: yes, I think I know what you patch does, add the new variable in globals.c, initialize it properly, and use it for the strcutured error paths in error.c > While I'd naturally prefer (a) to be resolved, I don't know whether > it is easy or not (whether anybody is able to go through the code > to assert or verify its resolution). On the other hand, I do have > a draft patch lying around (relative to libxml head) for (b). I'm interested in the b) patch, that would avoid the need for me to do the exact same thing ! > So, I wonder-- > > * Are (a) and (b) behaviors that libxml2's maintainers would also > like changed, or do they represent desired behavior for the system > (e.g., the system is "supposed to" become undefined in // 2)? I ideally would like to have both changed, but realistically right now fixing b) might be sufficient. > * Should I file a bug entry and submit my draft patch for (b) (to create > a new void *xmlStructuredErrorContext)? (It's about 451 lines, so > I don't have it attached to this already-long message, but as you'd > quickly see, most of those lines are just side effects of libxml2's > internal code generation.) yes please or just reply with the attached patch, I expect to do libxml2 maintainace today and tomorrow ! > Your thoughts or suggestions would be welcome. Thanks! thanks a lot for the report and details, spot on ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [email protected] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ _______________________________________________ xml mailing list, project page http://xmlsoft.org/ [email protected] http://mail.gnome.org/mailman/listinfo/xml
