Re: [RFC PATCH v3 4/8] selftests/fuse: factor-out test fixture setup/teardown

[Amir Goldstein]

On Mon, Sep 7, 2026 at 12:44 PM Luis Henriques <luis@xxxxxxxxxx> wrote:
>
> On Sat, Sep 05 2026, Amir Goldstein wrote:
>
> > On Fri, Sep 4, 2026 at 12:38 PM Luis Henriques <luis@xxxxxxxxxx> wrote:
> >>
> >> In order to reduce new tests setup/teardown code duplication, factor-out
> >> these functions from the existing acl_cache test into a new fuse_common.c
> >> file that can be reused in other tests.
> >>
> >> Signed-off-by: Luis Henriques <luis@xxxxxxxxxx>
> >> ---
> >>  .../selftests/filesystems/fuse/Makefile       |  8 ++-
> >>  .../filesystems/fuse/fuse_acl_cache_test.c    | 62 +++----------------
> >>  .../selftests/filesystems/fuse/fuse_common.c  | 60 ++++++++++++++++++
> >>  .../selftests/filesystems/fuse/fuse_common.h  | 25 ++++++++
> >>  4 files changed, 100 insertions(+), 55 deletions(-)
> >>  create mode 100644 tools/testing/selftests/filesystems/fuse/fuse_common.c
> >>  create mode 100644 tools/testing/selftests/filesystems/fuse/fuse_common.h
> >>
> >> diff --git a/tools/testing/selftests/filesystems/fuse/Makefile b/tools/testing/selftests/filesystems/fuse/Makefile
> >> index a3ee9b3a2f5d..7744f796eb06 100644
> >> --- a/tools/testing/selftests/filesystems/fuse/Makefile
> >> +++ b/tools/testing/selftests/filesystems/fuse/Makefile
> >> @@ -21,8 +21,12 @@ ifeq ($(VAR_LDLIBS),)
> >>  VAR_LDLIBS := -lfuse3 -pthread
> >>  endif
> >>
> >> +CFLAGS += $(VAR_CFLAGS)
> >> +LDLIBS += $(VAR_LDLIBS)
> >> +
> >>  $(OUTPUT)/fuse_mnt: CFLAGS += $(VAR_CFLAGS)
> >>  $(OUTPUT)/fuse_mnt: LDLIBS += $(VAR_LDLIBS)
> >>
> >> -$(OUTPUT)/fuse_acl_cache_test: CFLAGS += $(VAR_CFLAGS)
> >> -$(OUTPUT)/fuse_acl_cache_test: LDLIBS += $(VAR_LDLIBS)
> >> +$(OUTPUT)/fuse_acl_cache_test: fuse_common.c fuse_acl_cache_test.c
> >> +
> >> +EXTRA_CLEAN := fuse_common.o
> >> diff --git a/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c b/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c
> >> index 2411a6e285f1..12cbf9753d03 100644
> >> --- a/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c
> >> +++ b/tools/testing/selftests/filesystems/fuse/fuse_acl_cache_test.c
> >> @@ -33,23 +33,15 @@
> >>   */
> >>
> >>  #define _GNU_SOURCE
> >> -#include <errno.h>
> >>  #include <fcntl.h>
> >>  #include <linux/limits.h>
> >> -#include <pthread.h>
> >>  #include <stdint.h>
> >> -#include <stdio.h>
> >> -#include <stdlib.h>
> >> -#include <string.h>
> >> -#include <sys/stat.h>
> >>  #include <sys/xattr.h>
> >> -#include <unistd.h>
> >> -
> >> -#define FUSE_USE_VERSION 31
> >> -#include <fuse_lowlevel.h>
> >>
> >>  #include "kselftest_harness.h"
> >>
> >> +#include "fuse_common.h"
> >> +
> >>  /* ---- ACL binary encoding ------------------------------------------------ */
> >>  /*
> >>   * POSIX ACL v2 xattr format (little-endian):
> >> @@ -176,69 +168,33 @@ static const struct fuse_lowlevel_ops fs_ops = {
> >>         .getxattr = fs_getxattr,
> >>  };
> >>
> >> -/* ---- Daemon thread ------------------------------------------------------- */
> >> -
> >> -static void *run_daemon(void *arg)
> >> -{
> >> -       fuse_session_loop((struct fuse_session *)arg);
> >> -       return NULL;
> >> -}
> >> -
> >>  /* ---- kselftest harness --------------------------------------------------- */
> >>
> >>  FIXTURE(acl_cache) {
> >>         struct fuse_session *se;
> >> -       char                 mountpoint[PATH_MAX];
> >> +       char                 mountpoint[MOUNTPOINT_SZ];
> >>         char                 file_path[PATH_MAX];
> >>         pthread_t            thread;
> >>  };
> >
> > Nice!
> > I think it would be even nicer to have struct fuse_common_ctx
> > with the common members embedded in the per test state,
> > but I'll let you decide if you want to do that or not.
>
> Thanks for the suggestion.  It may make sense indeed to have that common
> struct, although it would probably only include the spinlock and a bool
> for enabling/disabling the cache.  I'll have a closer look and see if it
> makes sense.

I meant that it should include:
         struct fuse_session *se;
         char                 mountpoint[PATH_MAX];
         pthread_t            thread;

instead of passing them by reference individually to fs_setup/teardown()
not a big deal, just a bit nicer IMO.

Thanks,
Amir.