GNOME Bugzilla – Bug 108329
xmlSaveFile needs to be more robust
Last modified: 2009-08-15 18:40:50 UTC
This is actually a problem in both libxml1 and 2 xmlSaveFile is not very good wrt reporting errors to the caller. For example, it is possible for xmlSaveFile (and friends) to return success even when an error occured [1]. Also, if an error occurs, it is impossible for the caller to know why because errno gets clobbered. [1] Note that FILE* is buffered and so even if fwrite() passes, the data is not necessarily flushed to disk until fclose() which can fail. One possible solution is to fflush() the FILE* stream and handle any error cases. I personally prefer lower-level i/o, but I can certainly understand you wanting to use FILE*. anyways, below is an example implementation that properly checks handles errors and saves errno for unrecoverable errors so that our caller can decide what to do: int xmlSaveFile (const char *filename, xmlDocPtr doc) { size_t n, written = 0; int ret, fd, size; int errnosave; char *xmlbuf; ssize_t w; if ((fd = open (filename, O_WRONLY | O_CREAT | O_TRUNC, 0666)) == -1) return -1; xmlDocDumpFormatMemory (doc, (xmlChar **) &xmlbuf, &size, TRUE); if (size <= 0) { close (fd); unlink (filename); errno = ENOMEM; return -1; } n = (size_t) size; do { do { w = write (fd, xmlbuf + written, n - written); } while (w == -1 && errno == EINTR); if (w > 0) written += w; } while (w != -1 && written < n); xmlFree (xmlbuf); if (written < n || fsync (fd) == -1) { errnosave = errno; close (fd); unlink (filename); errno = errnosave; return -1; } while ((ret = close (fd)) == -1 && errno == EINTR) ; if (ret == -1) return -1; return 0; }
Hum, that piece of code is unusable as is... there is a whole familly of xmlSave...File functions with a lot more option, including compression etc. Anyway, streaming was deemed more important than error reporting. The current version of xmlSaveFormatFileEnc() which is where the save is being handled is: buf = xmlOutputBufferCreateFilename(filename, handler, cur->compression); if (buf == NULL) return(-1); xmlDocContentDumpOutput(buf, cur, encoding, format); ret = xmlOutputBufferClose(buf); return(ret); If you want more control over your I/O, that's perfectly possible by using the I/O layer and defining your own handlers. See include/libxml/xmlIO.h especially int xmlRegisterOutputCallbacks ( xmlOutputMatchCallback matchFunc, xmlOutputOpenCallback openFunc, xmlOutputWriteCallback writeFunc, xmlOutputCloseCallback closeFunc); Define a matchFunc which will accept the filenames, and define your own open/write/close functions with the associated semantic. Again I think streaming is far more important than doing those check, so I don't really consider this a bug, we differ on implementation decisions, but since it seems there is a way for you to keep a fine control over the I/O the status-quo sounds reasonnable to me. Daniel
it should at least do proper error checking so that it can return -1 on fail. the way it is now, you can't even depend on *that*. errno preservation is just a bonus.
Okay reopened, I may find the time to propagate errors in some way. Daniel
Check the error handling code in release libxml2-2.6.0, I think such problems are now trapped and provided back though a callback or via a global error status. The errno code is preserved as an internal libxml2 error value. thanks, Daniel