From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12107 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH v7] stdio: implement fopencookie(3) Date: Fri, 17 Nov 2017 12:57:01 -0500 Message-ID: <20171117175701.GE1627@brightrain.aerifal.cx> References: <20171116060055.12265-1-nenolod@dereferenced.org> <20171117173541.GD1627@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1510941433 2673 195.159.176.226 (17 Nov 2017 17:57:13 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 17 Nov 2017 17:57:13 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-12123-gllmg-musl=m.gmane.org@lists.openwall.com Fri Nov 17 18:57:10 2017 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1eFkt3-0000QL-5d for gllmg-musl@m.gmane.org; Fri, 17 Nov 2017 18:57:09 +0100 Original-Received: (qmail 7629 invoked by uid 550); 17 Nov 2017 17:57:14 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 7604 invoked from network); 17 Nov 2017 17:57:13 -0000 Content-Disposition: inline In-Reply-To: <20171117173541.GD1627@brightrain.aerifal.cx> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:12107 Archived-At: On Fri, Nov 17, 2017 at 12:35:41PM -0500, Rich Felker wrote: > On Thu, Nov 16, 2017 at 06:00:55AM +0000, William Pitcock wrote: > > The fopencookie(3) function allows the programmer to create a custom > > stdio implementation, using four hook functions which operate on a > > "cookie" data type. > > > > [...] > > + > > +struct fcookie { > > + void *cookie; > > + cookie_io_functions_t iofuncs; > > +}; > > + > > +struct cookie_FILE { > > + FILE f; > > + struct fcookie fc; > > + unsigned char buf[UNGET+BUFSIZ]; > > +}; > > + > > +static size_t cookieread(FILE *f, unsigned char *buf, size_t len) > > +{ > > + struct fcookie *fc = f->cookie; > > + ssize_t ret = -1; > > + size_t remain = len, readlen = 0; > > + > > + if (!fc->iofuncs.read) goto bail; > > + > > + ret = fc->iofuncs.read(fc->cookie, (char *) buf, len > 1 ? (len - 1) : 1); > > + if (ret <= 0) goto bail; > > + > > + readlen += ret; > > + remain -= ret; > > + > > + f->rpos = f->buf; > > + ret = fc->iofuncs.read(fc->cookie, (char *) f->rpos, f->buf_size); > > + if (ret <= 0) goto bail; > > + f->rend = f->rpos + ret; > > + > > + if (remain > 0) { > > + if (remain > f->buf_size) remain = f->buf_size; > > + memcpy(buf + readlen, f->rpos, remain); > > + readlen += remain; > > + f->rpos += remain; > > + } > > + > > + return readlen; > > + > > +bail: > > + f->flags |= F_EOF ^ ((F_ERR^F_EOF) & ret); > > + f->rpos = f->rend = f->buf; > > + return readlen; > > +} > > At least the following bugs are still present: > > 1. If len==1, the read func is called twice, and might block the > second time even though the requested read of 1 byte succeeded. > > 2. The return value is wrong. It must be between 0 and len and > represent the amount of data made available to the caller. Buffer > filling is not supposed to be included in that since it's an > internal matter, not something the caller sees. > > 3. A return value <=0 from buffer filling step is not cause to set the > EOF or error indicators, since the part the caller requested > succeeded. But see below... Also: 4. The return value of the second read call is basically ignored, except checking for error/eof. The code just assumes at least `f->buf_size` bytes were read successfully, copying junk out of the buffer back to the caller if fewer were read. Rich