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 _______________________________________________ xml mailing list, project page http://xmlsoft.org/ [email protected] http://mail.gnome.org/mailman/listinfo/xml
