Re: [xml] xmlIO enhancements?

[Daniel Veillard <veillard redhat com>]

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

  if you work around improving xmlIO it might be something to keep in mind.
That could get rid of one of the 3 stat() calls.

> 2) Any reason not to enhance the last line of __xmlIOErr:
>
>   __xmlSimpleError(domain, code, NULL, IOerr[idx], extra);
>
> to say
>
>   __xmlSimpleError(domain, code, strerror(errno), IOerr[idx], extra);
>
> if this change passes all tests i'll submit a patch unless someone
> thinks it a bad idea.

  the main question is the portability of strerror() according to
  http://www.opengroup.org/onlinepubs/009695399/functions/strerror.html
this should be ISO C, but I'm wondering about it on weird platforms/compilers.

> 3) xmlGzfileOpen_real does not have anything like: 
>
>     if (fd == NULL) xmlIOErr(0, path);
>
> after the gzopen(path, "rb") call, unlike xmlFileOpen_real. this
> minor inconsistency could be addressed easily enough. Any reason not to?

   Makes sense of course, yes.
  
> p.s. have a good vacation dv!

   Thanks :-) I may actually work a bit on libxml2 during the first week,

Daniel

-- 
Daniel Veillard      | Red Hat Desktop team http://redhat.com/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/