Add more thorough tests of freopen

[Florian Weimer]

* Joseph Myers:

> freopen is rather minimally tested in libio/tst-freopen and
> libio/test-freopen.  Add some more thorough tests, covering different
> cases for change of mode in particular.

I think it's clear that it's possible to use freopen on streams created
by fopen (where it is somewhat redundant for us) and on the (in glibc's
case initial) values of stdin, stdout, stderr, where it is essential for
non-POSIX platforms.  But there are other ways to create streams.  It
seems to be the interaction with popen, fmemopen, open_memstream, and
glibc's fopencookie has been ignored so far.

Some of these combinations do not work at all in the glibc
implementation, something that cannot be fixed easily, so I think we
should document these limitations in the glibc manual.  This seems to be
unaddressed in the POSIX text.

Considering these gaps in specification, the selection of additional
tests looks useful to me.

You could add mtrace and <support/descriptors.h> checks around all the
tests to verify that there are now leaks, as in this test:

  [PATCH v4 09/13] dirent: Add tst-closedir-leaks
  <https://inbox.sourceware.org/libc-alpha/c507e98237dc4cec6da0536eed3ac8f13697468f.1725047142.git.fweimer@redhat.com/>

Descriptor checks around each subtest would provide more coverage, but
just around everything would be okay, too.

A separate test (using support_become_root, support_can_chroot) could
check what happens if /proc is not mounted and the /proc/self/fd code
path is used.

> There are support/ changes to add helper interfaces used in checking
> file contents; those are worth careful review.

The support/ changes should be in a separate commit, so that we can
backport them independently if necessary.

> diff --git a/stdio-common/tst-freopen2.c b/stdio-common/tst-freopen2.c
> new file mode 100644
> index 0000000000..4a5a3c016e
> --- /dev/null
> +++ b/stdio-common/tst-freopen2.c
> @@ -0,0 +1,446 @@

> +  /* Test each pair of old and new modes from r w a.  */
> +
> +  verbose_printf ("Testing r -> r\n");
> +  fp = xfopen (file1, "r");
> +  fp = freopen (file2, "r", fp);
> +  TEST_VERIFY_EXIT (fp != NULL);
> +  TEST_COMPARE_FILE_STRING (fp, "file2");
> +  xfclose (fp);

As a general comment, I would have expected a check that the
non-modified file is in fact not modified.  Seems very likely given our
implementation, but still.  (I wonder if it would be conforming to
create a hard link to the new name on freopen.)

> +  verbose_printf ("testing freopen with NULL, different mode\n");
> +  fp = xfopen (file1, "w");
> +  ret = fputs ("different mode", fp);
> +  TEST_VERIFY (ret >= 0);
> +  fp = freopen (NULL, "r", fp);
> +  TEST_VERIFY_EXIT (fp != NULL);
> +  TEST_COMPARE_FILE_STRING (fp, "different mode");
> +  xfclose (fp);
> +
> +  /* Test freopen with NULL, renamed file.  */

This is supposed to exercise the /proc/self/fd path and show that no new
pathname lookup is peformed, right?  (Along with the following test.)  A
comment could mention that.

> diff --git a/support/file_contents.h b/support/file_contents.h
> new file mode 100644
> index 0000000000..eea05eebc0
> --- /dev/null
> +++ b/support/file_contents.h
> @@ -0,0 +1,56 @@

> +#include <support/check.h>
> +#include <stdio.h>
> +
> +__BEGIN_DECLS
> +
> +/* Check that an already-open file has exactly the given bytes,
> +   starting at the current offset.  */
> +int support_compare_file_bytes (FILE *fp, const char *contents, size_t length);

The comment should say if the file position indicator is updated or not.
Missing description of the sense of the return value (0 apparently means
equal, 1 means unequal or read error).  Should this report read errors
via process termination?

> +/* Check that an already-open file has exactly the given string as
> +   contents, starting at the current offset.  */
> +int support_compare_file_string (FILE *fp, const char *contents);

Likewise.

> +/* Check that a not-currently-open file has exactly the given
> +   bytes.  */
> +int support_open_and_compare_file_bytes (const char *file,
> +					 const char *contents,
> +					 size_t length);
> +
> +/* Check that a not-currently-open file has exactly the given string
> +   as contents, starting at the current offset.  */
> +int support_open_and_compare_file_string (const char *file,
> +					  const char *contents);

Missing return value description.

> +/* Compare bytes read from an open file with the given string.  */
> +#define TEST_COMPARE_FILE_STRING(FP, CONTENTS)			\
> +  TEST_COMPARE (support_compare_file_string (FP, CONTENTS), 0)

Similar question about the file position indicator.

> +/* Open a file and compare bytes read from it with the given string.  */
> +#define TEST_OPEN_AND_COMPARE_FILE_STRING(FILE, CONTENTS)		\
> +  TEST_COMPARE (support_open_and_compare_file_string (FILE, CONTENTS), 0)

Suggest: “[Read] a file”

The file does not remain open after the call.

Thanks,
Florian