* [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers
@ 2025-04-07 19:14 Gyorgy Sarvari
2025-04-07 19:14 ` [pseudo][PATCH v2 1/2] nftw, ftw: add wrapper Gyorgy Sarvari
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Gyorgy Sarvari @ 2025-04-07 19:14 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock
This is an (attempt at the) implementation of the nftw and ftw[1] calls in pseudo.
The main motivation is a change in btrfs-tools[2], in which
they have changed from walking the filetree and calling stat
on each entries separately to using the nftw call. As a result,
btrfs filesystem generation currently happens with incorrect
ownership details, as the nftw calls go around pseudo.
See also the relevant report[3] on the Yocto mailing list.
The main idea: with a custom wrapper capture the nftw call, and switch the callback
to our own callback. When our own callback is called, fix the received
stat struct, and call the original callback, this time with the correct
stat struct.
Big thanks to Lander Van Loock, who not only reported the
original issue, but also helped testing and reviewing the first version
of the code.
Please let me know what you think.
[1]: https://linux.die.net/man/3/nftw
[2]: https://github.com/kdave/btrfs-progs/commit/c6464d3f99ed1dabceff1168eabb207492c37624
[3]: https://lists.yoctoproject.org/g/yocto/message/64889
---
v1: https://lore.kernel.org/yocto-patches/20250317113445.3855518-1-skandigraun@gmail.com/T/
changes in v2:
- Add wrapper for ftw, ftw64, nftw and nftw64 (instead of only nftw in v1). All of them
use the same wrapper file, using macros to account for the naming differences.
- ftw64 and nftw64 depend on large file support of the system. To account for this, move
these implementations into their own subport folder, and compile them conditionally.
- Move tests into separate commit (and add some tests for the new calls too)
- The original implementation didn't consider multiple concurrent requests
when it was saving the original call's details (callback function pointer,
flags), and subsequent calls could overwrite data. This version stores
these details in an array that behaves similar to a LIFO proto-stack.
- Fix one bug in FTW_CHDIR flag behavior in conjuction with FTW_DEPTH flag (it switched
to a folder, which didn't match glibc implementation's behavior)
- Minor change in Makefile: since the tests between ftw/ftw64 and nftw/nftw64 are exactly
the same, they are also implemented in the same file, which however is expected to be
included in other files, and is not a compile-unit on its own. For this, the Makefile
looks for files with "test-" prefix in the test folder.
---
Gyorgy Sarvari (2):
nftw, ftw: add wrapper
nftw, ftw: add tests
Makefile.in | 4 +-
guts/README | 6 +-
ports/linux/guts/ftw64.c | 16 --
ports/linux/nftw64/guts/ftw64.c | 29 +++
ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
ports/linux/nftw64/pseudo_wrappers.c | 45 +++++
ports/linux/nftw64/wrapfuncs.in | 2 +
ports/linux/subports | 14 ++
ports/linux/wrapfuncs.in | 2 -
ports/unix/guts/ftw.c | 13 +-
ports/unix/guts/nftw.c | 7 +-
ports/unix/guts/nftw_wrapper_base.c | 211 ++++++++++++++++++++++
ports/unix/pseudo_wrappers.c | 45 +++++
ports/unix/wrapfuncs.in | 2 +-
test/ftw-test-impl.c | 226 +++++++++++++++++++++++
test/nftw-test-impl.c | 236 +++++++++++++++++++++++++
test/test-ftw.c | 4 +
test/test-ftw64.c | 4 +
test/test-nftw.c | 4 +
test/test-nftw.sh | 84 +++++++++
test/test-nftw64.c | 4 +
21 files changed, 937 insertions(+), 28 deletions(-)
delete mode 100644 ports/linux/guts/ftw64.c
create mode 100644 ports/linux/nftw64/guts/ftw64.c
rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
create mode 100644 ports/linux/nftw64/wrapfuncs.in
create mode 100644 ports/unix/guts/nftw_wrapper_base.c
create mode 100644 test/ftw-test-impl.c
create mode 100644 test/nftw-test-impl.c
create mode 100644 test/test-ftw.c
create mode 100644 test/test-ftw64.c
create mode 100644 test/test-nftw.c
create mode 100755 test/test-nftw.sh
create mode 100644 test/test-nftw64.c
^ permalink raw reply [flat|nested] 13+ messages in thread
* [pseudo][PATCH v2 1/2] nftw, ftw: add wrapper
2025-04-07 19:14 [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers Gyorgy Sarvari
@ 2025-04-07 19:14 ` Gyorgy Sarvari
2025-04-07 19:14 ` [pseudo][PATCH v2 2/2] nftw, ftw: add tests Gyorgy Sarvari
` (3 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Gyorgy Sarvari @ 2025-04-07 19:14 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock
Add a wrapper for nftw and ftw[1] calls (along with nftw64 and ftw64).
The call in brief: it accepts a path, which it walks. For every
entries it finds, it calls a user-specified callback function, and
passes some information about the entry to this callback.
The implementation saves the callback from the nftw call, and subtitutes
it with its own "fake_callback". When the real nftw calls the fake_callback,
it corrects the stat struct it received with information queried from pseudo.
Afterwards it calls the original callback and passes the now corrected
information to it.
The 4 functions are very similar to each other: nftw-nftw64 and ftw-ftw64 are
identical to their pair, except for the stat struct they use (stat vs stat64).
nftw is a "superset" of ftw: nftw is backwards compatible with ftw, but it
also accepts a number of extra flags to modify its behavior.
Since all the 4 functions are so similar, the same codebase is used to
implement all (which is also fairly similar to their implementation in
glibc).
[1]: https://linux.die.net/man/3/nftw
Signed-off-by: Gyorgy Sarvari <skandigraun@gmail.com>
---
guts/README | 6 +-
ports/linux/guts/ftw64.c | 16 --
ports/linux/nftw64/guts/ftw64.c | 29 ++++
ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
ports/linux/nftw64/pseudo_wrappers.c | 45 ++++++
ports/linux/nftw64/wrapfuncs.in | 2 +
ports/linux/subports | 14 ++
ports/linux/wrapfuncs.in | 2 -
ports/unix/guts/ftw.c | 13 +-
ports/unix/guts/nftw.c | 7 +-
ports/unix/guts/nftw_wrapper_base.c | 211 +++++++++++++++++++++++++
ports/unix/pseudo_wrappers.c | 45 ++++++
ports/unix/wrapfuncs.in | 2 +-
13 files changed, 373 insertions(+), 26 deletions(-)
delete mode 100644 ports/linux/guts/ftw64.c
create mode 100644 ports/linux/nftw64/guts/ftw64.c
rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
create mode 100644 ports/linux/nftw64/wrapfuncs.in
create mode 100644 ports/unix/guts/nftw_wrapper_base.c
diff --git a/guts/README b/guts/README
index 0a1fe5f..5bcc198 100644
--- a/guts/README
+++ b/guts/README
@@ -54,6 +54,8 @@ other functions:
__xmknod: __xmknodat
__xstat64: __fxstatat64
__xstat: __fxstatat
+ ftw: nftw
+ ftw64: nftw64
The following functions are full implementations:
@@ -129,15 +131,11 @@ calling the underlying routine.
eaccess
euidaccess
fts_open
- ftw64
- ftw
glob64
glob
lutimes
mkdtemp
mktemp
- nftw64
- nftw
opendir
pathconf
readlinkat
diff --git a/ports/linux/guts/ftw64.c b/ports/linux/guts/ftw64.c
deleted file mode 100644
index 48adb80..0000000
--- a/ports/linux/guts/ftw64.c
+++ /dev/null
@@ -1,16 +0,0 @@
-/*
- * Copyright (c) 2010 Wind River Systems; see
- * guts/COPYRIGHT for information.
- *
- * SPDX-License-Identifier: LGPL-2.1-only
- *
- * static int
- * wrap_ftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int), int nopenfd) {
- * int rc = -1;
- */
-
- rc = real_ftw64(path, fn, nopenfd);
-
-/* return rc;
- * }
- */
diff --git a/ports/linux/nftw64/guts/ftw64.c b/ports/linux/nftw64/guts/ftw64.c
new file mode 100644
index 0000000..1fe0868
--- /dev/null
+++ b/ports/linux/nftw64/guts/ftw64.c
@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2010 Wind River Systems; see
+ * guts/COPYRIGHT for information.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-only
+ *
+ * static int
+ * wrap_ftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int), int nopenfd) {
+ * int rc = -1;
+ */
+ typedef int (*pseudo_nftw64_func_t) (const char *filename,
+ const struct stat64 *status, int flag,
+ struct FTW *info);
+
+ // 1. Set the flag argument to 0, just like glibc does.
+ // 2. The difference between ftw and nftw callback
+ // is only the last parameter: struct FTW is only used
+ // by nftw(), and it is missing from ftw().
+ // However since otherwise the stacklayout for the
+ // functions is the same, this cast should work just the
+ // way we want it. This is also borrowed from glibc.
+
+ // This, of course in turn will be captured by pseudo, and will be passed
+ // to the respective wrapper of nftw64()
+ rc = nftw64(path, (pseudo_nftw64_func_t)fn, nopenfd, 0);
+
+/* return rc;
+ * }
+ */
diff --git a/ports/linux/guts/nftw64.c b/ports/linux/nftw64/guts/nftw64.c
similarity index 57%
rename from ports/linux/guts/nftw64.c
rename to ports/linux/nftw64/guts/nftw64.c
index 816faba..0dac9d3 100644
--- a/ports/linux/guts/nftw64.c
+++ b/ports/linux/nftw64/guts/nftw64.c
@@ -9,7 +9,12 @@
* int rc = -1;
*/
- rc = real_nftw64(path, fn, nopenfd, flag);
+/***********************************
+ * This call has a custom wrapper
+ * where all the logic happens just
+ * around this single line.
+ ***********************************/
+ rc = real_nftw64(path, NFTW_CALLBACK_NAME, nopenfd, flag);
/* return rc;
* }
diff --git a/ports/linux/nftw64/pseudo_wrappers.c b/ports/linux/nftw64/pseudo_wrappers.c
new file mode 100644
index 0000000..2bd8227
--- /dev/null
+++ b/ports/linux/nftw64/pseudo_wrappers.c
@@ -0,0 +1,45 @@
+#undef NFTW_NAME
+#undef NFTW_WRAP_NAME
+#undef NFTW_REAL_NAME
+#undef NFTW_STAT_STRUCT
+#undef NFTW_STAT_NAME
+#undef NFTW_LSTAT_NAME
+#undef NFTW_GUTS_INCLUDE
+#undef NFTW_CALLBACK_NAME
+#undef NFTW_STORAGE_STRUCT_NAME
+#undef NFTW_STORAGE_ARRAY_SIZE
+#undef NFTW_MUTEX_NAME
+#undef NFTW_STORAGE_ARRAY_NAME
+#undef NFTW_APPEND_FN_NAME
+#undef NFTW_DELETE_FN_NAME
+#undef NFTW_FIND_FN_NAME
+
+#define CONCAT_EXPANDED(prefix, value) prefix ## value
+#define CONCAT(prefix, value) CONCAT_EXPANDED(prefix, value)
+
+#define NFTW_NAME nftw64
+#define NFTW_WRAP_NAME CONCAT(wrap_, NFTW_NAME)
+#define NFTW_REAL_NAME CONCAT(real_, NFTW_NAME)
+#define NFTW_STAT_STRUCT stat64
+#define NFTW_STAT_NAME stat64
+#define NFTW_LSTAT_NAME lstat64
+
+// this file is in the same directory as this wrapper, but
+// this path is relative to the nftw_wrapper_base file, where
+// it gets included.
+#define NFTW_GUTS_INCLUDE "../../linux/nftw64/guts/nftw64.c"
+#define NFTW_CALLBACK_NAME CONCAT(wrap_callback_, NFTW_NAME)
+#define NFTW_STORAGE_STRUCT_NAME CONCAT(storage_struct_, NFTW_NAME)
+#define NFTW_STORAGE_ARRAY_SIZE CONCAT(storage_size_, NFTW_NAME)
+#define NFTW_MUTEX_NAME CONCAT(mutex_, NFTW_NAME)
+#define NFTW_STORAGE_ARRAY_NAME CONCAT(storage_array_, NFTW_NAME)
+#define NFTW_APPEND_FN_NAME CONCAT(append_to_array_, NFTW_NAME)
+#define NFTW_DELETE_FN_NAME CONCAT(delete_from_array_, NFTW_NAME)
+#define NFTW_FIND_FN_NAME CONCAT(find_in_array_, NFTW_NAME)
+
+// nftw64() is identical to nftw() in all regards, except
+// that it uses "stat64" structs instead of "stat", so
+// the whole codebase can be reused, accounting for naming
+// and type changes using the macros above.
+
+#include "../../unix/guts/nftw_wrapper_base.c"
diff --git a/ports/linux/nftw64/wrapfuncs.in b/ports/linux/nftw64/wrapfuncs.in
new file mode 100644
index 0000000..3e2ed67
--- /dev/null
+++ b/ports/linux/nftw64/wrapfuncs.in
@@ -0,0 +1,2 @@
+int nftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int, struct FTW *), int nopenfd, int flag); /* noignore_path=1, hand_wrapped=1 */
+int ftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int), int nopenfd);
diff --git a/ports/linux/subports b/ports/linux/subports
index 099ea59..b55bea9 100755
--- a/ports/linux/subports
+++ b/ports/linux/subports
@@ -70,3 +70,17 @@ else
fi
rm -f dummy.c dummy.o
+# nftw64 (and its deprecated pair, ftw64) are only present in case large
+# file support is present.
+cat > dummy.c <<EOF
+#define _GNU_SOURCE
+#include <features.h>
+#ifndef __USE_LARGEFILE64
+#error "no large file support"
+#endif
+int i;
+EOF
+if ${CC} -c -o dummy.o dummy.c >/dev/null 2>&1; then
+ echo linux/nftw64
+fi
+rm -f dummy.c dummy.o
diff --git a/ports/linux/wrapfuncs.in b/ports/linux/wrapfuncs.in
index 60ce5f5..c4b06ae 100644
--- a/ports/linux/wrapfuncs.in
+++ b/ports/linux/wrapfuncs.in
@@ -34,9 +34,7 @@ int __lxstat64(int ver, const char *path, struct stat64 *buf); /* flags=AT_SYMLI
int __fxstat64(int ver, int fd, struct stat64 *buf);
int __fxstatat64(int ver, int dirfd, const char *path, struct stat64 *buf, int flags);
FILE *fopen64(const char *path, const char *mode); /* noignore_path=1 */
-int nftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int, struct FTW *), int nopenfd, int flag); /* noignore_path=1 */
FILE *freopen64(const char *path, const char *mode, FILE *stream); /* noignore_path=1 */
-int ftw64(const char *path, int (*fn)(const char *, const struct stat64 *, int), int nopenfd);
int glob64(const char *pattern, int flags, int (*errfunc)(const char *, int), glob64_t *pglob);
int scandir64(const char *path, struct dirent64 ***namelist, int (*filter)(const struct dirent64 *), int (*compar)());
int truncate64(const char *path, off64_t length);
diff --git a/ports/unix/guts/ftw.c b/ports/unix/guts/ftw.c
index 58945a1..4b3f49f 100644
--- a/ports/unix/guts/ftw.c
+++ b/ports/unix/guts/ftw.c
@@ -9,7 +9,18 @@
* int rc = -1;
*/
- rc = real_ftw(path, fn, nopenfd);
+ typedef int (*pseudo_nftw_func_t) (const char *filename,
+ const struct stat *status, int flag,
+ struct FTW *info);
+
+ // 1. Set the flag argument to 0, just like glibc does.
+ // 2. The difference between ftw and nftw callback
+ // is only the last parameter: struct FTW is only used
+ // by nftw(), and it is missing from ftw().
+ // However since otherwise the stacklayout for the
+ // functions is the same, this cast should work just the
+ // way we want it. This is also borrowed from glibc.
+ rc = nftw(path, (pseudo_nftw_func_t)fn, nopenfd, 0);
/* return rc;
* }
diff --git a/ports/unix/guts/nftw.c b/ports/unix/guts/nftw.c
index dac3106..d22cd1c 100644
--- a/ports/unix/guts/nftw.c
+++ b/ports/unix/guts/nftw.c
@@ -9,7 +9,12 @@
* int rc = -1;
*/
- rc = real_nftw(path, fn, nopenfd, flag);
+/***********************************
+ * This call has a custom wrapper
+ * where all the logic happens just
+ * around this single line.
+ ***********************************/
+ rc = real_nftw(path, NFTW_CALLBACK_NAME, nopenfd, flag);
/* return rc;
* }
diff --git a/ports/unix/guts/nftw_wrapper_base.c b/ports/unix/guts/nftw_wrapper_base.c
new file mode 100644
index 0000000..7479b80
--- /dev/null
+++ b/ports/unix/guts/nftw_wrapper_base.c
@@ -0,0 +1,211 @@
+int
+NFTW_NAME(const char *path, int (*fn)(const char *, const struct NFTW_STAT_STRUCT *, int, struct FTW *), int nopenfd, int flag) {
+ sigset_t saved;
+
+ int rc = -1;
+
+ if (!pseudo_check_wrappers() || !NFTW_REAL_NAME) {
+ /* rc was initialized to the "failure" value */
+ pseudo_enosys("nftw");
+ return rc;
+ }
+
+ if (pseudo_disabled) {
+ rc = (*NFTW_REAL_NAME)(path, fn, nopenfd, flag);
+
+ return rc;
+ }
+
+ pseudo_debug(PDBGF_WRAPPER, "wrapper called: %s\n", __func__);
+ pseudo_sigblock(&saved);
+ pseudo_debug(PDBGF_WRAPPER | PDBGF_VERBOSE, "%s - signals blocked, obtaining lock\n", __func__);
+ if (pseudo_getlock()) {
+ errno = EBUSY;
+ sigprocmask(SIG_SETMASK, &saved, NULL);
+ pseudo_debug(PDBGF_WRAPPER, "%s failed to get lock, giving EBUSY.\n", __func__);
+ return -1;
+ }
+
+ int save_errno;
+ if (antimagic > 0) {
+ /* call the real syscall */
+ pseudo_debug(PDBGF_SYSCALL, "%s calling real syscall.\n", __func__);
+ rc = (*NFTW_REAL_NAME)(path, fn, nopenfd, flag);
+ } else {
+ path = pseudo_root_path(__func__, __LINE__, AT_FDCWD, path, 0);
+ if (pseudo_client_ignore_path(path)) {
+ /* call the real syscall */
+ pseudo_debug(PDBGF_SYSCALL, "%s ignored path, calling real syscall.\n", __func__);
+ rc = (*NFTW_REAL_NAME)(path, fn, nopenfd, flag);
+ } else {
+ /* exec*() use this to restore the sig mask */
+ pseudo_saved_sigmask = saved;
+ rc = NFTW_WRAP_NAME(path, fn, nopenfd, flag);
+ }
+ }
+
+ save_errno = errno;
+ pseudo_droplock();
+ sigprocmask(SIG_SETMASK, &saved, NULL);
+ pseudo_debug(PDBGF_WRAPPER | PDBGF_VERBOSE, "%s - yielded lock, restored signals\n", __func__);
+ pseudo_debug(PDBGF_WRAPPER, "wrapper completed: %s returns %d (errno: %d)\n", __func__, rc, save_errno);
+ errno = save_errno;
+ return rc;
+}
+
+struct NFTW_STORAGE_STRUCT_NAME {
+ int (*callback)(const char *, const struct NFTW_STAT_STRUCT *, int, struct FTW *);
+ int flags;
+ pthread_t tid;
+};
+
+static struct NFTW_STORAGE_STRUCT_NAME *NFTW_STORAGE_ARRAY_NAME;
+size_t NFTW_STORAGE_ARRAY_SIZE = 0;
+static pthread_mutex_t NFTW_MUTEX_NAME = PTHREAD_MUTEX_INITIALIZER;
+
+static void NFTW_APPEND_FN_NAME(struct NFTW_STORAGE_STRUCT_NAME *data_to_append){
+ NFTW_STORAGE_ARRAY_NAME = realloc(NFTW_STORAGE_ARRAY_NAME, ++NFTW_STORAGE_ARRAY_SIZE * sizeof(*data_to_append));
+ memcpy(&NFTW_STORAGE_ARRAY_NAME[NFTW_STORAGE_ARRAY_SIZE - 1], data_to_append, sizeof(*data_to_append));
+}
+
+int NFTW_FIND_FN_NAME(struct NFTW_STORAGE_STRUCT_NAME* target) {
+ pthread_t tid = pthread_self();
+
+ // return the last one, not the first
+ for (ssize_t i = NFTW_STORAGE_ARRAY_SIZE - 1; i >= 0; --i){
+ if (NFTW_STORAGE_ARRAY_NAME[i].tid == tid){
+ // need to dereference it, as next time this array
+ // may be realloc'd, making the original pointer
+ // invalid
+ *target = NFTW_STORAGE_ARRAY_NAME[i];
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+static void NFTW_DELETE_FN_NAME() {
+ pthread_t tid = pthread_self();
+
+ if (NFTW_STORAGE_ARRAY_SIZE == 1) {
+ if (NFTW_STORAGE_ARRAY_NAME[0].tid == tid) {
+ free(NFTW_STORAGE_ARRAY_NAME);
+ NFTW_STORAGE_ARRAY_NAME = NULL;
+ --NFTW_STORAGE_ARRAY_SIZE;
+ } else {
+ pseudo_diag("%s: Invalid callback storage content, can't find corresponding data", __func__);
+ }
+ return;
+ }
+
+ int found_idx = -1;
+ for (ssize_t i = NFTW_STORAGE_ARRAY_SIZE - 1; i >= 0; --i) {
+ if (NFTW_STORAGE_ARRAY_NAME[i].tid == tid) {
+ found_idx = i;
+ break;
+ }
+ }
+
+ if (found_idx == -1) {
+ pseudo_diag("%s: Invalid callback storage content, can't find corresponding data", __func__);
+ return;
+ }
+
+ // delete the item we just found
+ for (size_t i = found_idx + 1; i < NFTW_STORAGE_ARRAY_SIZE; ++i)
+ NFTW_STORAGE_ARRAY_NAME[i - 1] = NFTW_STORAGE_ARRAY_NAME[i];
+
+ NFTW_STORAGE_ARRAY_NAME = realloc(NFTW_STORAGE_ARRAY_NAME, --NFTW_STORAGE_ARRAY_SIZE * sizeof(struct NFTW_STORAGE_STRUCT_NAME));
+
+}
+
+static int NFTW_CALLBACK_NAME(const char* fpath, const struct NFTW_STAT_STRUCT *sb, int typeflag, struct FTW *ftwbuf) {
+ int orig_cwd_fd = -1;
+ char *orig_cwd = NULL;
+ char *target_dir = NULL;
+ struct NFTW_STORAGE_STRUCT_NAME saved_details;
+
+ if (!NFTW_FIND_FN_NAME(&saved_details)) {
+ pseudo_diag("%s: Could not find corresponding callback!", __func__);
+ return -1;
+ }
+
+ // This flag is handled by nftw, however the actual directory change happens
+ // outside of pseudo, so it doesn't have any effect. To mitigate this, handle
+ // it here also explicitly.
+ //
+ // This is very similar to what glibc is doing: keep an open FD for the
+ // current working directory, process the entry (determine the flags, etc),
+ // call the callback, and then switch back to the original folder - in the same
+ // process. Glibc doesn't seem to take any further thread-safety measures nor
+ // other special steps.
+ // Error checking is not done here, as if real_nftw couldn't perform it,
+ // then it has already returned an error, and this part isn't reached. This
+ // just repeats the successful steps.
+ //
+ // See io/ftw.c in glibc source
+ if (saved_details.flags & FTW_CHDIR) {
+ orig_cwd_fd = open(".", O_RDONLY | O_DIRECTORY);
+ if (orig_cwd_fd == -1) {
+ orig_cwd = getcwd(NULL, 0);
+ }
+
+ // If it is a folder that's content has been already walked with the
+ // FTW_DEPTH flag, then switch into this folder, instead of the parent of
+ // it. This matches the behavior of the real nftw in this special case.
+ // This seems to be undocumented - it was derived by observing this behavior.
+ if (typeflag == FTW_DP) {
+ chdir(fpath);
+ } else {
+ target_dir = malloc(ftwbuf->base + 1);
+ memset(target_dir, 0, ftwbuf->base + 1);
+ strncpy(target_dir, fpath, ftwbuf->base);
+ chdir(target_dir);
+ }
+ }
+
+ // This is the main point of this call. Instead of the stat that
+ // came from real_nftw, use the stat returned by pseudo.
+ // If the target can't be stat'd (DNR), then just forward whatever
+ // is inside - no information can be retrieved of it anyway.
+ if (typeflag != FTW_DNR) {
+ (saved_details.flags & FTW_PHYS) ? NFTW_LSTAT_NAME(fpath, sb) : NFTW_STAT_NAME(fpath, sb);
+ }
+
+ int ret = saved_details.callback(fpath, sb, typeflag, ftwbuf);
+
+ if (saved_details.flags & FTW_CHDIR) {
+ if (orig_cwd_fd != -1) {
+ fchdir(orig_cwd_fd);
+ close(orig_cwd_fd);
+ } else if (orig_cwd != NULL) {
+ chdir(orig_cwd);
+ }
+ free(target_dir);
+ }
+
+ return ret;
+}
+
+static int
+NFTW_WRAP_NAME(const char *path, int (*fn)(const char *, const struct NFTW_STAT_STRUCT *, int, struct FTW *), int nopenfd, int flag) {
+ int rc = -1;
+
+ struct NFTW_STORAGE_STRUCT_NAME saved_details;
+
+ saved_details.tid = pthread_self();
+ saved_details.flags = flag;
+ saved_details.callback = fn;
+
+ pthread_mutex_lock(&NFTW_MUTEX_NAME);
+ NFTW_APPEND_FN_NAME(&saved_details);
+ pthread_mutex_unlock(&NFTW_MUTEX_NAME);
+
+#include NFTW_GUTS_INCLUDE
+
+ pthread_mutex_lock(&NFTW_MUTEX_NAME);
+ NFTW_DELETE_FN_NAME();
+ pthread_mutex_unlock(&NFTW_MUTEX_NAME);
+ return rc;
+}
diff --git a/ports/unix/pseudo_wrappers.c b/ports/unix/pseudo_wrappers.c
index bf69aa9..5f15930 100644
--- a/ports/unix/pseudo_wrappers.c
+++ b/ports/unix/pseudo_wrappers.c
@@ -2,6 +2,10 @@
* SPDX-License-Identifier: LGPL-2.1-only
*
*/
+
+/**********************************************
+ * POPEN
+ **********************************************/
FILE *
popen(const char *command, const char *mode) {
sigset_t saved;
@@ -52,3 +56,44 @@ wrap_popen(const char *command, const char *mode) {
return rc;
}
+
+/**********************************************
+ * NFTW
+ **********************************************/
+
+#define CONCAT_EXPANDED(prefix, value) prefix ## value
+#define CONCAT(prefix, value) CONCAT_EXPANDED(prefix, value)
+
+#undef NFTW_NAME
+#undef NFTW_WRAP_NAME
+#undef NFTW_REAL_NAME
+#undef NFTW_STAT_STRUCT
+#undef NFTW_STAT_NAME
+#undef NFTW_LSTAT_NAME
+#undef NFTW_GUTS_INCLUDE
+#undef NFTW_CALLBACK_NAME
+#undef NFTW_STORAGE_STRUCT_NAME
+#undef NFTW_STORAGE_ARRAY_SIZE
+#undef NFTW_MUTEX_NAME
+#undef NFTW_STORAGE_ARRAY_NAME
+#undef NFTW_APPEND_FN_NAME
+#undef NFTW_DELETE_FN_NAME
+#undef NFTW_FIND_FN_NAME
+
+#define NFTW_NAME nftw
+#define NFTW_WRAP_NAME CONCAT(wrap_, NFTW_NAME)
+#define NFTW_REAL_NAME CONCAT(real_, NFTW_NAME)
+#define NFTW_STAT_NAME stat
+#define NFTW_STAT_STRUCT stat
+#define NFTW_LSTAT_NAME lstat
+#define NFTW_CALLBACK_NAME CONCAT(wrap_callback_, NFTW_NAME)
+#define NFTW_GUTS_INCLUDE "nftw.c"
+#define NFTW_STORAGE_STRUCT_NAME CONCAT(storage_struct_, NFTW_NAME)
+#define NFTW_STORAGE_ARRAY_SIZE CONCAT(storage_size_, NFTW_NAME)
+#define NFTW_MUTEX_NAME CONCAT(mutex_, NFTW_NAME)
+#define NFTW_STORAGE_ARRAY_NAME CONCAT(storage_array_, NFTW_NAME)
+#define NFTW_APPEND_FN_NAME CONCAT(append_to_array_, NFTW_NAME)
+#define NFTW_DELETE_FN_NAME CONCAT(delete_from_array_, NFTW_NAME)
+#define NFTW_FIND_FN_NAME CONCAT(find_in_array_, NFTW_NAME)
+
+#include "guts/nftw_wrapper_base.c"
diff --git a/ports/unix/wrapfuncs.in b/ports/unix/wrapfuncs.in
index 7724fc7..5b1f557 100644
--- a/ports/unix/wrapfuncs.in
+++ b/ports/unix/wrapfuncs.in
@@ -14,7 +14,7 @@ int faccessat(int dirfd, const char *path, int mode, int flags);
int faccessat2(int dirfd, const char *path, int mode, int flags);
FTS *fts_open(char * const *path_argv, int options, int (*compar)(const FTSENT **, const FTSENT **)); /* inode64=1 */
int ftw(const char *path, int (*fn)(const char *, const struct stat *, int), int nopenfd);
-int nftw(const char *path, int (*fn)(const char *, const struct stat *, int, struct FTW *), int nopenfd, int flag);
+int nftw(const char *path, int (*fn)(const char *, const struct stat *, int, struct FTW *), int nopenfd, int flag); /* hand_wrapped=1 */
int glob(const char *pattern, int flags, int (*errfunc)(const char *, int), glob_t *pglob);
int lutimes(const char *path, const struct timeval *tv); /* flags=AT_SYMLINK_NOFOLLOW */
char *mkdtemp(char *template);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [pseudo][PATCH v2 2/2] nftw, ftw: add tests
2025-04-07 19:14 [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers Gyorgy Sarvari
2025-04-07 19:14 ` [pseudo][PATCH v2 1/2] nftw, ftw: add wrapper Gyorgy Sarvari
@ 2025-04-07 19:14 ` Gyorgy Sarvari
2025-04-07 23:11 ` [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers Mark Hatle
` (2 subsequent siblings)
4 siblings, 0 replies; 13+ messages in thread
From: Gyorgy Sarvari @ 2025-04-07 19:14 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock
Add tests for nftw, ftw, nftw64 and ftw64 calls.
Signed-off-by: Gyorgy Sarvari <skandigraun@gmail.com>
---
Makefile.in | 4 +-
test/ftw-test-impl.c | 226 ++++++++++++++++++++++++++++++++++++++++
test/nftw-test-impl.c | 236 ++++++++++++++++++++++++++++++++++++++++++
test/test-ftw.c | 4 +
test/test-ftw64.c | 4 +
test/test-nftw.c | 4 +
test/test-nftw.sh | 84 +++++++++++++++
test/test-nftw64.c | 4 +
8 files changed, 564 insertions(+), 2 deletions(-)
create mode 100644 test/ftw-test-impl.c
create mode 100644 test/nftw-test-impl.c
create mode 100644 test/test-ftw.c
create mode 100644 test/test-ftw64.c
create mode 100644 test/test-nftw.c
create mode 100755 test/test-nftw.sh
create mode 100644 test/test-nftw64.c
diff --git a/Makefile.in b/Makefile.in
index 48fdbd2..27da831 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -55,7 +55,7 @@ GUTS=$(filter-out "$(GLOB_PATTERN)",$(wildcard $(GLOB_PATTERN)))
SOURCES=$(wildcard *.c)
OBJS=$(subst .c,.o,$(SOURCES))
-TESTS=$(patsubst %.c,%,$(wildcard test/*.c))
+TESTS=$(patsubst %.c,%,$(wildcard test/test-*.c))
SHOBJS=pseudo_tables.o pseudo_util.o
DBOBJS=pseudo_db.o
@@ -78,7 +78,7 @@ all: $(LIBPSEUDO) $(PSEUDO) $(PSEUDODB) $(PSEUDOLOG) $(PSEUDO_PROFILE)
test: all $(TESTS) | $(BIN) $(LIB)
./run_tests.sh -v
-test/%: test/%.c
+test/test-%: test/test-%.c
$(CC) $(CFLAGS) $(CFLAGS_PSEUDO) -o $@ $<
install-lib: $(LIBPSEUDO)
diff --git a/test/ftw-test-impl.c b/test/ftw-test-impl.c
new file mode 100644
index 0000000..edd15eb
--- /dev/null
+++ b/test/ftw-test-impl.c
@@ -0,0 +1,226 @@
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <ftw.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#define LAST_VAL 999
+#define LAST_PATH "LAST_SENTINEL"
+
+#define TEST_WITH_PSEUDO 1
+#define TEST_WITHOUT_PSEUDO 0
+
+static int current_idx = 0;
+static int* current_responses;
+static char** expected_fpaths;
+
+static int pseudo_active;
+
+static unsigned int expected_gid;
+static unsigned int expected_uid;
+
+static int current_recursion_level = 0;
+static int max_recursion = 0;
+
+
+static int callback(const char* fpath, const struct FTW_STAT_STRUCT *sb, int typeflag){
+ if (current_recursion_level < max_recursion) {
+ ++current_recursion_level;
+ if (FTW_NAME("./walking/a1", callback, 10) != 0) {
+ printf("Recursive call failed\n");
+ exit(1);
+ }
+ }
+
+
+ int ret = current_responses[current_idx];
+ // printf("idx: %d, path: %s, ret: %d\n", current_idx, fpath, ret);
+
+ if (ret == LAST_VAL){
+ printf("Unexpected callback, it should have stopped already! fpath: %s\n", fpath);
+ return FTW_STOP;
+ }
+
+ char* expected_fpath_ending = expected_fpaths[current_idx];
+
+ if (strcmp(expected_fpath_ending, LAST_PATH) == 0){
+ printf("Unexpected fpath received: %s\n", fpath);
+ return FTW_STOP;
+ }
+
+ const char* actual_fpath_ending = fpath + strlen(fpath) - strlen(expected_fpath_ending);
+
+ if (strcmp(actual_fpath_ending, expected_fpath_ending) != 0){
+ printf("Incorrect fpath received. Expected: %s, actual: %s\n", expected_fpath_ending, actual_fpath_ending);
+ return FTW_STOP;
+ }
+
+ if (pseudo_active) {
+ if (sb->st_gid != 0 || sb->st_uid != 0) {
+ printf("Invalid uid/gid! Gid (act/exp): %d/%d, Uid (act/exp): %d/%d\n", sb->st_gid, 0, sb->st_uid, 0);
+ return FTW_STOP;
+ }
+ } else if (sb->st_gid != expected_gid || sb->st_uid != expected_uid) {
+ printf("Invalid uid/gid! Gid (act/exp): %d/%d, Uid (act/exp): %d/%d\n", sb->st_gid, expected_gid, sb->st_uid, expected_uid);
+ return FTW_STOP;
+ }
+
+ ++current_idx;
+ return ret;
+}
+
+static int run_test(int* responses, char** fpaths, int expected_retval, int with_pseudo) {
+ int ret;
+ current_responses = responses;
+ expected_fpaths = fpaths;
+ pseudo_active = with_pseudo;
+
+ ret = FTW_NAME("./walking", callback, 10);
+ current_responses = NULL;
+ expected_fpaths = NULL;
+
+ if (ret != expected_retval){
+ printf("Incorrect return value. Expected: %d, actual: %d\n", expected_retval, ret);
+ return 1;
+ }
+
+ if (responses[current_idx] != LAST_VAL){
+ printf("Not all expected paths were walked!\n");
+ return 1;
+ }
+ return 0;
+}
+
+/*
+ * This test just walks the whole test directory structure, and verifies that
+ * all expected files are returned.
+ */
+static int test_walking(int with_pseudo){
+ int responses[] = {FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, LAST_VAL};
+
+ char* fpaths[] = {"walking",
+ "walking/a1",
+ "walking/a1/b2",
+ "walking/a1/b2/file5",
+ "walking/a1/b2/file4",
+ "walking/a1/b1",
+ "walking/a1/b1/c1",
+ "walking/a1/b1/c1/file",
+ "walking/a1/b1/c1/file3",
+ "walking/a1/b1/c1/file2",
+ "walking/a1/b3",
+ LAST_PATH};
+
+ int expected_retval = 0;
+
+ return run_test(responses, fpaths, expected_retval, with_pseudo);
+}
+
+/*
+ * This test is very similar to test_walking(), but the callback at the
+ * start also calls ftw(), "max_recursion" times.
+ * It is trying to test pseudo's implementation of handling multiple
+ * concurrent (n)ftw calls in the same thread.
+ */
+static int test_walking_recursion(int with_pseudo){
+ max_recursion = 3;
+
+ int responses[] = {FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE,
+ FTW_CONTINUE, LAST_VAL};
+
+ char* fpaths[] = {"walking/a1",
+ "walking/a1/b2",
+ "walking/a1/b2/file5",
+ "walking/a1/b2/file4",
+ "walking/a1/b1",
+ "walking/a1/b1/c1",
+ "walking/a1/b1/c1/file",
+ "walking/a1/b1/c1/file3",
+ "walking/a1/b1/c1/file2",
+ "walking/a1/b3",
+ "walking/a1",
+ "walking/a1/b2",
+ "walking/a1/b2/file5",
+ "walking/a1/b2/file4",
+ "walking/a1/b1",
+ "walking/a1/b1/c1",
+ "walking/a1/b1/c1/file",
+ "walking/a1/b1/c1/file3",
+ "walking/a1/b1/c1/file2",
+ "walking/a1/b3",
+ "walking/a1",
+ "walking/a1/b2",
+ "walking/a1/b2/file5",
+ "walking/a1/b2/file4",
+ "walking/a1/b1",
+ "walking/a1/b1/c1",
+ "walking/a1/b1/c1/file",
+ "walking/a1/b1/c1/file3",
+ "walking/a1/b1/c1/file2",
+ "walking/a1/b3",
+ "walking",
+ "walking/a1",
+ "walking/a1/b2",
+ "walking/a1/b2/file5",
+ "walking/a1/b2/file4",
+ "walking/a1/b1",
+ "walking/a1/b1/c1",
+ "walking/a1/b1/c1/file",
+ "walking/a1/b1/c1/file3",
+ "walking/a1/b1/c1/file2",
+ "walking/a1/b3",
+ LAST_PATH};
+ int expected_retval = 0;
+
+ return run_test(responses, fpaths, expected_retval, with_pseudo);
+}
+
+/*
+ * Arguments:
+ * argv[1]: always the test name
+ * argv[2], argv[3]: in case the test name refers to a test without using
+ * pseudo (no_pseudo), then they should be the gid and uid
+ * of the current user. Otherwise these arguments are ignored.
+ *
+ * ftw64 call only exists on Linux in case __USE_LARGEFILE64 is defined.
+ * If this is not the case, just skip this test.
+ */
+int main(int argc, char* argv[])
+{
+#if !defined(__USE_LARGEFILE64) && FTW_NAME == ftw64
+return 0
+#endif
+ if (argc < 2) {
+ printf("Need a test name as argument\n");
+ return 1;
+ }
+
+ if (strcmp(argv[1], "pseudo_no_recursion") == 0) {
+ return test_walking(TEST_WITH_PSEUDO);
+ } else if (strcmp(argv[1], "no_pseudo_no_recursion") == 0) {
+ expected_gid = atoi(argv[2]);
+ expected_uid = atoi(argv[3]);
+ return test_walking(TEST_WITHOUT_PSEUDO);
+ } if (strcmp(argv[1], "pseudo_recursion") == 0) {
+ return test_walking_recursion(TEST_WITH_PSEUDO);
+ } if (strcmp(argv[1], "no_pseudo_recursion") == 0) {
+ expected_gid = atoi(argv[2]);
+ expected_uid = atoi(argv[3]);
+ return test_walking_recursion(TEST_WITHOUT_PSEUDO);
+ } else {
+ printf("Unknown test name: %s\n", argv[1]);
+ return 1;
+ }
+}
diff --git a/test/nftw-test-impl.c b/test/nftw-test-impl.c
new file mode 100644
index 0000000..5bef281
--- /dev/null
+++ b/test/nftw-test-impl.c
@@ -0,0 +1,236 @@
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <ftw.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdlib.h>
+
+#define PATH_MAX 1024
+#define LAST_VAL 999
+#define LAST_PATH "LAST_SENTINEL"
+
+#define TEST_WITH_PSEUDO 1
+#define TEST_WITHOUT_PSEUDO 0
+
+#define TEST_CHDIR 1
+#define TEST_NO_CHDIR 0
+
+static int current_idx = 0;
+static int* current_responses;
+static char** expected_fpaths;
+
+static int pseudo_active;
+static int verify_folder = 0;
+static char* base_dir = NULL;
+
+static unsigned int expected_gid;
+static unsigned int expected_uid;
+
+static int compare_paths(const char *path1, const char *path2){
+ char full_path1[PATH_MAX] = {0};
+ char full_path2[PATH_MAX] = {0};
+
+ if (path1[0] == '.'){
+ strcat(full_path1, base_dir);
+ strcat(full_path1, path1 + 1);
+ } else {
+ strcpy(full_path1, path1);
+ }
+
+ if (path2[0] == '.'){
+ strcat(full_path2, base_dir);
+ strcat(full_path2, path2 + 1);
+ } else {
+ strcpy(full_path2, path2);
+ }
+
+ return strcmp(full_path1, full_path2);
+}
+
+static int callback(const char* fpath, const struct NFTW_STAT_STRUCT *sb, int typeflag, struct FTW *ftwbuf){
+ int ret = current_responses[current_idx];
+// printf("path: %s, ret: %d\n", fpath, ret);
+
+ if (ret == LAST_VAL){
+ printf("Unexpected callback, it should have stopped already! fpath: %s\n", fpath);
+ return FTW_STOP;
+ }
+
+ char* expected_fpath_ending = expected_fpaths[current_idx];
+
+ if (strcmp(expected_fpath_ending, LAST_PATH) == 0){
+ printf("Unexpected fpath received: %s\n", fpath);
+ return FTW_STOP;
+ }
+
+ const char* actual_fpath_ending = fpath + strlen(fpath) - strlen(expected_fpath_ending);
+
+ if (strcmp(actual_fpath_ending, expected_fpath_ending) != 0){
+ printf("Incorrect fpath received. Expected: %s, actual: %s\n", expected_fpath_ending, actual_fpath_ending);
+ return FTW_STOP;
+ }
+
+ if (pseudo_active) {
+ if (sb->st_gid != 0 || sb->st_uid != 0) {
+ printf("Invalid uid/gid! Gid (act/exp): %d/%d, Uid (act/exp): %d/%d\n", sb->st_gid, 0, sb->st_uid, 0);
+ return FTW_STOP;
+ }
+ } else if (sb->st_gid != expected_gid || sb->st_uid != expected_uid) {
+ printf("Invalid uid/gid! Gid (act/exp): %d/%d, Uid (act/exp): %d/%d\n", sb->st_gid, expected_gid, sb->st_uid, expected_uid);
+ return FTW_STOP;
+ }
+
+ if (verify_folder) {
+ int res;
+ char* cwd = NULL;
+ cwd = getcwd(NULL, 0);
+
+ char* exp_cwd = NULL;
+ if (typeflag == FTW_DP){
+ res = compare_paths(fpath, cwd);
+ } else {
+ char* exp_cwd = malloc(ftwbuf->base);
+ memset(exp_cwd, 0, ftwbuf->base);
+ strncpy(exp_cwd, fpath, ftwbuf->base - 1);
+ res = compare_paths(cwd, exp_cwd);
+ }
+
+ free(cwd);
+ free(exp_cwd);
+
+ if (res != 0) {
+ printf("Incorrect folder for %s\n", fpath);
+ return FTW_STOP;
+ }
+ }
+
+ ++current_idx;
+ return ret;
+}
+
+static int run_test(int* responses, char** fpaths, int expected_retval, int with_pseudo, int flags) {
+ int ret;
+ current_responses = responses;
+ expected_fpaths = fpaths;
+ pseudo_active = with_pseudo;
+
+ ret = NFTW_NAME("./walking", callback, 10, flags);
+ current_responses = NULL;
+ expected_fpaths = NULL;
+
+ if (ret != expected_retval){
+ printf("Incorrect return value. Expected: %d, actual: %d\n", expected_retval, ret);
+ return 1;
+ }
+
+ if (responses[current_idx] != LAST_VAL){
+ printf("Not all expected paths were walked!\n");
+ return 1;
+ }
+ return 0;
+}
+
+static int test_skip_siblings_file_depth_walking(int with_pseudo, int change_dir){
+ int responses[] = {FTW_SKIP_SIBLINGS, FTW_CONTINUE, FTW_SKIP_SIBLINGS, FTW_CONTINUE,
+ FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, FTW_CONTINUE, LAST_VAL};
+ char* fpaths[] = {"walking/a1/b2/file5",
+ "walking/a1/b2",
+ "walking/a1/b1/c1/file",
+ "walking/a1/b1/c1",
+ "walking/a1/b1",
+ "walking/a1/b3",
+ "walking/a1",
+ "walking",
+ LAST_PATH};
+ int expected_retval = 0;
+ int flags = FTW_ACTIONRETVAL | FTW_DEPTH;
+
+ // store base_dir, because the fpath returned by (n)ftw can be relative to this
+ // folder - that way a full absolute path can be constructed and compared,
+ // if needed.
+ if (change_dir){
+ flags |= FTW_CHDIR;
+ base_dir = getcwd(NULL, 0);
+ verify_folder = 1;
+ }
+
+ return run_test(responses, fpaths, expected_retval, with_pseudo, flags);
+}
+
+/*
+ * Every time a folder entry is sent to the callback, respond with FTW_SKIP_SUBTREE.
+ * This should skip that particular folder completely, and continue processing
+ * with its siblings (or parent, if there are no siblings).
+ * Return value is expected to be 0, default walking order.
+ */
+static int test_skip_subtree_on_folder(int with_pseudo){
+ int responses[] = {FTW_CONTINUE, FTW_CONTINUE, FTW_SKIP_SUBTREE, FTW_SKIP_SUBTREE,
+ FTW_SKIP_SUBTREE, LAST_VAL};
+ char* fpaths[] = {"walking",
+ "walking/a1",
+ "walking/a1/b2",
+ "walking/a1/b1",
+ "walking/a1/b3",
+ LAST_PATH};
+ int expected_retval = 0;
+ int flags = FTW_ACTIONRETVAL;
+
+ return run_test(responses, fpaths, expected_retval, with_pseudo, flags);
+}
+
+/*
+ * Arguments:
+ * argv[1]: always the test name
+ * argv[2], argv[3]: in case the test name refers to a test without using
+ * pseudo (no_pseudo), then they should be the gid and uid
+ * of the current user. Otherwise these arguments are ignored.
+ *
+ * skip_subtree_pseudo/skip_subtree_no_pseudo: these tests are calling nftw()
+ * with the FTW_ACTIONRETVAL flag, which reacts based on the return value from the
+ * callback. These tests check the call's reaction to FTW_SKIP_SUBTREE call,
+ * upon which nftw() should stop processing the current folder, and continue
+ * with the next sibling of the folder.
+ *
+ * skip_siblings_pseudo/skip_siblings_no_pseudo: very similar to skip_subtree
+ * tests, but it verified FTW_SKIP_SIBLINGS response, which should stop processing
+ * the current folder, and continue in its parent.
+ *
+ * skip_siblings_chdir_pseudo/skip_siblings_chdir_no_pseudoL same as skip_siblings
+ * tests, but also pass the FTW_CHDIR flag and verify that the working directory
+ * is changed as expected between callback calls.
+ *
+ * nftw64 call only exists on Linux in case __USE_LARGEFILE64 is defined.
+ * If this is not the case, just skip this test.
+ */
+int main(int argc, char* argv[])
+{
+#if !defined(__USE_LARGEFILE64) && NFTW_NAME == nftw64
+return 0
+#endif
+ if (argc < 2) {
+ printf("Need a test name as argument\n");
+ return 1;
+ }
+
+ if (argc > 2) {
+ expected_gid = atoi(argv[2]);
+ expected_uid = atoi(argv[3]);
+ }
+
+ if (strcmp(argv[1], "skip_subtree_pseudo") == 0) {
+ return test_skip_subtree_on_folder(TEST_WITH_PSEUDO);
+ } else if (strcmp(argv[1], "skip_subtree_no_pseudo") == 0) {
+ return test_skip_subtree_on_folder(TEST_WITHOUT_PSEUDO);
+ } else if (strcmp(argv[1], "skip_siblings_pseudo") == 0) {
+ return test_skip_siblings_file_depth_walking(TEST_WITH_PSEUDO, TEST_NO_CHDIR);
+ } else if (strcmp(argv[1], "skip_siblings_no_pseudo") == 0) {
+ return test_skip_siblings_file_depth_walking(TEST_WITHOUT_PSEUDO, TEST_NO_CHDIR);
+ } else if (strcmp(argv[1], "skip_siblings_chdir_pseudo") == 0) {
+ return test_skip_siblings_file_depth_walking(TEST_WITH_PSEUDO, TEST_CHDIR);
+ } else if (strcmp(argv[1], "skip_siblings_chdir_no_pseudo") == 0) {
+ return test_skip_siblings_file_depth_walking(TEST_WITHOUT_PSEUDO, TEST_CHDIR);
+ } else {
+ printf("Unknown test name\n");
+ return 1;
+ }
+}
diff --git a/test/test-ftw.c b/test/test-ftw.c
new file mode 100644
index 0000000..5c47dd9
--- /dev/null
+++ b/test/test-ftw.c
@@ -0,0 +1,4 @@
+#define FTW_NAME ftw
+#define FTW_STAT_STRUCT stat
+
+#include "ftw-test-impl.c"
diff --git a/test/test-ftw64.c b/test/test-ftw64.c
new file mode 100644
index 0000000..0b8f906
--- /dev/null
+++ b/test/test-ftw64.c
@@ -0,0 +1,4 @@
+#define FTW_NAME ftw64
+#define FTW_STAT_STRUCT stat64
+
+#include "ftw-test-impl.c"
diff --git a/test/test-nftw.c b/test/test-nftw.c
new file mode 100644
index 0000000..ecadc1e
--- /dev/null
+++ b/test/test-nftw.c
@@ -0,0 +1,4 @@
+#define NFTW_NAME nftw
+#define NFTW_STAT_STRUCT stat
+
+#include "nftw-test-impl.c"
diff --git a/test/test-nftw.sh b/test/test-nftw.sh
new file mode 100755
index 0000000..c13e12b
--- /dev/null
+++ b/test/test-nftw.sh
@@ -0,0 +1,84 @@
+#!/bin/bash
+#
+# Test nftw call and its behavior modifying flags
+# SPDX-License-Identifier: LGPL-2.1-only
+#
+
+trap "rm -rf ./walking" 0
+
+check_retval_and_fail_if_needed(){
+ if [ $1 -ne 0 ]; then
+ echo $2
+ exit 1
+ fi
+}
+
+
+mkdir -p walking/a1/b1/c1
+touch walking/a1/b1/c1/file
+mkdir walking/a1/b2
+mkdir walking/a1/b3
+touch walking/a1/b1/c1/file2
+touch walking/a1/b1/c1/file3
+touch walking/a1/b2/file4
+touch walking/a1/b2/file5
+
+./test/test-nftw skip_subtree_pseudo
+check_retval_and_fail_if_needed $? "nftw subtree skipping with pseudo failed"
+
+./test/test-nftw skip_siblings_pseudo
+check_retval_and_fail_if_needed $? "nftw sibling skipping with pseudo failed"
+
+./test/test-nftw skip_siblings_chdir_pseudo
+check_retval_and_fail_if_needed $? "nftw sibling skipping chddir with pseudo failed"
+
+./test/test-nftw64 skip_subtree_pseudo
+check_retval_and_fail_if_needed $? "nftw64 subtree skipping with pseudo failed"
+
+./test/test-nftw64 skip_siblings_pseudo
+check_retval_and_fail_if_needed $? "nftw64 sibling skipping with pseudo failed"
+
+./test/test-ftw pseudo_no_recursion
+check_retval_and_fail_if_needed $? "ftw non-recursive walking with pseudo failed"
+
+./test/test-ftw pseudo_recursion
+check_retval_and_fail_if_needed $? "ftw recursive walking with pseudo failed"
+
+./test/test-ftw64 pseudo_no_recursion
+check_retval_and_fail_if_needed $? "ftw64 non-recursive walking with pseudo failed"
+
+./test/test-ftw64 pseudo_recursion
+check_retval_and_fail_if_needed $? "ftw64 recursive walking with pseudo failed"
+
+
+export PSEUDO_DISABLED=1
+
+uid=`env -i id -u`
+gid=`env -i id -g`
+
+./test/test-nftw skip_subtree_no_pseudo $gid $uid
+check_retval_and_fail_if_needed $? "nftw subtree skipping without pseudo failed"
+
+./test/test-nftw skip_siblings_no_pseudo $gid $uid
+check_retval_and_fail_if_needed $? "nftw sibling skipping without pseudo failed"
+
+./test/test-nftw skip_siblings_chdir_no_pseudo $gid $uid
+check_retval_and_fail_if_needed $? "nftw sibling skipping chdir without pseudo failed"
+
+./test/test-nftw64 skip_subtree_no_pseudo $gid $uid
+check_retval_and_fail_if_needed $? "nftw subtree skipping without pseudo failed"
+
+./test/test-nftw64 skip_siblings_no_pseudo $gid $uid
+check_retval_and_fail_if_needed $? "nftw sibling skipping without pseudo failed"
+
+./test/test-ftw no_pseudo_no_recursion $gid $uid
+check_retval_and_fail_if_needed $? "ftw non-recursive walking without pseudo failed"
+
+./test/test-ftw no_pseudo_recursion $gid $uid
+check_retval_and_fail_if_needed $? "ftw recursive walking without pseudo failed"
+
+./test/test-ftw64 no_pseudo_no_recursion $gid $uid
+check_retval_and_fail_if_needed $? "ftw non-recursive walking without pseudo failed"
+
+./test/test-ftw64 no_pseudo_recursion $gid $uid
+check_retval_and_fail_if_needed $? "ftw recursive walking without pseudo failed"
diff --git a/test/test-nftw64.c b/test/test-nftw64.c
new file mode 100644
index 0000000..20f25af
--- /dev/null
+++ b/test/test-nftw64.c
@@ -0,0 +1,4 @@
+#define NFTW_NAME nftw64
+#define NFTW_STAT_STRUCT stat64
+
+#include "nftw-test-impl.c"
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers
2025-04-07 19:14 [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers Gyorgy Sarvari
2025-04-07 19:14 ` [pseudo][PATCH v2 1/2] nftw, ftw: add wrapper Gyorgy Sarvari
2025-04-07 19:14 ` [pseudo][PATCH v2 2/2] nftw, ftw: add tests Gyorgy Sarvari
@ 2025-04-07 23:11 ` Mark Hatle
[not found] ` <18341F33308F8E0B.31078@lists.yoctoproject.org>
[not found] ` <18342C3498EB800F.31078@lists.yoctoproject.org>
4 siblings, 0 replies; 13+ messages in thread
From: Mark Hatle @ 2025-04-07 23:11 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock
Thank you for the additional items. I'm going to attempt to review this in the
next day or so and I'll reply with my findings. On the surface it looks a bit
more complicated then I originally thought it would. But that may very well be
needed.
--Mark
On 4/7/25 2:14 PM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
> This is an (attempt at the) implementation of the nftw and ftw[1] calls in pseudo.
>
> The main motivation is a change in btrfs-tools[2], in which
> they have changed from walking the filetree and calling stat
> on each entries separately to using the nftw call. As a result,
> btrfs filesystem generation currently happens with incorrect
> ownership details, as the nftw calls go around pseudo.
>
> See also the relevant report[3] on the Yocto mailing list.
>
> The main idea: with a custom wrapper capture the nftw call, and switch the callback
> to our own callback. When our own callback is called, fix the received
> stat struct, and call the original callback, this time with the correct
> stat struct.
>
> Big thanks to Lander Van Loock, who not only reported the
> original issue, but also helped testing and reviewing the first version
> of the code.
>
> Please let me know what you think.
>
> [1]: https://linux.die.net/man/3/nftw
> [2]: https://github.com/kdave/btrfs-progs/commit/c6464d3f99ed1dabceff1168eabb207492c37624
> [3]: https://lists.yoctoproject.org/g/yocto/message/64889
>
>
> ---
> v1: https://lore.kernel.org/yocto-patches/20250317113445.3855518-1-skandigraun@gmail.com/T/
>
> changes in v2:
> - Add wrapper for ftw, ftw64, nftw and nftw64 (instead of only nftw in v1). All of them
> use the same wrapper file, using macros to account for the naming differences.
> - ftw64 and nftw64 depend on large file support of the system. To account for this, move
> these implementations into their own subport folder, and compile them conditionally.
> - Move tests into separate commit (and add some tests for the new calls too)
> - The original implementation didn't consider multiple concurrent requests
> when it was saving the original call's details (callback function pointer,
> flags), and subsequent calls could overwrite data. This version stores
> these details in an array that behaves similar to a LIFO proto-stack.
> - Fix one bug in FTW_CHDIR flag behavior in conjuction with FTW_DEPTH flag (it switched
> to a folder, which didn't match glibc implementation's behavior)
> - Minor change in Makefile: since the tests between ftw/ftw64 and nftw/nftw64 are exactly
> the same, they are also implemented in the same file, which however is expected to be
> included in other files, and is not a compile-unit on its own. For this, the Makefile
> looks for files with "test-" prefix in the test folder.
>
> ---
>
> Gyorgy Sarvari (2):
> nftw, ftw: add wrapper
> nftw, ftw: add tests
>
> Makefile.in | 4 +-
> guts/README | 6 +-
> ports/linux/guts/ftw64.c | 16 --
> ports/linux/nftw64/guts/ftw64.c | 29 +++
> ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
> ports/linux/nftw64/pseudo_wrappers.c | 45 +++++
> ports/linux/nftw64/wrapfuncs.in | 2 +
> ports/linux/subports | 14 ++
> ports/linux/wrapfuncs.in | 2 -
> ports/unix/guts/ftw.c | 13 +-
> ports/unix/guts/nftw.c | 7 +-
> ports/unix/guts/nftw_wrapper_base.c | 211 ++++++++++++++++++++++
> ports/unix/pseudo_wrappers.c | 45 +++++
> ports/unix/wrapfuncs.in | 2 +-
> test/ftw-test-impl.c | 226 +++++++++++++++++++++++
> test/nftw-test-impl.c | 236 +++++++++++++++++++++++++
> test/test-ftw.c | 4 +
> test/test-ftw64.c | 4 +
> test/test-nftw.c | 4 +
> test/test-nftw.sh | 84 +++++++++
> test/test-nftw64.c | 4 +
> 21 files changed, 937 insertions(+), 28 deletions(-)
> delete mode 100644 ports/linux/guts/ftw64.c
> create mode 100644 ports/linux/nftw64/guts/ftw64.c
> rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
> create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
> create mode 100644 ports/linux/nftw64/wrapfuncs.in
> create mode 100644 ports/unix/guts/nftw_wrapper_base.c
> create mode 100644 test/ftw-test-impl.c
> create mode 100644 test/nftw-test-impl.c
> create mode 100644 test/test-ftw.c
> create mode 100644 test/test-ftw64.c
> create mode 100644 test/test-nftw.c
> create mode 100755 test/test-nftw.sh
> create mode 100644 test/test-nftw64.c
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#1297): https://lists.yoctoproject.org/g/yocto-patches/message/1297
> Mute This Topic: https://lists.yoctoproject.org/mt/112139640/3616948
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 2/2] nftw, ftw: add tests
[not found] ` <18341F33308F8E0B.31078@lists.yoctoproject.org>
@ 2025-04-08 14:33 ` Gyorgy Sarvari
2025-04-08 16:06 ` Mark Hatle
0 siblings, 1 reply; 13+ messages in thread
From: Gyorgy Sarvari @ 2025-04-08 14:33 UTC (permalink / raw)
To: yocto-patches
On 4/7/25 21:14, Gyorgy Sarvari via lists.yoctoproject.org wrote:
> +#if !defined(__USE_LARGEFILE64) && FTW_NAME == ftw64
> +return 0
> +#endif
If nothing else, at the very least a semicolon is missing in this short
path when the call is not available (and in its nftw64 pair). Will keep
it in mind to fix - won't submit a v3 only with this on my own.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 2/2] nftw, ftw: add tests
2025-04-08 14:33 ` [yocto-patches] [pseudo][PATCH v2 2/2] nftw, ftw: add tests Gyorgy Sarvari
@ 2025-04-08 16:06 ` Mark Hatle
2025-04-26 21:14 ` Ferry Toth
0 siblings, 1 reply; 13+ messages in thread
From: Mark Hatle @ 2025-04-08 16:06 UTC (permalink / raw)
To: yocto-patches
On 4/8/25 9:33 AM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
> On 4/7/25 21:14, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>> +#if !defined(__USE_LARGEFILE64) && FTW_NAME == ftw64
>> +return 0
>> +#endif
> If nothing else, at the very least a semicolon is missing in this short
> path when the call is not available (and in its nftw64 pair). Will keep
> it in mind to fix - won't submit a v3 only with this on my own.
Thanks, I'll add this to my local merge for review and testing.
--Mark
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#1333): https://lists.yoctoproject.org/g/yocto-patches/message/1333
> Mute This Topic: https://lists.yoctoproject.org/mt/112153968/3616948
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 2/2] nftw, ftw: add tests
2025-04-08 16:06 ` Mark Hatle
@ 2025-04-26 21:14 ` Ferry Toth
0 siblings, 0 replies; 13+ messages in thread
From: Ferry Toth @ 2025-04-26 21:14 UTC (permalink / raw)
To: Mark Hatle, yocto-patches, Gyorgy Sarvari
Hi Mark,
Op 08-04-2025 om 18:06 schreef Mark Hatle:
>
>
> On 4/8/25 9:33 AM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>> On 4/7/25 21:14, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>>> +#if !defined(__USE_LARGEFILE64) && FTW_NAME == ftw64
>>> +return 0
>>> +#endif
>> If nothing else, at the very least a semicolon is missing in this short
>> path when the call is not available (and in its nftw64 pair). Will keep
>> it in mind to fix - won't submit a v3 only with this on my own.
>
> Thanks, I'll add this to my local merge for review and testing.
Tested-by: Ferry Toth <fntoth@gmail.com>
This is an import patch for walnascar and master. The reason is that
walnascar already (even though it is not released) has btrfs-tools 6.13
which will not generate bootable images without this patch.
Also, scarthgap has btrfs-tools 6.7.1 which creates an image with errors
(2000 in my case, or maybe it stops counting). Luckily that image boots.
But scarthgap being LTS imho should have a backported btrfs-tools and
because of that also pseudo with this patch or similar.
I tested this on Intel Edison scarthgap with this patch in a bbappend
(and the other pseudo patches refreshed and git formatted).
> --Mark
>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers
[not found] ` <18342C3498EB800F.31078@lists.yoctoproject.org>
@ 2025-05-02 1:08 ` Mark Hatle
2025-05-02 1:17 ` Mark Hatle
1 sibling, 0 replies; 13+ messages in thread
From: Mark Hatle @ 2025-05-02 1:08 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock, Richard Purdie
I'm sorry this took as long as it did to review. I've been able to rework some
things, but the test cases need to be reworked. They don't work properly for me.
I've posted my work-in-progress pseudo tree to:
https://github.com/mhatle/pseudo/tree/master-next
The top three commits (I split and reworked patch 1/2, 2/2 has minor rework in
it, but doesn't function properly) are the current version of this code:
Move ftw and ftw64 to calling ntfw and nftw64:
https://github.com/mhatle/pseudo/commit/be112c56767d2b149403ddb84bf99fa8efc35797
The above is based on your investigation, but changes the way the call through
works.
There is no need to declare a special type for the function, instead typecasting
to (void *) will avoid the compiler warning and work the same way. (I took this
idea from glibc, which uses void * for the same reason.). And as your note
already indicated, the cast "should work", it seems wrong, but glibc does this.
nftw, nftw64: add wrapper:
https://github.com/mhatle/pseudo/commit/6bb390832553e7d112fd2375e27f8cebdb9e2acd
I reworked a good chunk of this, but it was only for integration not functionality.
Your initial version included a full copy of the wrapper code. This is
unnecessary as the system will already generate this code. Then the 'guts'
function simply needs to call your implementation. The typical way we've done
this in the past is tot implment a 'pseudo_func' function that holds the custom
work. This way the automatic ports just call it.
Your solution for the nftw_wrapper_base.c appears to be working well. I did
change a few things (moved many of the defines into that C file for instance)
and I resolved compiler warnings about return codes being ignored on chdir and
fchdir. All in all I checked and I don't believe I changed how this functions,
just the integration details.
Note I did drop the part of the commit:
+# nftw64 (and its deprecated pair, ftw64) are only present in case large
+# file support is present.
+cat > dummy.c <<EOF
+#define _GNU_SOURCE
+#include <features.h>
+#ifndef __USE_LARGEFILE64
+#error "no large file support"
+#endif
+int i;
+EOF
+if ${CC} -c -o dummy.o dummy.c >/dev/null 2>&1; then
+ echo linux/nftw64
+fi
+rm -f dummy.c dummy.o
Since nftw64 is already in the wrapfuncs.in it should be unnecessary. However,
if we need to make this dynamic, the right way is to add your change BACK into
the system and remove it from wrapfuncs.in. If this is the case, I'd like to
make it a separate commit so we're clear and what we're doing.
ftw, nftw, ftw64 and nftw64: add tests:
https://github.com/mhatle/pseudo/commit/86d228c1308949b1917cdcd1feae1fd8b08457ea
This commit I could not get working. I attempted to diagnose it, and never did
figure out what was wrong.
One change I did make was to avoid stopping in the case of a failure, but
running all of the test cases so we got a full list of information from the test
run. I'm running these tests by doing:
./configure --prefix=`pwd`/foo
make
make test
The output I get from the test cases is:
Incorrect return value. Expected: 0, actual: 2
test-nftw: nftw subtree skipping with pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping with pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping chddir with pseudo: Failed
Incorrect return value. Expected: 0, actual: 2
test-nftw: nftw64 subtree skipping with pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw64 sibling skipping with pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Incorrect return value. Expected: 0, actual: 1
test-nftw: ftw non-recursive walking with pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Recursive call failed
test-nftw: ftw recursive walking with pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Incorrect return value. Expected: 0, actual: 1
test-nftw: ftw64 non-recursive walking with pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Recursive call failed
test-nftw: ftw64 recursive walking with pseudo: Failed
Incorrect return value. Expected: 0, actual: 2
test-nftw: nftw subtree skipping without pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping without pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping chdir without pseudo: Failed
Incorrect return value. Expected: 0, actual: 2
test-nftw: nftw subtree skipping without pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping without pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Incorrect return value. Expected: 0, actual: 1
test-nftw: ftw non-recursive walking without pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Recursive call failed
test-nftw: ftw recursive walking without pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Incorrect return value. Expected: 0, actual: 1
test-nftw: ftw non-recursive walking without pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Recursive call failed
test-nftw: ftw recursive walking without pseudo: Failed
test-nftw: Failed.
Can you take a look at the test cases, I don't really understand them very well.
Some of this stuff looks like it could be off-by-1 errors both in the
'Expected' and 'Actual' side of things. As for the other failures, I'm really
not sure everything that is going on here and I figured it might be more obvious
to someone else.
--Mark
On 4/7/25 6:11 PM, Mark Hatle wrote:
> Thank you for the additional items. I'm going to attempt to review this in the
> next day or so and I'll reply with my findings. On the surface it looks a bit
> more complicated then I originally thought it would. But that may very well be
> needed.
>
> --Mark
>
> On 4/7/25 2:14 PM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>> This is an (attempt at the) implementation of the nftw and ftw[1] calls in pseudo.
>>
>> The main motivation is a change in btrfs-tools[2], in which
>> they have changed from walking the filetree and calling stat
>> on each entries separately to using the nftw call. As a result,
>> btrfs filesystem generation currently happens with incorrect
>> ownership details, as the nftw calls go around pseudo.
>>
>> See also the relevant report[3] on the Yocto mailing list.
>>
>> The main idea: with a custom wrapper capture the nftw call, and switch the callback
>> to our own callback. When our own callback is called, fix the received
>> stat struct, and call the original callback, this time with the correct
>> stat struct.
>>
>> Big thanks to Lander Van Loock, who not only reported the
>> original issue, but also helped testing and reviewing the first version
>> of the code.
>>
>> Please let me know what you think.
>>
>> [1]: https://linux.die.net/man/3/nftw
>> [2]: https://github.com/kdave/btrfs-progs/commit/c6464d3f99ed1dabceff1168eabb207492c37624
>> [3]: https://lists.yoctoproject.org/g/yocto/message/64889
>>
>>
>> ---
>> v1: https://lore.kernel.org/yocto-patches/20250317113445.3855518-1-skandigraun@gmail.com/T/
>>
>> changes in v2:
>> - Add wrapper for ftw, ftw64, nftw and nftw64 (instead of only nftw in v1). All of them
>> use the same wrapper file, using macros to account for the naming differences.
>> - ftw64 and nftw64 depend on large file support of the system. To account for this, move
>> these implementations into their own subport folder, and compile them conditionally.
>> - Move tests into separate commit (and add some tests for the new calls too)
>> - The original implementation didn't consider multiple concurrent requests
>> when it was saving the original call's details (callback function pointer,
>> flags), and subsequent calls could overwrite data. This version stores
>> these details in an array that behaves similar to a LIFO proto-stack.
>> - Fix one bug in FTW_CHDIR flag behavior in conjuction with FTW_DEPTH flag (it switched
>> to a folder, which didn't match glibc implementation's behavior)
>> - Minor change in Makefile: since the tests between ftw/ftw64 and nftw/nftw64 are exactly
>> the same, they are also implemented in the same file, which however is expected to be
>> included in other files, and is not a compile-unit on its own. For this, the Makefile
>> looks for files with "test-" prefix in the test folder.
>>
>> ---
>>
>> Gyorgy Sarvari (2):
>> nftw, ftw: add wrapper
>> nftw, ftw: add tests
>>
>> Makefile.in | 4 +-
>> guts/README | 6 +-
>> ports/linux/guts/ftw64.c | 16 --
>> ports/linux/nftw64/guts/ftw64.c | 29 +++
>> ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
>> ports/linux/nftw64/pseudo_wrappers.c | 45 +++++
>> ports/linux/nftw64/wrapfuncs.in | 2 +
>> ports/linux/subports | 14 ++
>> ports/linux/wrapfuncs.in | 2 -
>> ports/unix/guts/ftw.c | 13 +-
>> ports/unix/guts/nftw.c | 7 +-
>> ports/unix/guts/nftw_wrapper_base.c | 211 ++++++++++++++++++++++
>> ports/unix/pseudo_wrappers.c | 45 +++++
>> ports/unix/wrapfuncs.in | 2 +-
>> test/ftw-test-impl.c | 226 +++++++++++++++++++++++
>> test/nftw-test-impl.c | 236 +++++++++++++++++++++++++
>> test/test-ftw.c | 4 +
>> test/test-ftw64.c | 4 +
>> test/test-nftw.c | 4 +
>> test/test-nftw.sh | 84 +++++++++
>> test/test-nftw64.c | 4 +
>> 21 files changed, 937 insertions(+), 28 deletions(-)
>> delete mode 100644 ports/linux/guts/ftw64.c
>> create mode 100644 ports/linux/nftw64/guts/ftw64.c
>> rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
>> create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
>> create mode 100644 ports/linux/nftw64/wrapfuncs.in
>> create mode 100644 ports/unix/guts/nftw_wrapper_base.c
>> create mode 100644 test/ftw-test-impl.c
>> create mode 100644 test/nftw-test-impl.c
>> create mode 100644 test/test-ftw.c
>> create mode 100644 test/test-ftw64.c
>> create mode 100644 test/test-nftw.c
>> create mode 100755 test/test-nftw.sh
>> create mode 100644 test/test-nftw64.c
>>
>>
>>
>>
>>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#1331): https://lists.yoctoproject.org/g/yocto-patches/message/1331
> Mute This Topic: https://lists.yoctoproject.org/mt/112139640/3616948
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers
[not found] ` <18342C3498EB800F.31078@lists.yoctoproject.org>
2025-05-02 1:08 ` [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers Mark Hatle
@ 2025-05-02 1:17 ` Mark Hatle
2025-05-02 8:53 ` Gyorgy Sarvari
[not found] ` <183BA9D5657FA6B7.6015@lists.yoctoproject.org>
1 sibling, 2 replies; 13+ messages in thread
From: Mark Hatle @ 2025-05-02 1:17 UTC (permalink / raw)
To: yocto-patches, skandigraun; +Cc: landervanloock, Richard Purdie, fntoth
[resend] my CC was incomplete and I want to make sure everyone interested is
aware of the review and status
+ skandigraun@gmail.com
+ fntoth@gmail.com
I'm sorry this took as long as it did to review. I've been able to rework some
things, but the test cases need to be reworked. They don't work properly for me.
I've posted my work-in-progress pseudo tree to:
https://github.com/mhatle/pseudo/tree/master-next
The top three commits (I split and reworked patch 1/2, 2/2 has minor rework in
it, but doesn't function properly) are the current version of this code:
Move ftw and ftw64 to calling ntfw and nftw64:
https://github.com/mhatle/pseudo/commit/be112c56767d2b149403ddb84bf99fa8efc35797
The above is based on your investigation, but changes the way the call through
works.
There is no need to declare a special type for the function, instead typecasting
to (void *) will avoid the compiler warning and work the same way. (I took this
idea from glibc, which uses void * for the same reason.). And as your note
already indicated, the cast "should work", it seems wrong, but glibc does this.
nftw, nftw64: add wrapper:
https://github.com/mhatle/pseudo/commit/6bb390832553e7d112fd2375e27f8cebdb9e2acd
I reworked a good chunk of this, but it was only for integration not functionality.
Your initial version included a full copy of the wrapper code. This is
unnecessary as the system will already generate this code. Then the 'guts'
function simply needs to call your implementation. The typical way we've done
this in the past is tot implment a 'pseudo_func' function that holds the custom
work. This way the automatic ports just call it.
Your solution for the nftw_wrapper_base.c appears to be working well. I did
change a few things (moved many of the defines into that C file for instance)
and I resolved compiler warnings about return codes being ignored on chdir and
fchdir. All in all I checked and I don't believe I changed how this functions,
just the integration details.
Note I did drop the part of the commit:
+# nftw64 (and its deprecated pair, ftw64) are only present in case large
+# file support is present.
+cat > dummy.c <<EOF
+#define _GNU_SOURCE
+#include <features.h>
+#ifndef __USE_LARGEFILE64
+#error "no large file support"
+#endif
+int i;
+EOF
+if ${CC} -c -o dummy.o dummy.c >/dev/null 2>&1; then
+ echo linux/nftw64
+fi
+rm -f dummy.c dummy.o
Since nftw64 is already in the wrapfuncs.in it should be unnecessary. However,
if we need to make this dynamic, the right way is to add your change BACK into
the system and remove it from wrapfuncs.in. If this is the case, I'd like to
make it a separate commit so we're clear and what we're doing.
ftw, nftw, ftw64 and nftw64: add tests:
https://github.com/mhatle/pseudo/commit/86d228c1308949b1917cdcd1feae1fd8b08457ea
This commit I could not get working. I attempted to diagnose it, and never did
figure out what was wrong.
One change I did make was to avoid stopping in the case of a failure, but
running all of the test cases so we got a full list of information from the test
run. I'm running these tests by doing:
./configure --prefix=`pwd`/foo
make
make test
The output I get from the test cases is:
Incorrect return value. Expected: 0, actual: 2
test-nftw: nftw subtree skipping with pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping with pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping chddir with pseudo: Failed
Incorrect return value. Expected: 0, actual: 2
test-nftw: nftw64 subtree skipping with pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw64 sibling skipping with pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Incorrect return value. Expected: 0, actual: 1
test-nftw: ftw non-recursive walking with pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Recursive call failed
test-nftw: ftw recursive walking with pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Incorrect return value. Expected: 0, actual: 1
test-nftw: ftw64 non-recursive walking with pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Recursive call failed
test-nftw: ftw64 recursive walking with pseudo: Failed
Incorrect return value. Expected: 0, actual: 2
test-nftw: nftw subtree skipping without pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping without pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping chdir without pseudo: Failed
Incorrect return value. Expected: 0, actual: 2
test-nftw: nftw subtree skipping without pseudo: Failed
Incorrect return value. Expected: 0, actual: 3
test-nftw: nftw sibling skipping without pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Incorrect return value. Expected: 0, actual: 1
test-nftw: ftw non-recursive walking without pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Recursive call failed
test-nftw: ftw recursive walking without pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Incorrect return value. Expected: 0, actual: 1
test-nftw: ftw non-recursive walking without pseudo: Failed
Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
alking/a1/b1/c1/file3
Recursive call failed
test-nftw: ftw recursive walking without pseudo: Failed
test-nftw: Failed.
Can you take a look at the test cases, I don't really understand them very well.
Some of this stuff looks like it could be off-by-1 errors both in the
'Expected' and 'Actual' side of things. As for the other failures, I'm really
not sure everything that is going on here and I figured it might be more obvious
to someone else.
--Mark
On 4/7/25 6:11 PM, Mark Hatle wrote:
> Thank you for the additional items. I'm going to attempt to review this in the
> next day or so and I'll reply with my findings. On the surface it looks a bit
> more complicated then I originally thought it would. But that may very well be
> needed.
>
> --Mark
>
> On 4/7/25 2:14 PM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>> This is an (attempt at the) implementation of the nftw and ftw[1] calls in pseudo.
>>
>> The main motivation is a change in btrfs-tools[2], in which
>> they have changed from walking the filetree and calling stat
>> on each entries separately to using the nftw call. As a result,
>> btrfs filesystem generation currently happens with incorrect
>> ownership details, as the nftw calls go around pseudo.
>>
>> See also the relevant report[3] on the Yocto mailing list.
>>
>> The main idea: with a custom wrapper capture the nftw call, and switch the callback
>> to our own callback. When our own callback is called, fix the received
>> stat struct, and call the original callback, this time with the correct
>> stat struct.
>>
>> Big thanks to Lander Van Loock, who not only reported the
>> original issue, but also helped testing and reviewing the first version
>> of the code.
>>
>> Please let me know what you think.
>>
>> [1]: https://linux.die.net/man/3/nftw
>> [2]: https://github.com/kdave/btrfs-progs/commit/c6464d3f99ed1dabceff1168eabb207492c37624
>> [3]: https://lists.yoctoproject.org/g/yocto/message/64889
>>
>>
>> ---
>> v1: https://lore.kernel.org/yocto-patches/20250317113445.3855518-1-skandigraun@gmail.com/T/
>>
>> changes in v2:
>> - Add wrapper for ftw, ftw64, nftw and nftw64 (instead of only nftw in v1). All of them
>> use the same wrapper file, using macros to account for the naming differences.
>> - ftw64 and nftw64 depend on large file support of the system. To account for this, move
>> these implementations into their own subport folder, and compile them conditionally.
>> - Move tests into separate commit (and add some tests for the new calls too)
>> - The original implementation didn't consider multiple concurrent requests
>> when it was saving the original call's details (callback function pointer,
>> flags), and subsequent calls could overwrite data. This version stores
>> these details in an array that behaves similar to a LIFO proto-stack.
>> - Fix one bug in FTW_CHDIR flag behavior in conjuction with FTW_DEPTH flag (it switched
>> to a folder, which didn't match glibc implementation's behavior)
>> - Minor change in Makefile: since the tests between ftw/ftw64 and nftw/nftw64 are exactly
>> the same, they are also implemented in the same file, which however is expected to be
>> included in other files, and is not a compile-unit on its own. For this, the Makefile
>> looks for files with "test-" prefix in the test folder.
>>
>> ---
>>
>> Gyorgy Sarvari (2):
>> nftw, ftw: add wrapper
>> nftw, ftw: add tests
>>
>> Makefile.in | 4 +-
>> guts/README | 6 +-
>> ports/linux/guts/ftw64.c | 16 --
>> ports/linux/nftw64/guts/ftw64.c | 29 +++
>> ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
>> ports/linux/nftw64/pseudo_wrappers.c | 45 +++++
>> ports/linux/nftw64/wrapfuncs.in | 2 +
>> ports/linux/subports | 14 ++
>> ports/linux/wrapfuncs.in | 2 -
>> ports/unix/guts/ftw.c | 13 +-
>> ports/unix/guts/nftw.c | 7 +-
>> ports/unix/guts/nftw_wrapper_base.c | 211 ++++++++++++++++++++++
>> ports/unix/pseudo_wrappers.c | 45 +++++
>> ports/unix/wrapfuncs.in | 2 +-
>> test/ftw-test-impl.c | 226 +++++++++++++++++++++++
>> test/nftw-test-impl.c | 236 +++++++++++++++++++++++++
>> test/test-ftw.c | 4 +
>> test/test-ftw64.c | 4 +
>> test/test-nftw.c | 4 +
>> test/test-nftw.sh | 84 +++++++++
>> test/test-nftw64.c | 4 +
>> 21 files changed, 937 insertions(+), 28 deletions(-)
>> delete mode 100644 ports/linux/guts/ftw64.c
>> create mode 100644 ports/linux/nftw64/guts/ftw64.c
>> rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
>> create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
>> create mode 100644 ports/linux/nftw64/wrapfuncs.in
>> create mode 100644 ports/unix/guts/nftw_wrapper_base.c
>> create mode 100644 test/ftw-test-impl.c
>> create mode 100644 test/nftw-test-impl.c
>> create mode 100644 test/test-ftw.c
>> create mode 100644 test/test-ftw64.c
>> create mode 100644 test/test-nftw.c
>> create mode 100755 test/test-nftw.sh
>> create mode 100644 test/test-nftw64.c
>>
>>
>>
>>
>>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#1331): https://lists.yoctoproject.org/g/yocto-patches/message/1331
> Mute This Topic: https://lists.yoctoproject.org/mt/112139640/3616948
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers
2025-05-02 1:17 ` Mark Hatle
@ 2025-05-02 8:53 ` Gyorgy Sarvari
2025-05-02 13:33 ` Mark Hatle
[not found] ` <183BA9D5657FA6B7.6015@lists.yoctoproject.org>
1 sibling, 1 reply; 13+ messages in thread
From: Gyorgy Sarvari @ 2025-05-02 8:53 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock, Richard Purdie, fntoth, mark.hatle
Thank you very much for spending time on this. Some comments inline.
On 5/2/25 03:17, Mark Hatle wrote:
> [resend] my CC was incomplete and I want to make sure everyone interested is
> aware of the review and status
>
> + skandigraun@gmail.com
> + fntoth@gmail.com
>
>
> I'm sorry this took as long as it did to review. I've been able to rework some
> things, but the test cases need to be reworked. They don't work properly for me.
>
> I've posted my work-in-progress pseudo tree to:
>
> https://github.com/mhatle/pseudo/tree/master-next
>
> The top three commits (I split and reworked patch 1/2, 2/2 has minor rework in
> it, but doesn't function properly) are the current version of this code:
>
> Move ftw and ftw64 to calling ntfw and nftw64:
>
> https://github.com/mhatle/pseudo/commit/be112c56767d2b149403ddb84bf99fa8efc35797
>
> The above is based on your investigation, but changes the way the call through
> works.
>
> There is no need to declare a special type for the function, instead typecasting
> to (void *) will avoid the compiler warning and work the same way. (I took this
> idea from glibc, which uses void * for the same reason.). And as your note
ACK.
> already indicated, the cast "should work", it seems wrong, but glibc does this.
:D
>
>
> nftw, nftw64: add wrapper:
>
> https://github.com/mhatle/pseudo/commit/6bb390832553e7d112fd2375e27f8cebdb9e2acd
>
> I reworked a good chunk of this, but it was only for integration not functionality.
>
> Your initial version included a full copy of the wrapper code. This is
> unnecessary as the system will already generate this code. Then the 'guts'
> function simply needs to call your implementation. The typical way we've done
> this in the past is tot implment a 'pseudo_func' function that holds the custom
> work. This way the automatic ports just call it.
Understood, thanks for the explanation.
>
> Your solution for the nftw_wrapper_base.c appears to be working well. I did
> change a few things (moved many of the defines into that C file for instance)
> and I resolved compiler warnings about return codes being ignored on chdir and
> fchdir. All in all I checked and I don't believe I changed how this functions,
> just the integration details.
ACK. Yes, at the first sight the main things haven't changed.
>
> Note I did drop the part of the commit:
>
> +# nftw64 (and its deprecated pair, ftw64) are only present in case large
> +# file support is present.
> +cat > dummy.c <<EOF
> +#define _GNU_SOURCE
> +#include <features.h>
> +#ifndef __USE_LARGEFILE64
> +#error "no large file support"
> +#endif
> +int i;
> +EOF
> +if ${CC} -c -o dummy.o dummy.c >/dev/null 2>&1; then
> + echo linux/nftw64
> +fi
> +rm -f dummy.c dummy.o
>
> Since nftw64 is already in the wrapfuncs.in it should be unnecessary. However,
> if we need to make this dynamic, the right way is to add your change BACK into
> the system and remove it from wrapfuncs.in. If this is the case, I'd like to
> make it a separate commit so we're clear and what we're doing.
I have no strong feelings about it. If we want to be pedantic, I think
it should be dynamic, on the other hand I have to admit that if the lack
of this check would have caused any real world problems, it would have
showed up in the past decade or two. I'm okay with not making it dynamic.
>
>
> ftw, nftw, ftw64 and nftw64: add tests:
>
> https://github.com/mhatle/pseudo/commit/86d228c1308949b1917cdcd1feae1fd8b08457ea
>
> This commit I could not get working. I attempted to diagnose it, and never did
> figure out what was wrong.
>
> One change I did make was to avoid stopping in the case of a failure, but
> running all of the test cases so we got a full list of information from the test
> run. I'm running these tests by doing:
>
> ./configure --prefix=`pwd`/foo
> make
> make test
>
> The output I get from the test cases is:
>
> Incorrect return value. Expected: 0, actual: 2
> test-nftw: nftw subtree skipping with pseudo: Failed
> Incorrect return value. Expected: 0, actual: 3
> test-nftw: nftw sibling skipping with pseudo: Failed
> Incorrect return value. Expected: 0, actual: 3
> test-nftw: nftw sibling skipping chddir with pseudo: Failed
> Incorrect return value. Expected: 0, actual: 2
> test-nftw: nftw64 subtree skipping with pseudo: Failed
> Incorrect return value. Expected: 0, actual: 3
> test-nftw: nftw64 sibling skipping with pseudo: Failed
> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
> alking/a1/b1/c1/file3
> Incorrect return value. Expected: 0, actual: 1
> test-nftw: ftw non-recursive walking with pseudo: Failed
> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
> alking/a1/b1/c1/file3
> Recursive call failed
> test-nftw: ftw recursive walking with pseudo: Failed
> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
> alking/a1/b1/c1/file3
> Incorrect return value. Expected: 0, actual: 1
> test-nftw: ftw64 non-recursive walking with pseudo: Failed
> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
> alking/a1/b1/c1/file3
> Recursive call failed
> test-nftw: ftw64 recursive walking with pseudo: Failed
> Incorrect return value. Expected: 0, actual: 2
> test-nftw: nftw subtree skipping without pseudo: Failed
> Incorrect return value. Expected: 0, actual: 3
> test-nftw: nftw sibling skipping without pseudo: Failed
> Incorrect return value. Expected: 0, actual: 3
> test-nftw: nftw sibling skipping chdir without pseudo: Failed
> Incorrect return value. Expected: 0, actual: 2
> test-nftw: nftw subtree skipping without pseudo: Failed
> Incorrect return value. Expected: 0, actual: 3
> test-nftw: nftw sibling skipping without pseudo: Failed
> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
> alking/a1/b1/c1/file3
> Incorrect return value. Expected: 0, actual: 1
> test-nftw: ftw non-recursive walking without pseudo: Failed
> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
> alking/a1/b1/c1/file3
> Recursive call failed
> test-nftw: ftw recursive walking without pseudo: Failed
> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
> alking/a1/b1/c1/file3
> Incorrect return value. Expected: 0, actual: 1
> test-nftw: ftw non-recursive walking without pseudo: Failed
> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
> alking/a1/b1/c1/file3
> Recursive call failed
> test-nftw: ftw recursive walking without pseudo: Failed
> test-nftw: Failed.
>
>
> Can you take a look at the test cases, I don't really understand them very well.
> Some of this stuff looks like it could be off-by-1 errors both in the
> 'Expected' and 'Actual' side of things. As for the other failures, I'm really
> not sure everything that is going on here and I figured it might be more obvious
> to someone else.
I suspect that I (and you also) became the victim of my own naiveness,
and the tests I created depend on the underlying FS type more than I
expected. I ran the tests only on ext4, but I guess that you are using a
different FS than I do, and that's why it fails (the tests expect to
encounter the entries in a specific order).
Going to come up with some FS- and fileorder-agnostic implementation.
Do you have any preference about the patch for the tests? Should I open
a PR in your github repo, or just send a patch on top of it here? Maybe
something else?
>
> --Mark
>
>
> On 4/7/25 6:11 PM, Mark Hatle wrote:
>> Thank you for the additional items. I'm going to attempt to review this in the
>> next day or so and I'll reply with my findings. On the surface it looks a bit
>> more complicated then I originally thought it would. But that may very well be
>> needed.
>>
>> --Mark
>>
>> On 4/7/25 2:14 PM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>>> This is an (attempt at the) implementation of the nftw and ftw[1] calls in pseudo.
>>>
>>> The main motivation is a change in btrfs-tools[2], in which
>>> they have changed from walking the filetree and calling stat
>>> on each entries separately to using the nftw call. As a result,
>>> btrfs filesystem generation currently happens with incorrect
>>> ownership details, as the nftw calls go around pseudo.
>>>
>>> See also the relevant report[3] on the Yocto mailing list.
>>>
>>> The main idea: with a custom wrapper capture the nftw call, and switch the callback
>>> to our own callback. When our own callback is called, fix the received
>>> stat struct, and call the original callback, this time with the correct
>>> stat struct.
>>>
>>> Big thanks to Lander Van Loock, who not only reported the
>>> original issue, but also helped testing and reviewing the first version
>>> of the code.
>>>
>>> Please let me know what you think.
>>>
>>> [1]: https://linux.die.net/man/3/nftw
>>> [2]: https://github.com/kdave/btrfs-progs/commit/c6464d3f99ed1dabceff1168eabb207492c37624
>>> [3]: https://lists.yoctoproject.org/g/yocto/message/64889
>>>
>>>
>>> ---
>>> v1: https://lore.kernel.org/yocto-patches/20250317113445.3855518-1-skandigraun@gmail.com/T/
>>>
>>> changes in v2:
>>> - Add wrapper for ftw, ftw64, nftw and nftw64 (instead of only nftw in v1). All of them
>>> use the same wrapper file, using macros to account for the naming differences.
>>> - ftw64 and nftw64 depend on large file support of the system. To account for this, move
>>> these implementations into their own subport folder, and compile them conditionally.
>>> - Move tests into separate commit (and add some tests for the new calls too)
>>> - The original implementation didn't consider multiple concurrent requests
>>> when it was saving the original call's details (callback function pointer,
>>> flags), and subsequent calls could overwrite data. This version stores
>>> these details in an array that behaves similar to a LIFO proto-stack.
>>> - Fix one bug in FTW_CHDIR flag behavior in conjuction with FTW_DEPTH flag (it switched
>>> to a folder, which didn't match glibc implementation's behavior)
>>> - Minor change in Makefile: since the tests between ftw/ftw64 and nftw/nftw64 are exactly
>>> the same, they are also implemented in the same file, which however is expected to be
>>> included in other files, and is not a compile-unit on its own. For this, the Makefile
>>> looks for files with "test-" prefix in the test folder.
>>>
>>> ---
>>>
>>> Gyorgy Sarvari (2):
>>> nftw, ftw: add wrapper
>>> nftw, ftw: add tests
>>>
>>> Makefile.in | 4 +-
>>> guts/README | 6 +-
>>> ports/linux/guts/ftw64.c | 16 --
>>> ports/linux/nftw64/guts/ftw64.c | 29 +++
>>> ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
>>> ports/linux/nftw64/pseudo_wrappers.c | 45 +++++
>>> ports/linux/nftw64/wrapfuncs.in | 2 +
>>> ports/linux/subports | 14 ++
>>> ports/linux/wrapfuncs.in | 2 -
>>> ports/unix/guts/ftw.c | 13 +-
>>> ports/unix/guts/nftw.c | 7 +-
>>> ports/unix/guts/nftw_wrapper_base.c | 211 ++++++++++++++++++++++
>>> ports/unix/pseudo_wrappers.c | 45 +++++
>>> ports/unix/wrapfuncs.in | 2 +-
>>> test/ftw-test-impl.c | 226 +++++++++++++++++++++++
>>> test/nftw-test-impl.c | 236 +++++++++++++++++++++++++
>>> test/test-ftw.c | 4 +
>>> test/test-ftw64.c | 4 +
>>> test/test-nftw.c | 4 +
>>> test/test-nftw.sh | 84 +++++++++
>>> test/test-nftw64.c | 4 +
>>> 21 files changed, 937 insertions(+), 28 deletions(-)
>>> delete mode 100644 ports/linux/guts/ftw64.c
>>> create mode 100644 ports/linux/nftw64/guts/ftw64.c
>>> rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
>>> create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
>>> create mode 100644 ports/linux/nftw64/wrapfuncs.in
>>> create mode 100644 ports/unix/guts/nftw_wrapper_base.c
>>> create mode 100644 test/ftw-test-impl.c
>>> create mode 100644 test/nftw-test-impl.c
>>> create mode 100644 test/test-ftw.c
>>> create mode 100644 test/test-ftw64.c
>>> create mode 100644 test/test-nftw.c
>>> create mode 100755 test/test-nftw.sh
>>> create mode 100644 test/test-nftw64.c
>>>
>>>
>>>
>>>
>>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#1331): https://lists.yoctoproject.org/g/yocto-patches/message/1331
>> Mute This Topic: https://lists.yoctoproject.org/mt/112139640/3616948
>> Group Owner: yocto-patches+owner@lists.yoctoproject.org
>> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers
[not found] ` <183BA9D5657FA6B7.6015@lists.yoctoproject.org>
@ 2025-05-02 9:53 ` Gyorgy Sarvari
2025-05-02 13:42 ` Mark Hatle
0 siblings, 1 reply; 13+ messages in thread
From: Gyorgy Sarvari @ 2025-05-02 9:53 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock, Richard Purdie, fntoth, mark.hatle
On 5/2/25 10:53, Gyorgy Sarvari via lists.yoctoproject.org wrote:
> Thank you very much for spending time on this. Some comments inline.
>
> On 5/2/25 03:17, Mark Hatle wrote:
>> [resend] my CC was incomplete and I want to make sure everyone interested is
>> aware of the review and status
>>
>> + skandigraun@gmail.com
>> + fntoth@gmail.com
>>
>>
>> I'm sorry this took as long as it did to review. I've been able to rework some
>> things, but the test cases need to be reworked. They don't work properly for me.
>>
>> I've posted my work-in-progress pseudo tree to:
>>
>> https://github.com/mhatle/pseudo/tree/master-next
>>
>> The top three commits (I split and reworked patch 1/2, 2/2 has minor rework in
>> it, but doesn't function properly) are the current version of this code:
>>
>> Move ftw and ftw64 to calling ntfw and nftw64:
>>
>> https://github.com/mhatle/pseudo/commit/be112c56767d2b149403ddb84bf99fa8efc35797
>>
>> The above is based on your investigation, but changes the way the call through
>> works.
>>
>> There is no need to declare a special type for the function, instead typecasting
>> to (void *) will avoid the compiler warning and work the same way. (I took this
>> idea from glibc, which uses void * for the same reason.). And as your note
> ACK.
>> already indicated, the cast "should work", it seems wrong, but glibc does this.
> :D
>>
>> nftw, nftw64: add wrapper:
>>
>> https://github.com/mhatle/pseudo/commit/6bb390832553e7d112fd2375e27f8cebdb9e2acd
>>
>> I reworked a good chunk of this, but it was only for integration not functionality.
>>
>> Your initial version included a full copy of the wrapper code. This is
>> unnecessary as the system will already generate this code. Then the 'guts'
>> function simply needs to call your implementation. The typical way we've done
>> this in the past is tot implment a 'pseudo_func' function that holds the custom
>> work. This way the automatic ports just call it.
> Understood, thanks for the explanation.
>> Your solution for the nftw_wrapper_base.c appears to be working well. I did
>> change a few things (moved many of the defines into that C file for instance)
>> and I resolved compiler warnings about return codes being ignored on chdir and
>> fchdir. All in all I checked and I don't believe I changed how this functions,
>> just the integration details.
> ACK. Yes, at the first sight the main things haven't changed.
There would be 2 things I have noticed since in the nftw_wrappers_base.c
file:
1. Some comments became outdated in the code, they can be removed:
Line 117 - The comment says no error checking is done, but you have
added some.
Line 183 - This comment was useful when the next statement was in a
separate guts file. It became somewhat misleading, and can be deleted.
2. in "static int NFTW_CALLBACK_NAME(...)" function you have introduced
pseudo_sb variable, instead of reusing the *sb pointer from the
arguments. Generally it's fine, however it might stay at its initial
state if the walked entry is unreadable (line 148). I *believe* that
even in this erroneous case the original sb struct contains some info,
like device ID or inode number (though I would have to verify to be sure
what's inside), which shouldn't be lost.
The current state also made the corresponding comment outdated in line
146 - though I think the comment isn't inherently bad (until proven
otherwise). What if sb would be copied to pseudo_sb in the else branch?
Or remove the condition completely (along with the comment)?
>> Note I did drop the part of the commit:
>>
>> +# nftw64 (and its deprecated pair, ftw64) are only present in case large
>> +# file support is present.
>> +cat > dummy.c <<EOF
>> +#define _GNU_SOURCE
>> +#include <features.h>
>> +#ifndef __USE_LARGEFILE64
>> +#error "no large file support"
>> +#endif
>> +int i;
>> +EOF
>> +if ${CC} -c -o dummy.o dummy.c >/dev/null 2>&1; then
>> + echo linux/nftw64
>> +fi
>> +rm -f dummy.c dummy.o
>>
>> Since nftw64 is already in the wrapfuncs.in it should be unnecessary. However,
>> if we need to make this dynamic, the right way is to add your change BACK into
>> the system and remove it from wrapfuncs.in. If this is the case, I'd like to
>> make it a separate commit so we're clear and what we're doing.
> I have no strong feelings about it. If we want to be pedantic, I think
> it should be dynamic, on the other hand I have to admit that if the lack
> of this check would have caused any real world problems, it would have
> showed up in the past decade or two. I'm okay with not making it dynamic.
>>
>> ftw, nftw, ftw64 and nftw64: add tests:
>>
>> https://github.com/mhatle/pseudo/commit/86d228c1308949b1917cdcd1feae1fd8b08457ea
>>
>> This commit I could not get working. I attempted to diagnose it, and never did
>> figure out what was wrong.
>>
>> One change I did make was to avoid stopping in the case of a failure, but
>> running all of the test cases so we got a full list of information from the test
>> run. I'm running these tests by doing:
>>
>> ./configure --prefix=`pwd`/foo
>> make
>> make test
>>
>> The output I get from the test cases is:
>>
>> Incorrect return value. Expected: 0, actual: 2
>> test-nftw: nftw subtree skipping with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping chddir with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 2
>> test-nftw: nftw64 subtree skipping with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw64 sibling skipping with pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Incorrect return value. Expected: 0, actual: 1
>> test-nftw: ftw non-recursive walking with pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Recursive call failed
>> test-nftw: ftw recursive walking with pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Incorrect return value. Expected: 0, actual: 1
>> test-nftw: ftw64 non-recursive walking with pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Recursive call failed
>> test-nftw: ftw64 recursive walking with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 2
>> test-nftw: nftw subtree skipping without pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping without pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping chdir without pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 2
>> test-nftw: nftw subtree skipping without pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping without pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Incorrect return value. Expected: 0, actual: 1
>> test-nftw: ftw non-recursive walking without pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Recursive call failed
>> test-nftw: ftw recursive walking without pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Incorrect return value. Expected: 0, actual: 1
>> test-nftw: ftw non-recursive walking without pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Recursive call failed
>> test-nftw: ftw recursive walking without pseudo: Failed
>> test-nftw: Failed.
>>
>>
>> Can you take a look at the test cases, I don't really understand them very well.
>> Some of this stuff looks like it could be off-by-1 errors both in the
>> 'Expected' and 'Actual' side of things. As for the other failures, I'm really
>> not sure everything that is going on here and I figured it might be more obvious
>> to someone else.
> I suspect that I (and you also) became the victim of my own naiveness,
> and the tests I created depend on the underlying FS type more than I
> expected. I ran the tests only on ext4, but I guess that you are using a
> different FS than I do, and that's why it fails (the tests expect to
> encounter the entries in a specific order).
> Going to come up with some FS- and fileorder-agnostic implementation.
>
> Do you have any preference about the patch for the tests? Should I open
> a PR in your github repo, or just send a patch on top of it here? Maybe
> something else?
>> --Mark
>>
>>
>> On 4/7/25 6:11 PM, Mark Hatle wrote:
>>> Thank you for the additional items. I'm going to attempt to review this in the
>>> next day or so and I'll reply with my findings. On the surface it looks a bit
>>> more complicated then I originally thought it would. But that may very well be
>>> needed.
>>>
>>> --Mark
>>>
>>> On 4/7/25 2:14 PM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>>>> This is an (attempt at the) implementation of the nftw and ftw[1] calls in pseudo.
>>>>
>>>> The main motivation is a change in btrfs-tools[2], in which
>>>> they have changed from walking the filetree and calling stat
>>>> on each entries separately to using the nftw call. As a result,
>>>> btrfs filesystem generation currently happens with incorrect
>>>> ownership details, as the nftw calls go around pseudo.
>>>>
>>>> See also the relevant report[3] on the Yocto mailing list.
>>>>
>>>> The main idea: with a custom wrapper capture the nftw call, and switch the callback
>>>> to our own callback. When our own callback is called, fix the received
>>>> stat struct, and call the original callback, this time with the correct
>>>> stat struct.
>>>>
>>>> Big thanks to Lander Van Loock, who not only reported the
>>>> original issue, but also helped testing and reviewing the first version
>>>> of the code.
>>>>
>>>> Please let me know what you think.
>>>>
>>>> [1]: https://linux.die.net/man/3/nftw
>>>> [2]: https://github.com/kdave/btrfs-progs/commit/c6464d3f99ed1dabceff1168eabb207492c37624
>>>> [3]: https://lists.yoctoproject.org/g/yocto/message/64889
>>>>
>>>>
>>>> ---
>>>> v1: https://lore.kernel.org/yocto-patches/20250317113445.3855518-1-skandigraun@gmail.com/T/
>>>>
>>>> changes in v2:
>>>> - Add wrapper for ftw, ftw64, nftw and nftw64 (instead of only nftw in v1). All of them
>>>> use the same wrapper file, using macros to account for the naming differences.
>>>> - ftw64 and nftw64 depend on large file support of the system. To account for this, move
>>>> these implementations into their own subport folder, and compile them conditionally.
>>>> - Move tests into separate commit (and add some tests for the new calls too)
>>>> - The original implementation didn't consider multiple concurrent requests
>>>> when it was saving the original call's details (callback function pointer,
>>>> flags), and subsequent calls could overwrite data. This version stores
>>>> these details in an array that behaves similar to a LIFO proto-stack.
>>>> - Fix one bug in FTW_CHDIR flag behavior in conjuction with FTW_DEPTH flag (it switched
>>>> to a folder, which didn't match glibc implementation's behavior)
>>>> - Minor change in Makefile: since the tests between ftw/ftw64 and nftw/nftw64 are exactly
>>>> the same, they are also implemented in the same file, which however is expected to be
>>>> included in other files, and is not a compile-unit on its own. For this, the Makefile
>>>> looks for files with "test-" prefix in the test folder.
>>>>
>>>> ---
>>>>
>>>> Gyorgy Sarvari (2):
>>>> nftw, ftw: add wrapper
>>>> nftw, ftw: add tests
>>>>
>>>> Makefile.in | 4 +-
>>>> guts/README | 6 +-
>>>> ports/linux/guts/ftw64.c | 16 --
>>>> ports/linux/nftw64/guts/ftw64.c | 29 +++
>>>> ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
>>>> ports/linux/nftw64/pseudo_wrappers.c | 45 +++++
>>>> ports/linux/nftw64/wrapfuncs.in | 2 +
>>>> ports/linux/subports | 14 ++
>>>> ports/linux/wrapfuncs.in | 2 -
>>>> ports/unix/guts/ftw.c | 13 +-
>>>> ports/unix/guts/nftw.c | 7 +-
>>>> ports/unix/guts/nftw_wrapper_base.c | 211 ++++++++++++++++++++++
>>>> ports/unix/pseudo_wrappers.c | 45 +++++
>>>> ports/unix/wrapfuncs.in | 2 +-
>>>> test/ftw-test-impl.c | 226 +++++++++++++++++++++++
>>>> test/nftw-test-impl.c | 236 +++++++++++++++++++++++++
>>>> test/test-ftw.c | 4 +
>>>> test/test-ftw64.c | 4 +
>>>> test/test-nftw.c | 4 +
>>>> test/test-nftw.sh | 84 +++++++++
>>>> test/test-nftw64.c | 4 +
>>>> 21 files changed, 937 insertions(+), 28 deletions(-)
>>>> delete mode 100644 ports/linux/guts/ftw64.c
>>>> create mode 100644 ports/linux/nftw64/guts/ftw64.c
>>>> rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
>>>> create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
>>>> create mode 100644 ports/linux/nftw64/wrapfuncs.in
>>>> create mode 100644 ports/unix/guts/nftw_wrapper_base.c
>>>> create mode 100644 test/ftw-test-impl.c
>>>> create mode 100644 test/nftw-test-impl.c
>>>> create mode 100644 test/test-ftw.c
>>>> create mode 100644 test/test-ftw64.c
>>>> create mode 100644 test/test-nftw.c
>>>> create mode 100755 test/test-nftw.sh
>>>> create mode 100644 test/test-nftw64.c
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#1476): https://lists.yoctoproject.org/g/yocto-patches/message/1476
> Mute This Topic: https://lists.yoctoproject.org/mt/112139640/6084445
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/14038302/6084445/1344600526/xyzzy [skandigraun@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers
2025-05-02 8:53 ` Gyorgy Sarvari
@ 2025-05-02 13:33 ` Mark Hatle
0 siblings, 0 replies; 13+ messages in thread
From: Mark Hatle @ 2025-05-02 13:33 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock, Richard Purdie, fntoth
On 5/2/25 3:53 AM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
> Thank you very much for spending time on this. Some comments inline.
>
> On 5/2/25 03:17, Mark Hatle wrote:
>> [resend] my CC was incomplete and I want to make sure everyone interested is
>> aware of the review and status
>>
>> + skandigraun@gmail.com
>> + fntoth@gmail.com
>>
>>
>> I'm sorry this took as long as it did to review. I've been able to rework some
>> things, but the test cases need to be reworked. They don't work properly for me.
>>
>> I've posted my work-in-progress pseudo tree to:
>>
>> https://github.com/mhatle/pseudo/tree/master-next
>>
>> The top three commits (I split and reworked patch 1/2, 2/2 has minor rework in
>> it, but doesn't function properly) are the current version of this code:
>>
>> Move ftw and ftw64 to calling ntfw and nftw64:
>>
>> https://github.com/mhatle/pseudo/commit/be112c56767d2b149403ddb84bf99fa8efc35797
>>
>> The above is based on your investigation, but changes the way the call through
>> works.
>>
>> There is no need to declare a special type for the function, instead typecasting
>> to (void *) will avoid the compiler warning and work the same way. (I took this
>> idea from glibc, which uses void * for the same reason.). And as your note
> ACK.
>> already indicated, the cast "should work", it seems wrong, but glibc does this.
> :D
>>
>>
>> nftw, nftw64: add wrapper:
>>
>> https://github.com/mhatle/pseudo/commit/6bb390832553e7d112fd2375e27f8cebdb9e2acd
>>
>> I reworked a good chunk of this, but it was only for integration not functionality.
>>
>> Your initial version included a full copy of the wrapper code. This is
>> unnecessary as the system will already generate this code. Then the 'guts'
>> function simply needs to call your implementation. The typical way we've done
>> this in the past is tot implment a 'pseudo_func' function that holds the custom
>> work. This way the automatic ports just call it.
> Understood, thanks for the explanation.
>>
>> Your solution for the nftw_wrapper_base.c appears to be working well. I did
>> change a few things (moved many of the defines into that C file for instance)
>> and I resolved compiler warnings about return codes being ignored on chdir and
>> fchdir. All in all I checked and I don't believe I changed how this functions,
>> just the integration details.
> ACK. Yes, at the first sight the main things haven't changed.
>>
>> Note I did drop the part of the commit:
>>
>> +# nftw64 (and its deprecated pair, ftw64) are only present in case large
>> +# file support is present.
>> +cat > dummy.c <<EOF
>> +#define _GNU_SOURCE
>> +#include <features.h>
>> +#ifndef __USE_LARGEFILE64
>> +#error "no large file support"
>> +#endif
>> +int i;
>> +EOF
>> +if ${CC} -c -o dummy.o dummy.c >/dev/null 2>&1; then
>> + echo linux/nftw64
>> +fi
>> +rm -f dummy.c dummy.o
>>
>> Since nftw64 is already in the wrapfuncs.in it should be unnecessary. However,
>> if we need to make this dynamic, the right way is to add your change BACK into
>> the system and remove it from wrapfuncs.in. If this is the case, I'd like to
>> make it a separate commit so we're clear and what we're doing.
> I have no strong feelings about it. If we want to be pedantic, I think
> it should be dynamic, on the other hand I have to admit that if the lack
> of this check would have caused any real world problems, it would have
> showed up in the past decade or two. I'm okay with not making it dynamic.
>>
>>
>> ftw, nftw, ftw64 and nftw64: add tests:
>>
>> https://github.com/mhatle/pseudo/commit/86d228c1308949b1917cdcd1feae1fd8b08457ea
>>
>> This commit I could not get working. I attempted to diagnose it, and never did
>> figure out what was wrong.
>>
>> One change I did make was to avoid stopping in the case of a failure, but
>> running all of the test cases so we got a full list of information from the test
>> run. I'm running these tests by doing:
>>
>> ./configure --prefix=`pwd`/foo
>> make
>> make test
>>
>> The output I get from the test cases is:
>>
>> Incorrect return value. Expected: 0, actual: 2
>> test-nftw: nftw subtree skipping with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping chddir with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 2
>> test-nftw: nftw64 subtree skipping with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw64 sibling skipping with pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Incorrect return value. Expected: 0, actual: 1
>> test-nftw: ftw non-recursive walking with pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Recursive call failed
>> test-nftw: ftw recursive walking with pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Incorrect return value. Expected: 0, actual: 1
>> test-nftw: ftw64 non-recursive walking with pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Recursive call failed
>> test-nftw: ftw64 recursive walking with pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 2
>> test-nftw: nftw subtree skipping without pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping without pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping chdir without pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 2
>> test-nftw: nftw subtree skipping without pseudo: Failed
>> Incorrect return value. Expected: 0, actual: 3
>> test-nftw: nftw sibling skipping without pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Incorrect return value. Expected: 0, actual: 1
>> test-nftw: ftw non-recursive walking without pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Recursive call failed
>> test-nftw: ftw recursive walking without pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Incorrect return value. Expected: 0, actual: 1
>> test-nftw: ftw non-recursive walking without pseudo: Failed
>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>> alking/a1/b1/c1/file3
>> Recursive call failed
>> test-nftw: ftw recursive walking without pseudo: Failed
>> test-nftw: Failed.
>>
>>
>> Can you take a look at the test cases, I don't really understand them very well.
>> Some of this stuff looks like it could be off-by-1 errors both in the
>> 'Expected' and 'Actual' side of things. As for the other failures, I'm really
>> not sure everything that is going on here and I figured it might be more obvious
>> to someone else.
> I suspect that I (and you also) became the victim of my own naiveness,
> and the tests I created depend on the underlying FS type more than I
> expected. I ran the tests only on ext4, but I guess that you are using a
> different FS than I do, and that's why it fails (the tests expect to
> encounter the entries in a specific order).
> Going to come up with some FS- and fileorder-agnostic implementation.
I'm just running on an Ubuntu 22.04 with ext4 filesystem. But ya, I suspect
ordering or something else is the issue.
> Do you have any preference about the patch for the tests? Should I open
> a PR in your github repo, or just send a patch on top of it here? Maybe
> something else?
Sending it here is best. This gives others an opportunity to review the changes
as well.
(speaking of which, I will be sending the 2 that I believe work to the list for
completion today. I just ran out of time yesterday!)
If you agree the other two of them are correct, then just focus on the test case
patch (unless of course bugs are found and we need to fix them.)
The 'master-next' branch is just that, a work in progress, so I expect some
non-fast-forward commits there to fix these issues. I'd like to try to get them
checked in in working chunks to make it easier to bisect if problems are found
in the future.
Ohh as an FYI, when you do run the test cases, you will currently see errors in
additional test cases. These are known issues:
The 'test-parallel-rename' and 'test-parallel-symlinks' will absolutely fail
right now, know bugs in pseudo.
Additionally, tetst-acl and test-xattr test cases may not work depending on
environment.
--Mark
>> --Mark
>>
>>
>> On 4/7/25 6:11 PM, Mark Hatle wrote:
>>> Thank you for the additional items. I'm going to attempt to review this in the
>>> next day or so and I'll reply with my findings. On the surface it looks a bit
>>> more complicated then I originally thought it would. But that may very well be
>>> needed.
>>>
>>> --Mark
>>>
>>> On 4/7/25 2:14 PM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>>>> This is an (attempt at the) implementation of the nftw and ftw[1] calls in pseudo.
>>>>
>>>> The main motivation is a change in btrfs-tools[2], in which
>>>> they have changed from walking the filetree and calling stat
>>>> on each entries separately to using the nftw call. As a result,
>>>> btrfs filesystem generation currently happens with incorrect
>>>> ownership details, as the nftw calls go around pseudo.
>>>>
>>>> See also the relevant report[3] on the Yocto mailing list.
>>>>
>>>> The main idea: with a custom wrapper capture the nftw call, and switch the callback
>>>> to our own callback. When our own callback is called, fix the received
>>>> stat struct, and call the original callback, this time with the correct
>>>> stat struct.
>>>>
>>>> Big thanks to Lander Van Loock, who not only reported the
>>>> original issue, but also helped testing and reviewing the first version
>>>> of the code.
>>>>
>>>> Please let me know what you think.
>>>>
>>>> [1]: https://linux.die.net/man/3/nftw
>>>> [2]: https://github.com/kdave/btrfs-progs/commit/c6464d3f99ed1dabceff1168eabb207492c37624
>>>> [3]: https://lists.yoctoproject.org/g/yocto/message/64889
>>>>
>>>>
>>>> ---
>>>> v1: https://lore.kernel.org/yocto-patches/20250317113445.3855518-1-skandigraun@gmail.com/T/
>>>>
>>>> changes in v2:
>>>> - Add wrapper for ftw, ftw64, nftw and nftw64 (instead of only nftw in v1). All of them
>>>> use the same wrapper file, using macros to account for the naming differences.
>>>> - ftw64 and nftw64 depend on large file support of the system. To account for this, move
>>>> these implementations into their own subport folder, and compile them conditionally.
>>>> - Move tests into separate commit (and add some tests for the new calls too)
>>>> - The original implementation didn't consider multiple concurrent requests
>>>> when it was saving the original call's details (callback function pointer,
>>>> flags), and subsequent calls could overwrite data. This version stores
>>>> these details in an array that behaves similar to a LIFO proto-stack.
>>>> - Fix one bug in FTW_CHDIR flag behavior in conjuction with FTW_DEPTH flag (it switched
>>>> to a folder, which didn't match glibc implementation's behavior)
>>>> - Minor change in Makefile: since the tests between ftw/ftw64 and nftw/nftw64 are exactly
>>>> the same, they are also implemented in the same file, which however is expected to be
>>>> included in other files, and is not a compile-unit on its own. For this, the Makefile
>>>> looks for files with "test-" prefix in the test folder.
>>>>
>>>> ---
>>>>
>>>> Gyorgy Sarvari (2):
>>>> nftw, ftw: add wrapper
>>>> nftw, ftw: add tests
>>>>
>>>> Makefile.in | 4 +-
>>>> guts/README | 6 +-
>>>> ports/linux/guts/ftw64.c | 16 --
>>>> ports/linux/nftw64/guts/ftw64.c | 29 +++
>>>> ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
>>>> ports/linux/nftw64/pseudo_wrappers.c | 45 +++++
>>>> ports/linux/nftw64/wrapfuncs.in | 2 +
>>>> ports/linux/subports | 14 ++
>>>> ports/linux/wrapfuncs.in | 2 -
>>>> ports/unix/guts/ftw.c | 13 +-
>>>> ports/unix/guts/nftw.c | 7 +-
>>>> ports/unix/guts/nftw_wrapper_base.c | 211 ++++++++++++++++++++++
>>>> ports/unix/pseudo_wrappers.c | 45 +++++
>>>> ports/unix/wrapfuncs.in | 2 +-
>>>> test/ftw-test-impl.c | 226 +++++++++++++++++++++++
>>>> test/nftw-test-impl.c | 236 +++++++++++++++++++++++++
>>>> test/test-ftw.c | 4 +
>>>> test/test-ftw64.c | 4 +
>>>> test/test-nftw.c | 4 +
>>>> test/test-nftw.sh | 84 +++++++++
>>>> test/test-nftw64.c | 4 +
>>>> 21 files changed, 937 insertions(+), 28 deletions(-)
>>>> delete mode 100644 ports/linux/guts/ftw64.c
>>>> create mode 100644 ports/linux/nftw64/guts/ftw64.c
>>>> rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
>>>> create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
>>>> create mode 100644 ports/linux/nftw64/wrapfuncs.in
>>>> create mode 100644 ports/unix/guts/nftw_wrapper_base.c
>>>> create mode 100644 test/ftw-test-impl.c
>>>> create mode 100644 test/nftw-test-impl.c
>>>> create mode 100644 test/test-ftw.c
>>>> create mode 100644 test/test-ftw64.c
>>>> create mode 100644 test/test-nftw.c
>>>> create mode 100755 test/test-nftw.sh
>>>> create mode 100644 test/test-nftw64.c
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#1476): https://lists.yoctoproject.org/g/yocto-patches/message/1476
> Mute This Topic: https://lists.yoctoproject.org/mt/112139640/3616948
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers
2025-05-02 9:53 ` Gyorgy Sarvari
@ 2025-05-02 13:42 ` Mark Hatle
0 siblings, 0 replies; 13+ messages in thread
From: Mark Hatle @ 2025-05-02 13:42 UTC (permalink / raw)
To: yocto-patches; +Cc: landervanloock, Richard Purdie, fntoth
On 5/2/25 4:53 AM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
> On 5/2/25 10:53, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>> Thank you very much for spending time on this. Some comments inline.
>>
>> On 5/2/25 03:17, Mark Hatle wrote:
>>> [resend] my CC was incomplete and I want to make sure everyone interested is
>>> aware of the review and status
>>>
>>> + skandigraun@gmail.com
>>> + fntoth@gmail.com
>>>
>>>
>>> I'm sorry this took as long as it did to review. I've been able to rework some
>>> things, but the test cases need to be reworked. They don't work properly for me.
>>>
>>> I've posted my work-in-progress pseudo tree to:
>>>
>>> https://github.com/mhatle/pseudo/tree/master-next
>>>
>>> The top three commits (I split and reworked patch 1/2, 2/2 has minor rework in
>>> it, but doesn't function properly) are the current version of this code:
>>>
>>> Move ftw and ftw64 to calling ntfw and nftw64:
>>>
>>> https://github.com/mhatle/pseudo/commit/be112c56767d2b149403ddb84bf99fa8efc35797
>>>
>>> The above is based on your investigation, but changes the way the call through
>>> works.
>>>
>>> There is no need to declare a special type for the function, instead typecasting
>>> to (void *) will avoid the compiler warning and work the same way. (I took this
>>> idea from glibc, which uses void * for the same reason.). And as your note
>> ACK.
>>> already indicated, the cast "should work", it seems wrong, but glibc does this.
>> :D
>>>
>>> nftw, nftw64: add wrapper:
>>>
>>> https://github.com/mhatle/pseudo/commit/6bb390832553e7d112fd2375e27f8cebdb9e2acd
>>>
>>> I reworked a good chunk of this, but it was only for integration not functionality.
>>>
>>> Your initial version included a full copy of the wrapper code. This is
>>> unnecessary as the system will already generate this code. Then the 'guts'
>>> function simply needs to call your implementation. The typical way we've done
>>> this in the past is tot implment a 'pseudo_func' function that holds the custom
>>> work. This way the automatic ports just call it.
>> Understood, thanks for the explanation.
>>> Your solution for the nftw_wrapper_base.c appears to be working well. I did
>>> change a few things (moved many of the defines into that C file for instance)
>>> and I resolved compiler warnings about return codes being ignored on chdir and
>>> fchdir. All in all I checked and I don't believe I changed how this functions,
>>> just the integration details.
>> ACK. Yes, at the first sight the main things haven't changed.
> There would be 2 things I have noticed since in the nftw_wrappers_base.c
> file:
> 1. Some comments became outdated in the code, they can be removed:
> Line 117 - The comment says no error checking is done, but you have
> added some.
> Line 183 - This comment was useful when the next statement was in a
> separate guts file. It became somewhat misleading, and can be deleted.
Good catch, I overlooked this and I'll get it corrected.
> 2. in "static int NFTW_CALLBACK_NAME(...)" function you have introduced
> pseudo_sb variable, instead of reusing the *sb pointer from the
I forgot to mention this. This was due to two reasons, first the compiler
warned we were changing type from const struct *sb to struct *sb.
This lead me to investigate the way the memory was used, the struct passed into
the callback should be const as the caller (glibc) not only created the memory
but is responsible for cleaning it up. I found indications in the code that the
memory was re-used and prior state may be as well.
So if we re-use 'sb' (drop the cost), we could end up breaking something from
the glibc function. So it was safer to declare a new variable and use it (even
if it is slower.)
> arguments. Generally it's fine, however it might stay at its initial
> state if the walked entry is unreadable (line 148). I *believe* that
I always forget this detail. I know global and static are initialized to zeros
(unless otherwise defined), but I forgot about the local function declaration of
the array. I'll add a memset and get this zero'd out, good catch.
> even in this erroneous case the original sb struct contains some info,
> like device ID or inode number (though I would have to verify to be sure
> what's inside), which shouldn't be lost.
It's really down the our stat call can arbitrarily modify things, and this could
in-turn break the sb. (this is all theoretical of course, I suspect as you say
it's fine in practice.)
> The current state also made the corresponding comment outdated in line
> 146 - though I think the comment isn't inherently bad (until proven
> otherwise). What if sb would be copied to pseudo_sb in the else branch?
> Or remove the condition completely (along with the comment)?
I think the comment is fine, but I'll add that for safety we'd defining it locally.
Hopefully in a few hours I can get those changes made and I'll update everyone.
--Mark
>>> Note I did drop the part of the commit:
>>>
>>> +# nftw64 (and its deprecated pair, ftw64) are only present in case large
>>> +# file support is present.
>>> +cat > dummy.c <<EOF
>>> +#define _GNU_SOURCE
>>> +#include <features.h>
>>> +#ifndef __USE_LARGEFILE64
>>> +#error "no large file support"
>>> +#endif
>>> +int i;
>>> +EOF
>>> +if ${CC} -c -o dummy.o dummy.c >/dev/null 2>&1; then
>>> + echo linux/nftw64
>>> +fi
>>> +rm -f dummy.c dummy.o
>>>
>>> Since nftw64 is already in the wrapfuncs.in it should be unnecessary. However,
>>> if we need to make this dynamic, the right way is to add your change BACK into
>>> the system and remove it from wrapfuncs.in. If this is the case, I'd like to
>>> make it a separate commit so we're clear and what we're doing.
>> I have no strong feelings about it. If we want to be pedantic, I think
>> it should be dynamic, on the other hand I have to admit that if the lack
>> of this check would have caused any real world problems, it would have
>> showed up in the past decade or two. I'm okay with not making it dynamic.
>>>
>>> ftw, nftw, ftw64 and nftw64: add tests:
>>>
>>> https://github.com/mhatle/pseudo/commit/86d228c1308949b1917cdcd1feae1fd8b08457ea
>>>
>>> This commit I could not get working. I attempted to diagnose it, and never did
>>> figure out what was wrong.
>>>
>>> One change I did make was to avoid stopping in the case of a failure, but
>>> running all of the test cases so we got a full list of information from the test
>>> run. I'm running these tests by doing:
>>>
>>> ./configure --prefix=`pwd`/foo
>>> make
>>> make test
>>>
>>> The output I get from the test cases is:
>>>
>>> Incorrect return value. Expected: 0, actual: 2
>>> test-nftw: nftw subtree skipping with pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 3
>>> test-nftw: nftw sibling skipping with pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 3
>>> test-nftw: nftw sibling skipping chddir with pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 2
>>> test-nftw: nftw64 subtree skipping with pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 3
>>> test-nftw: nftw64 sibling skipping with pseudo: Failed
>>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>>> alking/a1/b1/c1/file3
>>> Incorrect return value. Expected: 0, actual: 1
>>> test-nftw: ftw non-recursive walking with pseudo: Failed
>>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>>> alking/a1/b1/c1/file3
>>> Recursive call failed
>>> test-nftw: ftw recursive walking with pseudo: Failed
>>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>>> alking/a1/b1/c1/file3
>>> Incorrect return value. Expected: 0, actual: 1
>>> test-nftw: ftw64 non-recursive walking with pseudo: Failed
>>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>>> alking/a1/b1/c1/file3
>>> Recursive call failed
>>> test-nftw: ftw64 recursive walking with pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 2
>>> test-nftw: nftw subtree skipping without pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 3
>>> test-nftw: nftw sibling skipping without pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 3
>>> test-nftw: nftw sibling skipping chdir without pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 2
>>> test-nftw: nftw subtree skipping without pseudo: Failed
>>> Incorrect return value. Expected: 0, actual: 3
>>> test-nftw: nftw sibling skipping without pseudo: Failed
>>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>>> alking/a1/b1/c1/file3
>>> Incorrect return value. Expected: 0, actual: 1
>>> test-nftw: ftw non-recursive walking without pseudo: Failed
>>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>>> alking/a1/b1/c1/file3
>>> Recursive call failed
>>> test-nftw: ftw recursive walking without pseudo: Failed
>>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>>> alking/a1/b1/c1/file3
>>> Incorrect return value. Expected: 0, actual: 1
>>> test-nftw: ftw non-recursive walking without pseudo: Failed
>>> Incorrect fpath received. Expected: walking/a1/b1/c1/file, actual:
>>> alking/a1/b1/c1/file3
>>> Recursive call failed
>>> test-nftw: ftw recursive walking without pseudo: Failed
>>> test-nftw: Failed.
>>>
>>>
>>> Can you take a look at the test cases, I don't really understand them very well.
>>> Some of this stuff looks like it could be off-by-1 errors both in the
>>> 'Expected' and 'Actual' side of things. As for the other failures, I'm really
>>> not sure everything that is going on here and I figured it might be more obvious
>>> to someone else.
>> I suspect that I (and you also) became the victim of my own naiveness,
>> and the tests I created depend on the underlying FS type more than I
>> expected. I ran the tests only on ext4, but I guess that you are using a
>> different FS than I do, and that's why it fails (the tests expect to
>> encounter the entries in a specific order).
>> Going to come up with some FS- and fileorder-agnostic implementation.
>>
>> Do you have any preference about the patch for the tests? Should I open
>> a PR in your github repo, or just send a patch on top of it here? Maybe
>> something else?
>>> --Mark
>>>
>>>
>>> On 4/7/25 6:11 PM, Mark Hatle wrote:
>>>> Thank you for the additional items. I'm going to attempt to review this in the
>>>> next day or so and I'll reply with my findings. On the surface it looks a bit
>>>> more complicated then I originally thought it would. But that may very well be
>>>> needed.
>>>>
>>>> --Mark
>>>>
>>>> On 4/7/25 2:14 PM, Gyorgy Sarvari via lists.yoctoproject.org wrote:
>>>>> This is an (attempt at the) implementation of the nftw and ftw[1] calls in pseudo.
>>>>>
>>>>> The main motivation is a change in btrfs-tools[2], in which
>>>>> they have changed from walking the filetree and calling stat
>>>>> on each entries separately to using the nftw call. As a result,
>>>>> btrfs filesystem generation currently happens with incorrect
>>>>> ownership details, as the nftw calls go around pseudo.
>>>>>
>>>>> See also the relevant report[3] on the Yocto mailing list.
>>>>>
>>>>> The main idea: with a custom wrapper capture the nftw call, and switch the callback
>>>>> to our own callback. When our own callback is called, fix the received
>>>>> stat struct, and call the original callback, this time with the correct
>>>>> stat struct.
>>>>>
>>>>> Big thanks to Lander Van Loock, who not only reported the
>>>>> original issue, but also helped testing and reviewing the first version
>>>>> of the code.
>>>>>
>>>>> Please let me know what you think.
>>>>>
>>>>> [1]: https://linux.die.net/man/3/nftw
>>>>> [2]: https://github.com/kdave/btrfs-progs/commit/c6464d3f99ed1dabceff1168eabb207492c37624
>>>>> [3]: https://lists.yoctoproject.org/g/yocto/message/64889
>>>>>
>>>>>
>>>>> ---
>>>>> v1: https://lore.kernel.org/yocto-patches/20250317113445.3855518-1-skandigraun@gmail.com/T/
>>>>>
>>>>> changes in v2:
>>>>> - Add wrapper for ftw, ftw64, nftw and nftw64 (instead of only nftw in v1). All of them
>>>>> use the same wrapper file, using macros to account for the naming differences.
>>>>> - ftw64 and nftw64 depend on large file support of the system. To account for this, move
>>>>> these implementations into their own subport folder, and compile them conditionally.
>>>>> - Move tests into separate commit (and add some tests for the new calls too)
>>>>> - The original implementation didn't consider multiple concurrent requests
>>>>> when it was saving the original call's details (callback function pointer,
>>>>> flags), and subsequent calls could overwrite data. This version stores
>>>>> these details in an array that behaves similar to a LIFO proto-stack.
>>>>> - Fix one bug in FTW_CHDIR flag behavior in conjuction with FTW_DEPTH flag (it switched
>>>>> to a folder, which didn't match glibc implementation's behavior)
>>>>> - Minor change in Makefile: since the tests between ftw/ftw64 and nftw/nftw64 are exactly
>>>>> the same, they are also implemented in the same file, which however is expected to be
>>>>> included in other files, and is not a compile-unit on its own. For this, the Makefile
>>>>> looks for files with "test-" prefix in the test folder.
>>>>>
>>>>> ---
>>>>>
>>>>> Gyorgy Sarvari (2):
>>>>> nftw, ftw: add wrapper
>>>>> nftw, ftw: add tests
>>>>>
>>>>> Makefile.in | 4 +-
>>>>> guts/README | 6 +-
>>>>> ports/linux/guts/ftw64.c | 16 --
>>>>> ports/linux/nftw64/guts/ftw64.c | 29 +++
>>>>> ports/linux/{ => nftw64}/guts/nftw64.c | 7 +-
>>>>> ports/linux/nftw64/pseudo_wrappers.c | 45 +++++
>>>>> ports/linux/nftw64/wrapfuncs.in | 2 +
>>>>> ports/linux/subports | 14 ++
>>>>> ports/linux/wrapfuncs.in | 2 -
>>>>> ports/unix/guts/ftw.c | 13 +-
>>>>> ports/unix/guts/nftw.c | 7 +-
>>>>> ports/unix/guts/nftw_wrapper_base.c | 211 ++++++++++++++++++++++
>>>>> ports/unix/pseudo_wrappers.c | 45 +++++
>>>>> ports/unix/wrapfuncs.in | 2 +-
>>>>> test/ftw-test-impl.c | 226 +++++++++++++++++++++++
>>>>> test/nftw-test-impl.c | 236 +++++++++++++++++++++++++
>>>>> test/test-ftw.c | 4 +
>>>>> test/test-ftw64.c | 4 +
>>>>> test/test-nftw.c | 4 +
>>>>> test/test-nftw.sh | 84 +++++++++
>>>>> test/test-nftw64.c | 4 +
>>>>> 21 files changed, 937 insertions(+), 28 deletions(-)
>>>>> delete mode 100644 ports/linux/guts/ftw64.c
>>>>> create mode 100644 ports/linux/nftw64/guts/ftw64.c
>>>>> rename ports/linux/{ => nftw64}/guts/nftw64.c (57%)
>>>>> create mode 100644 ports/linux/nftw64/pseudo_wrappers.c
>>>>> create mode 100644 ports/linux/nftw64/wrapfuncs.in
>>>>> create mode 100644 ports/unix/guts/nftw_wrapper_base.c
>>>>> create mode 100644 test/ftw-test-impl.c
>>>>> create mode 100644 test/nftw-test-impl.c
>>>>> create mode 100644 test/test-ftw.c
>>>>> create mode 100644 test/test-ftw64.c
>>>>> create mode 100644 test/test-nftw.c
>>>>> create mode 100755 test/test-nftw.sh
>>>>> create mode 100644 test/test-nftw64.c
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>
>>
>>
>>
>>
>
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#1477): https://lists.yoctoproject.org/g/yocto-patches/message/1477
> Mute This Topic: https://lists.yoctoproject.org/mt/112139640/3616948
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13201099/3616948/947757854/xyzzy [mark.hatle@kernel.crashing.org]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-05-02 13:44 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-07 19:14 [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers Gyorgy Sarvari
2025-04-07 19:14 ` [pseudo][PATCH v2 1/2] nftw, ftw: add wrapper Gyorgy Sarvari
2025-04-07 19:14 ` [pseudo][PATCH v2 2/2] nftw, ftw: add tests Gyorgy Sarvari
2025-04-07 23:11 ` [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers Mark Hatle
[not found] ` <18341F33308F8E0B.31078@lists.yoctoproject.org>
2025-04-08 14:33 ` [yocto-patches] [pseudo][PATCH v2 2/2] nftw, ftw: add tests Gyorgy Sarvari
2025-04-08 16:06 ` Mark Hatle
2025-04-26 21:14 ` Ferry Toth
[not found] ` <18342C3498EB800F.31078@lists.yoctoproject.org>
2025-05-02 1:08 ` [yocto-patches] [pseudo][PATCH v2 0/2] nftw, ftw: add wrappers Mark Hatle
2025-05-02 1:17 ` Mark Hatle
2025-05-02 8:53 ` Gyorgy Sarvari
2025-05-02 13:33 ` Mark Hatle
[not found] ` <183BA9D5657FA6B7.6015@lists.yoctoproject.org>
2025-05-02 9:53 ` Gyorgy Sarvari
2025-05-02 13:42 ` Mark Hatle
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.