[PATCH RFC v3 3/3] fuse: use the newly created helper functions

horst@birthelmer.com posted 3 patches 1 month ago
There is a newer version of this series
[PATCH RFC v3 3/3] fuse: use the newly created helper functions
Posted by horst@birthelmer.com 1 month ago
From: Horst Birthelmer <hbirthelmer@ddn.com>

new helper functions are:
- fuse_getattr_args_fill()
- fuse_open_args_fill()

Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
---
 fs/fuse/dir.c  | 9 +--------
 fs/fuse/file.c | 9 +--------
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 4b6b3d2758ff..ca8b69282c60 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -1493,14 +1493,7 @@ static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
 		inarg.getattr_flags |= FUSE_GETATTR_FH;
 		inarg.fh = ff->fh;
 	}
-	args.opcode = FUSE_GETATTR;
-	args.nodeid = get_node_id(inode);
-	args.in_numargs = 1;
-	args.in_args[0].size = sizeof(inarg);
-	args.in_args[0].value = &inarg;
-	args.out_numargs = 1;
-	args.out_args[0].size = sizeof(outarg);
-	args.out_args[0].value = &outarg;
+	fuse_getattr_args_fill(&args, get_node_id(inode), &inarg, &outarg);
 	err = fuse_simple_request(fm, &args);
 	if (!err) {
 		if (fuse_invalid_attr(&outarg.attr) ||
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index 676f6bfde9f8..c0375b32967d 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -73,14 +73,7 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
 		inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID;
 	}
 
-	args.opcode = opcode;
-	args.nodeid = nodeid;
-	args.in_numargs = 1;
-	args.in_args[0].size = sizeof(inarg);
-	args.in_args[0].value = &inarg;
-	args.out_numargs = 1;
-	args.out_args[0].size = sizeof(*outargp);
-	args.out_args[0].value = outargp;
+	fuse_open_args_fill(&args, nodeid, opcode, &inarg, outargp);
 
 	return fuse_simple_request(fm, &args);
 }

-- 
2.51.0
Re: [PATCH RFC v3 3/3] fuse: use the newly created helper functions
Posted by Joanne Koong 1 month ago
On Thu, Jan 8, 2026 at 6:23 AM <horst@birthelmer.com> wrote:
>
> From: Horst Birthelmer <hbirthelmer@ddn.com>
>
> new helper functions are:
> - fuse_getattr_args_fill()
> - fuse_open_args_fill()
>
> Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> ---
>  fs/fuse/dir.c  | 9 +--------
>  fs/fuse/file.c | 9 +--------
>  2 files changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 4b6b3d2758ff..ca8b69282c60 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -1493,14 +1493,7 @@ static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
>                 inarg.getattr_flags |= FUSE_GETATTR_FH;
>                 inarg.fh = ff->fh;
>         }
> -       args.opcode = FUSE_GETATTR;
> -       args.nodeid = get_node_id(inode);
> -       args.in_numargs = 1;
> -       args.in_args[0].size = sizeof(inarg);
> -       args.in_args[0].value = &inarg;
> -       args.out_numargs = 1;
> -       args.out_args[0].size = sizeof(outarg);
> -       args.out_args[0].value = &outarg;
> +       fuse_getattr_args_fill(&args, get_node_id(inode), &inarg, &outarg);
>         err = fuse_simple_request(fm, &args);
>         if (!err) {
>                 if (fuse_invalid_attr(&outarg.attr) ||
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 676f6bfde9f8..c0375b32967d 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -73,14 +73,7 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
>                 inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID;
>         }
>
> -       args.opcode = opcode;
> -       args.nodeid = nodeid;
> -       args.in_numargs = 1;
> -       args.in_args[0].size = sizeof(inarg);
> -       args.in_args[0].value = &inarg;
> -       args.out_numargs = 1;
> -       args.out_args[0].size = sizeof(*outargp);
> -       args.out_args[0].value = outargp;
> +       fuse_open_args_fill(&args, nodeid, opcode, &inarg, outargp);
>
>         return fuse_simple_request(fm, &args);
>  }
>

This is a very minor nit but imo the split is a bit nicer if patch 2/3
is this patch with your helper changes:

+/*
+ * Helper function to initialize fuse_args for OPEN/OPENDIR operations
+ */
+void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
+ struct fuse_open_in *inarg, struct fuse_open_out *outarg)
+{
+ args->opcode = opcode;
...
+}
+
+/*
+ * Helper function to initialize fuse_args for GETATTR operations
+ */
+void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
+     struct fuse_getattr_in *inarg,
+     struct fuse_attr_out *outarg)
+{
+ args->opcode = FUSE_GETATTR;
...
+}
+

and then patch 3 is your open+getattr changes. That way, it's easier
to see that the changes in this patch to fuse_do_getattr() and
fuse_send_open() have no functional changes in logic.


Thanks,
Joanne
> --
> 2.51.0
>
Re: Re: [PATCH RFC v3 3/3] fuse: use the newly created helper functions
Posted by Horst Birthelmer 1 month ago
On Thu, Jan 08, 2026 at 02:19:10PM -0800, Joanne Koong wrote:
> On Thu, Jan 8, 2026 at 6:23 AM <horst@birthelmer.com> wrote:
> >
> > From: Horst Birthelmer <hbirthelmer@ddn.com>
> >
> > new helper functions are:
> > - fuse_getattr_args_fill()
> > - fuse_open_args_fill()
> >
> > Signed-off-by: Horst Birthelmer <hbirthelmer@ddn.com>
> > ---
> >  fs/fuse/dir.c  | 9 +--------
> >  fs/fuse/file.c | 9 +--------
> >  2 files changed, 2 insertions(+), 16 deletions(-)
> >
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index 4b6b3d2758ff..ca8b69282c60 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -1493,14 +1493,7 @@ static int fuse_do_getattr(struct mnt_idmap *idmap, struct inode *inode,
> >                 inarg.getattr_flags |= FUSE_GETATTR_FH;
> >                 inarg.fh = ff->fh;
> >         }
> > -       args.opcode = FUSE_GETATTR;
> > -       args.nodeid = get_node_id(inode);
> > -       args.in_numargs = 1;
> > -       args.in_args[0].size = sizeof(inarg);
> > -       args.in_args[0].value = &inarg;
> > -       args.out_numargs = 1;
> > -       args.out_args[0].size = sizeof(outarg);
> > -       args.out_args[0].value = &outarg;
> > +       fuse_getattr_args_fill(&args, get_node_id(inode), &inarg, &outarg);
> >         err = fuse_simple_request(fm, &args);
> >         if (!err) {
> >                 if (fuse_invalid_attr(&outarg.attr) ||
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index 676f6bfde9f8..c0375b32967d 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -73,14 +73,7 @@ static int fuse_send_open(struct fuse_mount *fm, u64 nodeid,
> >                 inarg.open_flags |= FUSE_OPEN_KILL_SUIDGID;
> >         }
> >
> > -       args.opcode = opcode;
> > -       args.nodeid = nodeid;
> > -       args.in_numargs = 1;
> > -       args.in_args[0].size = sizeof(inarg);
> > -       args.in_args[0].value = &inarg;
> > -       args.out_numargs = 1;
> > -       args.out_args[0].size = sizeof(*outargp);
> > -       args.out_args[0].value = outargp;
> > +       fuse_open_args_fill(&args, nodeid, opcode, &inarg, outargp);
> >
> >         return fuse_simple_request(fm, &args);
> >  }
> >
> 
> This is a very minor nit but imo the split is a bit nicer if patch 2/3
> is this patch with your helper changes:
> 
> +/*
> + * Helper function to initialize fuse_args for OPEN/OPENDIR operations
> + */
> +void fuse_open_args_fill(struct fuse_args *args, u64 nodeid, int opcode,
> + struct fuse_open_in *inarg, struct fuse_open_out *outarg)
> +{
> + args->opcode = opcode;
> ...
> +}
> +
> +/*
> + * Helper function to initialize fuse_args for GETATTR operations
> + */
> +void fuse_getattr_args_fill(struct fuse_args *args, u64 nodeid,
> +     struct fuse_getattr_in *inarg,
> +     struct fuse_attr_out *outarg)
> +{
> + args->opcode = FUSE_GETATTR;
> ...
> +}
> +
> 
> and then patch 3 is your open+getattr changes. That way, it's easier
> to see that the changes in this patch to fuse_do_getattr() and
> fuse_send_open() have no functional changes in logic.

My rational here was, that if people don't like the changes to those functions
I can easily backtrack by just not providing this patch, so basically laziness.

I can easily change that.

Thanks for your explanation.

> 
> 
> Thanks,
> Joanne
> > --
> > 2.51.0
> >