diff mbox series

[uclibc-ng-devel,1/2] stdio: report the position as an open_memstream's size, as POSIX requires

Message ID 20260831080159.3552089-1-lordrasmus@gmail.com
State New
Headers show
Series [uclibc-ng-devel,1/2] stdio: report the position as an open_memstream's size, as POSIX requires | expand

Commit Message

Ramin Moussavi Aug. 31, 2026, 8:01 a.m. UTC 
From: ramin <lordrasmus@gmail.com>

POSIX says of *sizep, after a successful fflush() or fclose():

  the variable pointed to by sizep shall contain the smaller of the current
  buffer length and the number of bytes ... between the beginning of the
  buffer and the current file position indicator

That is the position.  oms_write() stored a high water mark instead,

    if (COOKIE->pos > COOKIE->eof)
            *COOKIE->sizeloc = COOKIE->eof = COOKIE->pos;

so the size only ever grew, and the one assignment in the seek path that
looked like it compensated, *COOKIE->sizeloc = COOKIE->eof, stored the value
that was already there.  Both other implementations agree with POSIX here:
glibc's _IO_mem_sync uses fp->_IO_write_ptr - fp->_IO_write_base, musl's
ms_write does *c->sizep = c->pos.

Two consequences, the second the one that bites:

    "abc",    seek 10, close       reported 3, should be 10
    "abcdef", seek 2, write 'Z'    reported 6 and handed back "abZdef",
                                   should be 3 and "abZ"

A stream reused by rewinding and writing something shorter thus returned the
old length together with the stale tail.

eof keeps its second job, marking how far the buffer holds defined content,
which is where the 0-fill for a seek past the end starts.

POSIX also requires the terminating null after every successful fflush(), and
that cannot be done from a callback: __stdio_wcommit() only reaches oms_write()
when the FILE buffer holds something, so a flush right after an fseek() calls
nothing at all.  The terminator is therefore kept at the position at all times.
Seeking back over existing data would destroy one byte, so that byte is
remembered in the cookie and put back when the position moves on -- glibc
writes the terminator from its flush hook and leaves the data alone, and
programs may rely on that.

SEEK_END is deliberately left as it was.  POSIX makes it
implementation-defined "whether the file position is adjusted relative to the
current buffer length or relative to the buffer size that would be set by an
fflush() call", and the two references pick different sides: musl uses the
buffer length (c->len), glibc the flushed size, which for a write-only stream
is the position.  The comment now says which one this is and why there is a
choice.

Measured on i686 against glibc on the host, seven cases.  Everything POSIX
defines is now bit for bit identical, including "abcdef", seek 2, seek 5,
close, which returns 5 and "abcde" -- the saved byte restored.  The only
difference left is the byte past the reported size after a flush that had
nothing buffered: glibc leaves the old data there and is not conformant in
that corner, we write the null.  A caller must not read past *sizep anyway.

Signed-off-by: Ramin Moussavi <lordrasmus@gmail.com>
---
 libc/stdio/open_memstream.c | 45 ++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)
diff mbox series

Patch

diff --git a/libc/stdio/open_memstream.c b/libc/stdio/open_memstream.c
index 71a84138d..7e8e2134f 100644
--- a/libc/stdio/open_memstream.c
+++ b/libc/stdio/open_memstream.c
@@ -26,8 +26,33 @@  typedef struct {
 	size_t eof;
 	char **bufloc;
 	size_t *sizeloc;
+	size_t saved_at;		/* Where a data byte was replaced by the */
+	char saved_ch;			/* terminator, to put it back later. */
+	char have_saved;
 } __oms_cookie;
 
+/* POSIX wants the terminator in place and the size updated after every
+ * fflush(), but a cookie stream has no flush callback: __stdio_wcommit() only
+ * reaches oms_write() when the FILE buffer holds something.  So both are kept
+ * current at all times instead.  Seeking back over existing data would destroy
+ * the byte at the new position, so that one byte is remembered and restored
+ * once the position moves on -- glibc writes the terminator from its flush hook
+ * (_IO_mem_sync) and leaves the data alone, and programs may rely on that. */
+static void oms_term(register __oms_cookie *c)
+{
+	if (c->have_saved) {
+		c->buf[c->saved_at] = c->saved_ch;
+		c->have_saved = 0;
+	}
+	if (c->pos < c->eof) {
+		c->saved_at = c->pos;
+		c->saved_ch = c->buf[c->pos];
+		c->have_saved = 1;
+	}
+	c->buf[c->pos] = 0;
+	*c->sizeloc = c->pos;
+}
+
 /* Nothing to do here, as memstreams are write-only. */
 /*  static ssize_t oms_read(void *cookie, char *buf, size_t bufsize) */
 /*  { */
@@ -57,13 +82,17 @@  static ssize_t oms_write(register void *cookie, const char *buf, size_t bufsize)
 		}
 	}
 
+	/* bufsize is at least 1 here, so the saved byte -- which sits at pos --
+	 * is about to be overwritten and must not be restored. */
+	COOKIE->have_saved = 0;
+
 	memcpy(COOKIE->buf + COOKIE->pos, buf, bufsize);
 	COOKIE->pos += bufsize;
 
 	if (COOKIE->pos > COOKIE->eof) {
-		*COOKIE->sizeloc = COOKIE->eof = COOKIE->pos;
-		COOKIE->buf[COOKIE->eof] = 0; /* Need to nul-terminate. */
+		COOKIE->eof = COOKIE->pos;	/* Buffer defined this far. */
 	}
+	oms_term(COOKIE);
 
 	return bufsize;
 }
@@ -78,8 +107,12 @@  static int oms_seek(register void *cookie, __offmax_t *pos, int whence)
 	 * unless debugging. */
 	assert(((unsigned int) whence) <= 2);
 
+	/* POSIX leaves it implementation-defined whether SEEK_END refers to the
+	 * buffer length or to the size an fflush() would report; glibc uses the
+	 * latter, which for a write-only stream is the position, so both bases are
+	 * the same here.  musl uses the former. */
 	if (whence != SEEK_SET) {
-		p += (whence == SEEK_CUR) ? COOKIE->pos : /* SEEK_END */ COOKIE->eof;
+		p += COOKIE->pos;
 	}
 
 	/* Note: glibc only allows seeking in the buffer.  We'll actually restrict
@@ -105,10 +138,11 @@  static int oms_seek(register void *cookie, __offmax_t *pos, int whence)
 
 	*pos = COOKIE->pos = --leastlen;
 
-	if (leastlen > COOKIE->eof) {
+	if (leastlen > COOKIE->eof) {	/* Seeked past the end: 0-fill the gap. */
 		memset(COOKIE->buf + COOKIE->eof, 0, leastlen - COOKIE->eof);
-		*COOKIE->sizeloc = COOKIE->eof;
+		COOKIE->eof = leastlen;
 	}
+	oms_term(COOKIE);
 
 	return 0;
 }
@@ -142,6 +176,7 @@  FILE *open_memstream(char **bufloc, size_t *sizeloc)
 		*cookie->buf = 0;		/* Set nul terminator for buffer. */
 		*(cookie->bufloc = bufloc) = cookie->buf;
 		*(cookie->sizeloc = sizeloc) = cookie->eof = cookie->pos = 0;
+		cookie->have_saved = 0;
 
 #ifndef __BCC__
 		fp = fopencookie(cookie, "w", _oms_io_funcs);