Re: [PATCH v7] stdio: implement fopencookie(3)

[Rich Felker <dalias@libc.org>]

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