[PATCH v7] stdio-common: Add more tests of the setvbuf() function

[Florian Weimer]

* Nick Clifton:

> This patch series extends the current test of the setvbuf() function
> in order to cover more use cases. In particular it checks that:
>
>   * stdout and stderr can be set into unbuffered mode and that writes
>     to either stream appear immediately.
>   * stdin and stderr can be set into line buffered mode and that
>     writes to either are buffered until a new-line character is
>     encountered.
>   * full buffering can be set on reads and writes to an in-memory
>     file and that buffering does take place.
>   * line buffering can be set on reads and writes to an in-memory
>     file and that buffering on writes does happen.
>   * buffering can be disabled for reads and writes to/from an
>     ordinary file and that data written is immediately available
>     for reading.

The FIXMEs need to be resolved or removed.  Using xmmap as suggested is
fine, but as explained below, it does not allow us to detect incorrectly
written trailing null bytes, so maybe reading directly from the file is
better.

The copyright year should be 2025 (only) now.

As far as I can see, we do not test this code:

  /* FIXME This can/should be moved to genops ?? */
  if (fp->_flags & (_IO_LINE_BUF|_IO_UNBUFFERED))
    {
      /* We used to flush all line-buffered stream.  This really isn't
	 required by any standard.  My recollection is that
	 traditional Unix systems did this for stdout.  stderr better
	 not be line buffered.  So we do just that here
	 explicitly.  --drepper */
      _IO_acquire_lock (stdout);

      if ((stdout->_flags & (_IO_LINKED | _IO_NO_WRITES | _IO_LINE_BUF))
	  == (_IO_LINKED | _IO_LINE_BUF))
	_IO_OVERFLOW (stdout, EOF);

      _IO_release_lock (stdout);
    }

The inner comment does not match current POSIX.  POSIX requires that
input on an unbuffered stream triggers flush on any line-buffered
stream, not just standard output.  This is tricky to implement properly.
POSIX has some language around deadlock detection specifically for this
case.

I filed:

  No deadlock detection for flushing due to line buffering
  <https://sourceware.org/bugzilla/show_bug.cgi?id=32566>

  Line-buffered flushing on unrelated input streams is restricted to
  stdout
  <https://sourceware.org/bugzilla/show_bug.cgi?id=32567>

> diff --git a/stdio-common/tst-setvbuf2.c b/stdio-common/tst-setvbuf2.c
> new file mode 100644
> index 0000000000..1e1c6adae7
> --- /dev/null
> +++ b/stdio-common/tst-setvbuf2.c

> +static int
> +do_test (void)
> +{
> +  TEST_VERIFY_EXIT (setvbuf (stdin,  NULL, _IONBF, 0) == 0);
> +  TEST_VERIFY_EXIT (setvbuf (stderr, NULL, _IONBF, 0) == 0);
> +  TEST_VERIFY_EXIT (setvbuf (stdout, NULL, _IONBF, 0) == 0);
> +
> +  /* The theory was that this test would be run with stdout and stderr
> +     redirected into a single file.  Then writes to stdout and stderr would
> +     be performed with and without newlines and finally the contents of the
> +     file would be examined to find out if any buffering has taken place.
> +     Unfortunately whilst this works when run by hand, or from a makefile
> +     running on an ordinary terminal, it does not work when run by Linaro's
> +     CI system.
> +
> +     There is no way to distinguish Linaro's execution environment from a
> +     normal execution environment.  (Testing that stdout/stderr are attached
> +     to terminals does not work, since they are not - they are attached to an
> +     output file: tst-setvbuf.out).  All of which means that in the end we
> +     cannot test the behaviour of buffering when writing to standard files.
> +     (See tst-setvuf4.c and tst-setvbuf5.c for tests that do work when
> +     writing to disk based files).
> +
> +     Hence if we get this far, we consider that the test has passed.  */

We can use dup2 (or xdup2) to install any file descriptor we want before
calling setvbuf.  For example, we can use regular files and check using
FUSE if a single character is written immediately.

I think the previous approach using regular files and mmap would work,
too.  The only tricky part is that if we redirect standard output, the
regular test error reporting will not work (but the exit status will
still reflect the test outcome).

This applies to to the other tests as well.

> diff --git a/stdio-common/tst-setvbuf3.c b/stdio-common/tst-setvbuf3.c
> new file mode 100644
> index 0000000000..8a7e55f85a
> --- /dev/null
> +++ b/stdio-common/tst-setvbuf3.c
> +  /* Use a library allocated line buffer for stderr.  */
> +  if (setvbuf (stderr, NULL, _IOLBF, LOCAL_BUF_SIZE) != 0)
> +    FAIL_UNSUPPORTED ("tst-setvbuf3.c: POSIX standard does not guarantee\
> + being able to set line buffering mode on stderr");
> +
> +  /* Use a program allocated line buffer for stdout.  */
> +  if (setvbuf (stdout, local_buf, _IOLBF, sizeof local_buf) != 0)
> +    FAIL_UNSUPPORTED ("tst-setvbuf3.c: POSIX standard does not guarantee\
> + being able to set line buffering mode on stdout");

We should test our implementation and what we document (or otherwise
expect to work), even if POSIX does not require a specific behavior.  So
if we implement this, it should be FAIL_EXIT1, not FAIL_UNSUPPORTED.
And you can trim the comments as well.

> +   Note - because of the POSIX rules on the interactions of multiple handles
> +   on the same stream (see section 2.5.1 "Interaction of File Descriptors
> +   and Standard I/O Streams" in the POSIX specification) we cannot just open a
> +   file twice, once for reading and once for writing and then check that
> +   writes to the file do not happen until the buffer is full.  Nor can we
> +   open a single stream for both reading and writing and test that way
> +   because any time we reposition the file pointer (ie by calling fseek) the
> +   buffer is flushed.
> +
> +   In theory we could use fmemopen() to create a memory backed stream and
> +   then check the buffering behaviour that way.  But it turns out the glibc's
> +   implementation does not support buffering, so that does not work.
> +
> +   Another alternative is open_memstream() - which does use glibc's default
> +   I/O code.  But it turns out that the function is not suitable for this test
> +   as it specifically does not support having the memory buffer examined after
> +   a write has completed but before a flush has been performed.
> +  
> +   So we resort to opening an ordinary file and using mmap to provide us with
> +   a memory page that we can examine.  */

The file mapping approach is undefined according to POSIX.  It's
possible that you can make it defined by using msync with MS_INVALIDATE.
Typical Linux file systems do not need it because Linux has a shared
page cache, and any non-direct write is also reflected there.

It may be easier to test using file descriptors directly because with
mmap alone, it is not possible to detect incorrectly written trailing
null bytes.

Thanks,
Florian