From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/7816 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: stdio fixes & internals documentation Date: Thu, 28 May 2015 21:39:37 -0400 Message-ID: <20150529013937.GA32093@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1432863605 3286 80.91.229.3 (29 May 2015 01:40:05 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 29 May 2015 01:40:05 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-7828-gllmg-musl=m.gmane.org@lists.openwall.com Fri May 29 03:40:01 2015 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1Yy9HD-0007jy-OQ for gllmg-musl@m.gmane.org; Fri, 29 May 2015 03:39:59 +0200 Original-Received: (qmail 22093 invoked by uid 550); 29 May 2015 01:39:57 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 21999 invoked from network); 29 May 2015 01:39:50 -0000 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:7816 Archived-At: I'm writing this up because I need to fix a bug in ungetc -- it's failing when the target FILE is at eof rather than clearing the eof status and succeeding, despite having logic in the source that was intended to make this case work right. So I'm going to go ahead and make some tentative documentation of stdio internals for my own use and for others who want to review these changes. This stuff can eventually be cleaned up to go into the musl manual... :-) So far, just for stuff used in reading: f->read() assumes reading is currently a valid operation (so __toread was called successfully without a subsequent __towrite) and the read buffer is empty (but the values of the pointers don't matter). Right now it's responsible for setting EOF and error flags for the file. This is probably a bad idea because the logic, which is stdio-global and not specific to the FILE type, is repeated in different underlying FILE type implementations' read functions. __toread() assumes the FILE is in a valid state to begin reading, but does not misbehave horribly even if it was in the middle of writing. If the FILE's fopen mode does not permit reading, it sets error status and returns EOF. If the file is at EOF, it also returns EOF. This means the caller (such as ungetc) cannot distinguish EOF from "error going into reading mode" without temporarily clearing and restoring the error flag (which would be ugly and inefficient). On failure (either eof status or error switching to read mode) the buffer pointers are left unchanged; on success, they're all set to the beginning of the buffer. The first thing I want to do is fix the known bug in ungetc, and I think the easiest way to do that is to make __toread set valid read buffer pointers when it fails due to eof status. Then, instead of ungetc checking the return value of __toread, it can instead call __toread and then just check rpos. That is, instead of: if ((!f->rend && __toread(f)) || f->rpos <= f->buf - UNGET) { // error it can instead do: if (!f->rend) __toread(f); if (f->rpos <= f->buf - UNGET) { // error or perhaps: if (!f->rpos) __toread(f); if (!f->rpos || f->rpos <= f->buf - UNGET) { // error I like the second version better because it does not assume that a null pointer compares <= any valid pointer, which could be wrong if pointer <= is implemented as a signed comparison. I changed the conditional from !f->rend to !f->rpos to simultaneously optimize two code paths: 1. If f->rpos is initially non-null, the compiler can skip re-checking that it's non-null in the second if. 2. If f->rpos is initially null, then after __toread it needs to be re-read, but only one field (rpos) needs to be loaded from memory instead of two (rend and rpos). Note that if the file is not open in a mode that permits writing, then f->rpos is initially a null pointer and __toread will never set it to something non-null, nor will f->read (since it's not permitted to call f->read without a successful __toread). Once these fixes are taken care of I'd like to look at the EOF logic in f->read() and moving it out to the callers (only __uflow and fread) where we won't have to worry about bugs (which I think exist) in the FILE-type-specific read functions. Rich