When we upgraded our Musl version, we got a newer version of setvbuf()
changed in Musl commit 0b80a7b04 in 2018. The old version of setvbuf()
silently ignored the user's buffer and continued to use stdio's default
buffer - a 1024-byte buffer. The new version tried to be better, and
use the user-supply buffer, but created a problem:
Musl's internal implementation uses the first UNGET (=8) bytes of the
buffer for ungetc() support. When Musl wants a 1024-byte buffer, they
actually allocate 8+1024. In order not to change that implementation
detail, their setvbuf() subtracted 8 bytes from the user's supplied
buffer and its length. So if the user provided a 131072 byte (128 KB)
buffer, actually only 131064 bytes were used as the buffer size. This
causes problems for reading from block devices - where the size of
the buffer must be a multiple of BSIZE, and no longer is - as exposed
in issue #1180.
The ugly fix in this patch is to subtract 512, instead of 8, from the
user's given buffer size. A better fix would have been to move the
8-byte unget buffer outside the main buffer, or alternatively for
setvbuf() to allocate the buffer itself (and remember to free it later)
but both fixes would require much more elaborate changes to Musl's
stdio, which I didn't want to do.
The code in this patch is a modified version of
musl_1.1.24/src/stdio/setvbuf.c.
Fixes #1180.
Signed-off-by: Nadav Har'El <n...@scylladb.com>
---
libc/stdio/setvbuf.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 libc/stdio/setvbuf.c
diff --git a/libc/stdio/setvbuf.c b/libc/stdio/setvbuf.c
new file mode 100644
index 00000000..81614307
--- /dev/null
+++ b/libc/stdio/setvbuf.c
@@ -0,0 +1,43 @@
+#include "stdio_impl.h"
+
+/* The behavior of this function is undefined except when it is the first
+ * operation on the stream, so the presence or absence of locking is not
+ * observable in a program whose behavior is defined. Thus no locking is
+ * performed here. */
+
+int setvbuf(FILE *restrict f, char *restrict buf, int type, size_t size)
+{
+ f->lbf = EOF;
+
+ if (type == _IONBF) {
+ f->buf_size = 0;
+ } else if (type == _IOLBF || type == _IOFBF) {
+ // Because of https://github.com/cloudius-systems/osv/issues/1180
+ // in OSv we need to subtract BSIZE (512) bytes, where Musl subtracts
+ // UNGET (8) bytes from the user's buffer size. This subtraction is
+ // ugly and wasteful and we should eventually consider a different
+ // approach. Two better but considerably more complex approaches:
+ // 1. Move the UNGET buffer to be in the FILE object, not the
+ // buffer, so we could use the full buffer for actual reading.
+ // 2. Allocate size bytes here (by the way, POSIX does this when
+ // buf==NULL and we don't support it yet) - instead of using
+ // the user's buffer. We can size the allocation at UNGET+size
+ // so we'll have real buffer size of "size".
+ // The problem with this approach is that we'll need to remember
+ // to free this buffer when the file is closed (or buffer is
+ // changed again) and this code is currently missing.
+#define BSIZE_ALIGNED_UNGET 512
+ if (buf && size >= BSIZE_ALIGNED_UNGET) {
+ f->buf = (void *)(buf + BSIZE_ALIGNED_UNGET);
+ f->buf_size = size - BSIZE_ALIGNED_UNGET;
+ }
+ if (type == _IOLBF && f->buf_size)
+ f->lbf = '\n';
+ } else {
+ return -1;
+ }
+
+ f->flags |= F_SVB;
+
+ return 0;
+}
--
2.31.1
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/20211129120711.970264-1-nyh%40scylladb.com.
