[PATCH v2] libio: Add test case for fflush

[Florian Weimer]

* Frédéric Bérat:

> diff --git a/libio/Makefile b/libio/Makefile
> index 4370152964..27534b9675 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -100,6 +100,8 @@ tests = \
>    tst-fclose-unopened \
>    tst-fclose-unopened2 \
>    tst-fdopen-seek-failure \
> +  tst-fflush \
> +  tst-fflush-NULL \
>    tst-fgetc-after-eof \
>    tst-fgetwc \
>    tst-fgetws \
> @@ -146,6 +148,9 @@ tests = \
>    tst_wscanf \
>    # tests
>  
> +# tst-fflush-NULL as XFAIL until read stream bug is fixed
> +test-xfail-tst-fflush-NULL = yes

Please file a new bug in Bugzilla and reference its number in the
comment.

> diff --git a/libio/tst-fflush-skeleton.c b/libio/tst-fflush-skeleton.c
> new file mode 100644
> index 0000000000..5db9fc96da
> --- /dev/null
> +++ b/libio/tst-fflush-skeleton.c
> @@ -0,0 +1,196 @@

> +#ifndef	FILE_FLUSH_TYPE
> +# define FILE_FLUSH_TYPE FILE_FLUSH
> +# define S_FLUSH_TYPE "FILE"
> +#endif

I suggest to put this in to the libio/tst-fflush.c wrapper.

> +struct
> +{
> +  FILE *file;
> +  char *name;
> +  int fd;
> +  char *mfile;
> +} files[TEST_FILE_COUNT];
> +
> +static void
> +base_init (int file)
> +{
> +  files[file].file = NULL;
> +  files[file].fd = -1;
> +  files[file].name = NULL;
> +  files[file].mfile = NULL;
> +}
> +
> +static void
> +file_init (int file)
> +{
> +  int fd = -1;
> +
> +  base_init (file);
> +
> +  if (file >= TEST_FILE_COUNT)
> +    return;
> +
> +  xclose (create_temp_file ("tst-fflush", &files[file].name));
> +
> +  fd = xopen (files[file].name, O_RDONLY, 0);
> +  files[file].mfile = (char *) xmmap (NULL, CONTENT_SZ_MAX, PROT_READ,
> +				      MAP_SHARED, fd);
> +  xclose (fd);
> +}
> +
> +static void
> +file_cleanup (int file)
> +{
> +  free (files[file].name);
> +  xmunmap (files[file].mfile, CONTENT_SZ_MAX);
> +
> +  base_init (file);
> +}
> +
> +static int
> +file_changed (int to_check, int global_flush, const char *mode)
> +{

This could use FILE_FLUSH_TYPE directly.

> +  struct stat stats = { };
> +  bool content_matches = 0;
> +  char expected[CONTENT_SZ_MAX] = { };
> +
> +  verbose_printf ("Check that %s (%d) exactly contains the data we put in\n",
> +		  files[to_check].name, to_check);
> +
> +  /* File should contain "N:M" where both N and M are one digit exactly.  */
> +  snprintf (expected, sizeof (expected), "%d:%d", global_flush, to_check);
> +  content_matches
> +    = (strncmp (files[to_check].mfile, expected, sizeof (expected)) == 0);

I think this should use memcmp, not strncmp?  So perhaps

   TEST_COMPARE_BLOB (files[to_check].mfile, sizeof (expected),
                      expected, sizeof (expected));

> +  TEST_VERIFY_EXIT (content_matches != 0);

Maybe:

  if (support_record_failure_is_failed)
    FAIL_EXIT1 ("exiting due to previous failure");

We should probably something like

  support_record_failure_barrier ();

for that.

> +  TEST_VERIFY_EXIT (fstat (files[to_check].fd, &stats) >= 0);
> +  TEST_VERIFY_EXIT (stats.st_size == 3);
> +  if (strncmp(mode, "r", 1) == 0)

Missing space before “(mode”.  Alternatively, just use: mode[0] == 'r'

> +    TEST_VERIFY_EXIT (lseek (files[to_check].fd, 0, SEEK_CUR) == 1);
> +  else
> +    TEST_VERIFY_EXIT (lseek (files[to_check].fd, 0, SEEK_CUR) == 3);

I think the exit isn't required, so maybe (with a comment)?

  TEST_COMPARE (lseek (files[to_check].fd, 0, SEEK_CUR),
                mode[0] == 'r' ? 1 : 3);

> +  /* Not reached if the data doesn't match.  */
> +  return FILE_CHANGED;
> +}
> +
> +static void
> +file_flush (int global_flush, const char *mode)
> +{
> +  for (int i = 0; i < TEST_FILE_COUNT; i++) {

“{” should be on its own line.

> +      files[i].file = xfopen (files[i].name, mode);
> +      TEST_VERIFY_EXIT (files[i].file != NULL);
> +      files[i].fd = fileno (files[i].file);
> +  }
> +
> +  /* Print a unique identifier in each file, that is not too long nor contain
> +     new line to not trigger _IO_OVERFLOW/_IO_SYNC.  */
> +  for (int i = 0; i < TEST_FILE_COUNT; i++) {

Likewise.

> +      if (strncmp(mode, "r", 1) == 0)

Missing space, or even better: mode[0] == 'r'

> +	{
> +	  char c = (char) fgetc (files[i].file);
> +	  c = (char) fgetc (files[i].file);
> +	  ungetc (c, files[i].file);

Please make the ungetc conditional and run this subtest twice.

> +	}
> +      else
> +	{
> +	  fprintf (files[i].file, "%d:%d", global_flush, i);
> +	}
> +  }
> +
> +  if (global_flush)

Could use FILE_FLUSH_TYPE directly.

> +    {
> +      fflush (NULL);
> +    }
> +  else
> +    {
> +      for (int i = 0; i < TEST_FILE_COUNT; i++)
> +	fflush (files[i].file);
> +    }

Missing checks for fflush return values.

Overall structure of the test looks reasonable to me, thanks.

Florian