From owner-cvs-all Tue Oct 29 19:55:45 2002 Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 761E037B401; Tue, 29 Oct 2002 19:55:43 -0800 (PST) Received: from flood.ping.uio.no (flood.ping.uio.no [129.240.78.31]) by mx1.FreeBSD.org (Postfix) with ESMTP id B041343E7B; Tue, 29 Oct 2002 19:55:42 -0800 (PST) (envelope-from des@ofug.org) Received: by flood.ping.uio.no (Postfix, from userid 2602) id 47381534E; Wed, 30 Oct 2002 04:55:40 +0100 (CET) X-URL: http://www.ofug.org/~des/ X-Disclaimer: The views expressed in this message do not necessarily coincide with those of any organisation or company with which I am or have been affiliated. To: Bill Fenner Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: Moving forward with libfetch (was Re: cvs commit: src/lib/libfetch common.c) References: <200210300111.RAA12355@windsor.research.att.com> From: Dag-Erling Smorgrav Date: Wed, 30 Oct 2002 04:55:40 +0100 In-Reply-To: <200210300111.RAA12355@windsor.research.att.com> (Bill Fenner's message of "Tue, 29 Oct 2002 17:11:31 -0800") Message-ID: Lines: 31 User-Agent: Gnus/5.090007 (Oort Gnus v0.07) Emacs/21.2 (i386--freebsd) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG --=-=-= Bill Fenner 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