Date: Wed, 30 Oct 2002 04:55:40 +0100 From: Dag-Erling Smorgrav <des@ofug.org> To: Bill Fenner <fenner@research.att.com> Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: Moving forward with libfetch (was Re: cvs commit: src/lib/libfetch common.c) Message-ID: <xzpvg3k4m2r.fsf@flood.ping.uio.no> In-Reply-To: <200210300111.RAA12355@windsor.research.att.com> (Bill Fenner's message of "Tue, 29 Oct 2002 17:11:31 -0800") References: <200210300111.RAA12355@windsor.research.att.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=
Bill Fenner <fenner@research.att.com> writes:
> Given that _fetch_read() has to be able to return 0 to permit reading to
> EOF, is there really something that needs to be improved from rev 1.33?
Yes, 1.34 also fixes a bug that prevented the caller from being able
to tell the difference between EOF and a timeout. This is important
for fetch(1) (and would be for pkg_add(8) as well except it ignores
read errors).
To summarize, 1.33 was correct but there were still cases where
_fetch_read() returned the wrong value. 1.34 fixed that but went a
little bit too far.
As for the core dumps, it seems that fixing _fetch_read() exposed a
bug in _http_fillbuf(), where it fails to set the error flag if
_fetch_read() returns -1, causing _http_readfn() to return 0 instead
of -1 (unless some data was read before the error, in which case it
will return the amount of data read, then 0 on subsequent calls).
This bug was there all along, but never manifested itself because,
until 1.34, _fetch_read() would never return -1 unless select() did,
which as far as I can tell can't happen in that particular piece of
code.
I've attached a patch for http.c.
DES
--
Dag-Erling Smorgrav - des@ofug.org
--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=http.diff
Index: http.c
===================================================================
RCS file: /home/ncvs/src/lib/libfetch/http.c,v
retrieving revision 1.62
diff -u -u -r1.62 http.c
--- http.c 27 Oct 2002 15:43:40 -0000 1.62
+++ http.c 30 Oct 2002 03:45:48 -0000
@@ -196,8 +196,10 @@
if (io->chunked == 0) {
if (_http_growbuf(io, len) == -1)
return (-1);
- if ((io->buflen = _fetch_read(io->conn, io->buf, len)) == -1)
+ if ((io->buflen = _fetch_read(io->conn, io->buf, len)) == -1) {
+ io->error = 1;
return (-1);
+ }
io->bufpos = 0;
return (io->buflen);
}
@@ -217,8 +219,10 @@
len = io->chunksize;
if (_http_growbuf(io, len) == -1)
return (-1);
- if ((io->buflen = _fetch_read(io->conn, io->buf, len)) == -1)
+ if ((io->buflen = _fetch_read(io->conn, io->buf, len)) == -1) {
+ io->error = 1;
return (-1);
+ }
io->chunksize -= io->buflen;
if (io->chunksize == 0) {
--=-=-=--
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?xzpvg3k4m2r.fsf>