[v6] libio: Add test case for fflush
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Test passed
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
Commit Message
Hello,
This patch adds a test to verify that `fflush (FILE)` and `fflush (NULL)` are
semantically equivalent from the FILE perspective, which currently fails if the
file is opened with read mode.
Note: Since a lot of issues have been fixed related to fflush since the
first version, this test may partially be redundant.
Fred.
Changes since v5:
- Removed XFAIL as failure has been fixed (but for real).
Changes since v4:
- Rebased on master
- Removed XFAIL as failure has been fixed.
Changes since v3:
- Inverted the logic to select `fflush (NULL)` vs `fflush (FILE)`, the former
is now 0 and the later 1.
- Naming the file tracking structure, and using it to initialize the table.
- Removed the return value from `file_changed` since it was barely used. The
test status exit check is therefore moved inside the function.
- Don't cast xmmap return value.
Changes since v2:
- Removed ungetc as it doesn't bring any benefit to the current test, this
needs to be addressed specifically.
- Don't exit on each test, but rather on the first test file that fails.
- Directly use macros in the file_flush function instead of passing it as a
parameter
- Removed unnecessary test after xfopen
- Removed FILE_FLUSH_TYPE and S_FLUSH_TYPE definition from the skeleton
- Re-indent the file
Changes since v1:
- Add a test specifically for a stream opened with read mode.
- Split test in 2, and mark the `fflush (NULL)` as XFAIL (see comment in the
patch).
-- 8< --
Subject: [PATCH] libio: Add test case for fflush
Since one path uses _IO_SYNC and the other _IO_OVERFLOW, the newly added
test cases verifies that `fflush (FILE)` and `fflush (NULL)` are
semantically equivalent from the FILE perspective.
---
libio/Makefile | 2 +
libio/tst-fflush-NULL.c | 24 ++++++
libio/tst-fflush-skeleton.c | 166 ++++++++++++++++++++++++++++++++++++
libio/tst-fflush.c | 24 ++++++
4 files changed, 216 insertions(+)
create mode 100644 libio/tst-fflush-NULL.c
create mode 100644 libio/tst-fflush-skeleton.c
create mode 100644 libio/tst-fflush.c
Comments
On Fri, 4 Apr 2025, Frédéric Bérat wrote:
> +struct file_tracking
> +{
> + FILE *file;
> + char *name;
> + int fd;
> + char *mfile;
> +} files[TEST_FILE_COUNT];
> +
> +static void
> +file_init (int file)
> +{
> + int fd = -1;
> +
> + files[file] = (struct file_tracking) { .fd = -1, };
> +
> + if (file >= TEST_FILE_COUNT)
> + return;
If this condition is true, a buffer overrun has already occurred. So the
check should be moved before the assignment (and it should probably become
an assertion, unless the function is actually meant to get called with
file >= TEST_FILE_COUNT).
> + for (int i = 0; i < TEST_FILE_COUNT; i++)
> + {
> + if (mode[0] == 'r')
> + {
> + fgetc (files[i].file);
> + }
> + else
> + {
> + fprintf (files[i].file, "%d:%d", FILE_FLUSH_TYPE, i);
> + }
We generally avoid braces around single expression statements like here
(and probably also around the single if inside for).
> + if (!FILE_FLUSH_TYPE)
> + {
> + TEST_VERIFY (fflush (NULL) == 0);
> + }
Likewise.
@@ -101,6 +101,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 \
new file mode 100644
@@ -0,0 +1,24 @@
+/* Test that fflush (FILE) and fflush (NULL) are semantically equivalent.
+ This is the `fflush (NULL)` part.
+
+ Copyright (C) 2024 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#define FILE_FLUSH_TYPE 0
+#define S_FLUSH_TYPE "NULL"
+
+#include "tst-fflush-skeleton.c"
new file mode 100644
@@ -0,0 +1,166 @@
+/* Test that fflush (FILE) and fflush (NULL) are semantically equivalent.
+
+ Copyright (C) 2024 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+/* A success on this test doesn't imply the effectiveness of fflush as
+ we can't ensure that the file wasn't already in the expected state
+ before the call of the function. It only ensures that, if the test
+ fails, fflush is broken. */
+
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <sys/stat.h>
+#include <sys/mman.h>
+
+#include <support/check.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/test-driver.h>
+#include <support/xstdio.h>
+#include <support/xunistd.h>
+
+#define CONTENT_SZ_MAX 32
+#define TEST_FILE_COUNT 10
+
+struct file_tracking
+{
+ FILE *file;
+ char *name;
+ int fd;
+ char *mfile;
+} files[TEST_FILE_COUNT];
+
+static void
+file_init (int file)
+{
+ int fd = -1;
+
+ files[file] = (struct file_tracking) { .fd = -1, };
+
+ 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 = 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);
+ files[file] = (struct file_tracking) { .fd = -1, };
+}
+
+static void
+file_changed (int to_check, const char *mode)
+{
+ struct stat stats = { };
+ 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", FILE_FLUSH_TYPE, to_check);
+ TEST_COMPARE_BLOB (files[to_check].mfile, sizeof (expected),
+ expected, sizeof (expected));
+
+ TEST_VERIFY (fstat (files[to_check].fd, &stats) >= 0);
+ TEST_VERIFY (stats.st_size == 3);
+ /* In read mode we expect to be at position 1, in write mode at position 3 */
+ TEST_COMPARE (lseek (files[to_check].fd, 0, SEEK_CUR),
+ mode[0] == 'r' ? 1 : 3);
+
+ if (support_record_failure_is_failed ())
+ FAIL_EXIT1 ("exiting due to previous failure");
+
+ /* Not reached if the data doesn't match. */
+}
+
+static void
+file_flush (const char *mode)
+{
+ for (int i = 0; i < TEST_FILE_COUNT; i++)
+ {
+ files[i].file = xfopen (files[i].name, mode);
+ 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++)
+ {
+ if (mode[0] == 'r')
+ {
+ fgetc (files[i].file);
+ }
+ else
+ {
+ fprintf (files[i].file, "%d:%d", FILE_FLUSH_TYPE, i);
+ }
+ }
+
+ if (!FILE_FLUSH_TYPE)
+ {
+ TEST_VERIFY (fflush (NULL) == 0);
+ }
+ else
+ {
+ for (int i = 0; i < TEST_FILE_COUNT; i++)
+ TEST_VERIFY (fflush (files[i].file) == 0);
+ }
+
+ for (int i = 0; i < TEST_FILE_COUNT; i++)
+ {
+ verbose_printf ("Check that file %s has been modified after fflush\n",
+ files[i].name);
+ file_changed (i, mode);
+ }
+
+ for (int i = 0; i < TEST_FILE_COUNT; i++)
+ xfclose (files[i].file);
+}
+
+static int
+do_test (void)
+{
+ for (int i = 0; i < TEST_FILE_COUNT; i++)
+ file_init (i);
+
+ verbose_printf ("Checking fflush(" S_FLUSH_TYPE "), WRITE mode\n");
+ file_flush ("w");
+
+ verbose_printf ("Checking fflush(" S_FLUSH_TYPE "), READWRITE mode\n");
+ file_flush ("r+");
+
+ for (int i = 0; i < TEST_FILE_COUNT; i++)
+ file_cleanup (i);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
new file mode 100644
@@ -0,0 +1,24 @@
+/* Test that fflush (FILE) and fflush (NULL) are semantically equivalent.
+ This is the `fflush (FILE)` part.
+
+ Copyright (C) 2024 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#define FILE_FLUSH_TYPE 1
+#define S_FLUSH_TYPE "FILE"
+
+#include "tst-fflush-skeleton.c"