Re: [xml] xmlIO enhancements?

[Joel Reed <joelwreed comcast net>]

On Tue, Apr 05, 2005 at 04:32:02PM -0400, Daniel Veillard wrote:
> On Tue, Apr 05, 2005 at 03:49:05PM -0400, Joel Reed wrote:
> > On Tue, Apr 05, 2005 at 01:34:32PM -0400, Daniel Veillard wrote:
> > > On Tue, Apr 05, 2005 at 01:22:50PM -0400, Joel Reed wrote:
> > > > I'm reviewing xmlFileOpen_real and xmlGzfileOpen_real in xmlIO.c and have
> > > > two quick questions/possible improvements (i'll fire off a patch if no
> > > > dissenters).
> > > >
> > > > 1) xmlFileOpen_real does a
> > > >
> > > >   if (!xmlCheckFilename(path)) return(NULL);
> > > >
> > > > before the fopen(path, "r") call. This quick-out prevents library users
> > > > from ever seeing a message in their structure error handler like
> > > > "No such file or directory." By deleting this check, we can let fopen call
> > > > fail and set errno, then let normal libxml error handling bubble the
> > > > error back up to library users. Any reason why not to do this?
> > >
> > >    the optimistic way might be better yes. Might be faster too, see
> > >
> > > http://bugzilla.gnome.org/show_bug.cgi?id=168414
> >
> > ok, did the patch & i think it uncovered a few test case inconsistencies.
> > for example in XMLtests, we do:
> >
> > 1) $(top_builddir)/xmllint test/$$i 2>&1 > result.$$name
> > 2) $(top_builddir)/xmllint result.$$name
> >
> > this fails for test/ent2 which has a line:
> >
> >   <!ENTITY title PUBLIC "-//MY-TITLE//FR" "title.xml">
> >
> > it fails because title.xml is in test/ and #2 above produces a 
> > "No such file or directory I/O error." because "result.$$name"
> > is not in test/
> >
> > basically, the patch is making visible these errors which silently failed 
> > before. 
> >
> > how should i fix the testcases? would it be acceptable to say 
> >
> > 1) $(top_builddir)/xmllint test/$$i 2>&1 > test/result.$$name
> > 2) $(top_builddir)/xmllint test/result.$$name
>
>   it is more complex than that I'm afraid. It's is a lookup for an external
> parsed entity, which is not required . It does show up:
>
> paphio:~/XML -> xmllint test/ent2 > ent2
> paphio:~/XML -> xmllint ent2
> warning: failed to load external entity "title.xml"
> <?xml version="1.0"?>
> <!DOCTYPE EXAMPLE SYSTEM "example.dtd" [
> <!ENTITY xml "Extensible Markup Language">
> <!ENTITY title PUBLIC "-//MY-TITLE//FR" "title.xml">
> <!ENTITY image SYSTEM "img.gif" NDATA GIF>
> ]>
> <EXAMPLE>
>   &title;
> This text is about XML, the &xml; and this is an embedded <IMG src="image"/>
> </EXAMPLE>
> paphio:~/XML ->
>
>   but as a warning only. Turning it into a (fatal) error and you simply break
> the XML spec compatibility of the parser !
>   You cannot raise an I/O error as a fatal error all the time. Also if the
> file doesn't exist on-disk it may still be present in a catalog, in that
> case no warning and no error should be raised.

presumably then, the best approach is to fix the callers who rely on
"file doesn't exist on-disk" != error since only callers know whether
its an error or not. 

hmm, is this still worth the trouble? i'll look at the code a bit.

jr