diff --git a/configure.ac b/configure.ac index 755e4a75e3..d5e297130c 100644 --- a/configure.ac +++ b/configure.ac @@ -735,6 +735,7 @@ tests/src/vfs/extfs/helpers-list/Makefile tests/src/vfs/extfs/helpers-list/data/config.sh tests/src/vfs/extfs/helpers-list/misc/Makefile tests/src/vfs/ftpfs/Makefile +tests/src/vfs/local/Makefile ]) AC_OUTPUT diff --git a/src/vfs/local/local.c b/src/vfs/local/local.c index f2fe46f1cc..31f209c42d 100644 --- a/src/vfs/local/local.c +++ b/src/vfs/local/local.c @@ -93,9 +93,10 @@ local_opendir (const vfs_path_t *vpath) * networked filesystems such as CIFS. Networked FUSE-based systems appear to be affected as * well. What's worse is that when you retry `readdir`, the file list still comes out empty. * - * So our only option seems to be to try `readdir` immediately after `opendir` and use - * `rewinddir` if successful, otherwise reopen the directory, which usually leads to correct - * file listing. + * So our only option seems to be to try `readdir` immediately after `opendir` and reopen the + * directory if it failed with EINTR, which usually leads to correct file listing, or use + * `rewinddir` otherwise, even if `readdir` failed, since FreeBSD and macOS may have advanced + * the stream anyway. * * However, this has caused problems on FUSE-based systems that do not properly implement * `rewinddir` in the past. No silver bullet... @@ -104,7 +105,16 @@ local_opendir (const vfs_path_t *vpath) { dir = opendir (path); if (dir == NULL) + { + // Unlike readdir(), opendir() always sets errno on failure, so no reset is needed + if (errno == EINTR) + continue; + return NULL; + } + + // Reset errno before readdir to avoid reading a stale EINTR (#5156) + errno = 0; if (readdir (dir) == NULL && errno == EINTR) { @@ -129,7 +139,15 @@ local_readdir (void *data) struct dirent *d; unsigned char type; - d = readdir (*(DIR **) data); + // Retry if interrupted, resetting errno to avoid reading a stale EINTR (#5156). FreeBSD and + // macOS may lose the entries read before the interruption, but rewinding would repeat the + // ones already returned. + do + { + errno = 0; + d = readdir (*(DIR **) data); + } + while (d == NULL && errno == EINTR); if (d == NULL) return NULL; diff --git a/tests/.gitignore b/tests/.gitignore index 53aac0c4be..67122ff6ff 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -178,4 +178,5 @@ src/vfs/extfs/helpers-list/run.log src/vfs/extfs/helpers-list/run.trs src/vfs/extfs/helpers-list/test-suite.log src/vfs/ftpfs/ftpfs_parse_long_list +src/vfs/local/local_opendir *.trs diff --git a/tests/src/vfs/Makefile.am b/tests/src/vfs/Makefile.am index 8530f99bf8..d0c9f27025 100644 --- a/tests/src/vfs/Makefile.am +++ b/tests/src/vfs/Makefile.am @@ -1,5 +1,5 @@ -SUBDIRS = +SUBDIRS = local if ENABLE_VFS_EXTFS SUBDIRS += extfs diff --git a/tests/src/vfs/local/Makefile.am b/tests/src/vfs/local/Makefile.am new file mode 100644 index 0000000000..996357d55d --- /dev/null +++ b/tests/src/vfs/local/Makefile.am @@ -0,0 +1,22 @@ +PACKAGE_STRING = "/src/vfs/local" + +AM_CPPFLAGS = \ + $(GLIB_CFLAGS) \ + -I$(top_srcdir) \ + @CHECK_CFLAGS@ + +LIBS = @CHECK_LIBS@ \ + $(top_builddir)/src/libinternal.la \ + $(top_builddir)/lib/libmc.la + +if ENABLE_MCLIB +LIBS += $(GLIB_LIBS) +endif + +TESTS = \ + local_opendir + +check_PROGRAMS = $(TESTS) + +local_opendir_SOURCES = \ + local_opendir.c diff --git a/tests/src/vfs/local/local_opendir.c b/tests/src/vfs/local/local_opendir.c new file mode 100644 index 0000000000..5be37ebd41 --- /dev/null +++ b/tests/src/vfs/local/local_opendir.c @@ -0,0 +1,301 @@ +/* + src/vfs/local - tests for local_opendir() and local_readdir() functions + + Copyright (C) 2026 + Free Software Foundation, Inc. + + Written by: + Yury V. Zaytsev , 2026 + + This file is part of the Midnight Commander. + + The Midnight Commander is free software: you can redistribute it + and/or modify it under the terms of the GNU General Public License as + published by the Free Software Foundation, either version 3 of the License, + or (at your option) any later version. + + The Midnight Commander 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 General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . + */ + +#define TEST_SUITE_NAME "/src/vfs/local" + +#include "tests/mctest.h" + +#include +#include // rmdir() + +#include "lib/strutil.h" + +/* --------------------------------------------------------------------------------------------- */ +/* mocked functions */ +/* --------------------------------------------------------------------------------------------- */ + +static DIR *mock_opendir (const char *name); +static struct dirent *mock_readdir (DIR *dirp); + +// route the opendir() and readdir() calls of the local VFS through the mocks +#define opendir(name) mock_opendir (name) +#define readdir(dirp) mock_readdir (dirp) +#include "src/vfs/local/local.c" +#undef opendir +#undef readdir + +// calls to fail with EINTR before passing through +static int mock_opendir__eintr_count = 0; + +/* @Mock */ +static DIR * +mock_opendir (const char *name) +{ + if (mock_opendir__eintr_count > 0) + { + mock_opendir__eintr_count--; + errno = EINTR; + return NULL; + } + + return opendir (name); +} + +// calls to fail with EINTR before passing through +static int mock_readdir__eintr_count = 0; + +// whether to report the end of the directory right away +static gboolean mock_readdir__eof = FALSE; + +// entries to consume before failing with EIO, or -1 to pass through +static int mock_readdir__fail_after = -1; + +/* @Mock */ +static struct dirent * +mock_readdir (DIR *dirp) +{ + if (mock_readdir__eintr_count > 0) + { + mock_readdir__eintr_count--; + errno = EINTR; + return NULL; + } + + if (mock_readdir__eof) + return NULL; // like readdir(), leaving errno untouched + + if (mock_readdir__fail_after < 0) + return readdir (dirp); + + // FreeBSD and macOS keep the advanced offset if reading fails after partial progress + for (; mock_readdir__fail_after > 0; mock_readdir__fail_after--) + (void) readdir (dirp); + mock_readdir__fail_after = -1; + + errno = EIO; + return NULL; +} + +/* --------------------------------------------------------------------------------------------- */ + +/* @Before */ +static void +setup (void) +{ + str_init_strings (NULL); + + vfs_init (); + vfs_init_localfs (); +} + +/* --------------------------------------------------------------------------------------------- */ + +/* @After */ +static void +teardown (void) +{ + vfs_shut (); + str_uninit_strings (); +} + +/* --------------------------------------------------------------------------------------------- */ + +static int +count_entries (const vfs_path_t *vpath) +{ + void *info; + struct vfs_dirent *entry; + int count = 0; + + info = local_opendir (vpath); + mctest_assert_not_null (info); + + while ((entry = local_readdir (info)) != NULL) + { + vfs_dirent_free (entry); + count++; + } + + local_closedir (info); + + return count; +} + +/* --------------------------------------------------------------------------------------------- */ + +/* @Test */ +START_TEST (test_local_opendir_stale_eintr) +{ + char *dir; + vfs_path_t *vpath; + void *info; + + // given: a directory without any entries, not even "." and ".." (as on some FUSE filesystems), + // and errno left at EINTR by an earlier interrupted syscall (#5156) + dir = g_dir_make_tmp ("mctest-XXXXXX", NULL); + mctest_assert_not_null (dir); + vpath = vfs_path_from_str (dir); + mock_readdir__eof = TRUE; + errno = EINTR; + + // when + info = local_opendir (vpath); + + // then: the directory is opened rather than reopened forever + mctest_assert_not_null (info); + + // cleanup + local_closedir (info); + mock_readdir__eof = FALSE; + vfs_path_free (vpath, TRUE); + rmdir (dir); + g_free (dir); +} +END_TEST + +/* --------------------------------------------------------------------------------------------- */ + +/* @Test */ +START_TEST (test_local_opendir_readdir_error) +{ + char *dir; + vfs_path_t *vpath; + int expected; + int actual; + + // given: a directory with entries ("." and ".."), the first readdir() of which fails with an + // error other than EINTR after consuming one of them + dir = g_dir_make_tmp ("mctest-XXXXXX", NULL); + mctest_assert_not_null (dir); + vpath = vfs_path_from_str (dir); + expected = count_entries (vpath); + ck_assert_int_gt (expected, 0); + mock_readdir__fail_after = 1; + + // when + actual = count_entries (vpath); + + // then: the stream is rewound, so that no entry is lost + ck_assert_int_eq (actual, expected); + + // cleanup + vfs_path_free (vpath, TRUE); + rmdir (dir); + g_free (dir); +} +END_TEST + +/* --------------------------------------------------------------------------------------------- */ + +/* @Test */ +START_TEST (test_local_opendir_opendir_eintr) +{ + char *dir; + vfs_path_t *vpath; + void *info; + + // given: a directory, the opening of which is interrupted once + dir = g_dir_make_tmp ("mctest-XXXXXX", NULL); + mctest_assert_not_null (dir); + vpath = vfs_path_from_str (dir); + mock_opendir__eintr_count = 1; + + // when + info = local_opendir (vpath); + + // then: opening is retried + mctest_assert_not_null (info); + + // cleanup + local_closedir (info); + vfs_path_free (vpath, TRUE); + rmdir (dir); + g_free (dir); +} +END_TEST + +/* --------------------------------------------------------------------------------------------- */ + +/* @Test */ +START_TEST (test_local_readdir_eintr) +{ + char *dir; + vfs_path_t *vpath; + void *info; + struct vfs_dirent *entry; + int expected; + int actual; + + // given: a directory with entries ("." and ".."), the listing of which is interrupted once + // after the first of them + dir = g_dir_make_tmp ("mctest-XXXXXX", NULL); + mctest_assert_not_null (dir); + vpath = vfs_path_from_str (dir); + expected = count_entries (vpath); + ck_assert_int_gt (expected, 1); + info = local_opendir (vpath); + mctest_assert_not_null (info); + entry = local_readdir (info); + mctest_assert_not_null (entry); + vfs_dirent_free (entry); + mock_readdir__eintr_count = 1; + + // when + for (actual = 1; (entry = local_readdir (info)) != NULL; actual++) + vfs_dirent_free (entry); + + // then: reading is retried, so that the listing is complete + ck_assert_int_eq (actual, expected); + + // cleanup + local_closedir (info); + vfs_path_free (vpath, TRUE); + rmdir (dir); + g_free (dir); +} +END_TEST + +/* --------------------------------------------------------------------------------------------- */ + +int +main (void) +{ + TCase *tc_core; + + tc_core = tcase_create ("Core"); + + tcase_add_checked_fixture (tc_core, setup, teardown); + + // Add new tests here: *************** + tcase_add_test (tc_core, test_local_opendir_stale_eintr); + tcase_add_test (tc_core, test_local_opendir_readdir_error); + tcase_add_test (tc_core, test_local_opendir_opendir_eintr); + tcase_add_test (tc_core, test_local_readdir_eintr); + // *********************************** + + return mctest_run_all (tc_core); +} + +/* --------------------------------------------------------------------------------------------- */