Hi, Guillem Jover wrote: > On Tue, 2010-02-02 at 13:04:29 -0600, Jonathan Nieder wrote:
> > Better would be to add a new gzclearerr() function to allow the two > > error indicators to be explicitly cleared. > > There's already a gzclearerr provided by zlib, which clears both the > EOF and error flags. And I think it's perfectly acceptable for code > which might want to handle the interrupted case to call gzclearerr, > and resume the previous zlib call. > > The actual problem though, as you initially pointed out in the > debian-dpkg thread, is that not all zlib IO functions preserve > enough state to always recover from a partial read/write. Thanks for the reminder. (Since I had to look it up, the thread in question is at <http://lists.debian.org/debian-dpkg/2009/10/msg00091.html>.) > I had skimmed previously over the gzio.c (stdio based) code and there > were several other problematic functions, I think gzwrite is the worst one. gzwrite, gzprintf, gzputs, gzgets, gzflush, and gzclose are not resumable, but I suspect it is only worth fixing gzwrite (hence gzputs), gzflush, and gzclose. Considering each function in gzio.c from zlib 1.2.3.5 in turn: - gzread() returns the number of bytes read, so in theory it should be fine. In practice, it is pretty good, too. Just one issue: In transparent mode, on short reads, the fread() call sets the error flag on the file handle but not the zlib error number. The caller has to do another one-byte gzread() to learn the nature of the error. - gzwrite() forgets the number of bytes actually written and returns 0 on error (-1 in gzlib.c). A resumable function would return the short count. - gzprintf(): it is not worth making this resumable. Applications that need a resumable gzprintf should write a sprintf coroutine (or use snprintf() in one fell swoop, if the string to be written is short enough) and use gzwrite(). In the simplest case, the semantics of SA_RESTART might be implementable and good enough, though. - gzputs() is implemented in terms of gzwrite() and shares its problems. - gzgets() is not resumable, but neither is fgets(). A resumable gzgets() would need another output parameter to tell how many bytes were read. - gzputc() and gzputc() are resumable. - gzungetc() does not do I/O. - do_flush() forgets the number of bytes actually written on error, so gzflush() is not resumable. - if gzseek() or gzrewind() fails, the error indicator is not set, so one can’t detect if it is a zlib internal failure or fseek() failure. gzseek() and gzrewind() are resumable. They might screw up the file offset if not called again after an error. - gztell() does not do I/O. - gzoffset() is not implemented. - gzeof() returns 1 for I/O errors as well as EOF. This is fine, but it should be documented. - gzdirect() does not do I/O. - gzclose() is not resumable. Neither is fclose(), unless one calls fflush() first. Unfortunately, a gzflush() is not enough to write out everything that gzclose() is going to write to the file. Maybe a new resumable function to do everything except close the file would help? If a close() call is interrupted, I am not sure what one is supposed to do. Friendly operating systems will prevent that, I guess. gzclose() does not report I/O errors when writing its last few bytes. - gzerror() looks safe. - gzclearerr() does exactly what one would want it to. Sorry for the nonsense! > but doing so now over the new > gzlib.c based code it seems to be in a way better shape, although there > might still be some other problems, I've not checked thoroughly. I haven’t read this over yet. Glad to hear there seems to have been some progress. > The EINTR, EAGAIN, EWOULDBLOCK errors seem to be safe to be handled now > with gzclearerr, as there's no possibility of a partial fread/fwrite > returning with those anymore. write() still can, no? >From gz_comp(), it seems the length of partial writes are still sometimes forgotten: if (have && write(state->fd, state->next, have) != have) { gz_error(state, Z_ERRNO, zstrerror()); return -1; } Hope that helps, Jonathan -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
