On Sat, Oct 12, 2024 at 10:20:04AM +0200, Amir Goldstein wrote:
> On Sat, Oct 12, 2024 at 9:21 AM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > On Fri, Oct 11, 2024 at 11:46 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
> > >
> > > Allow filesystems to use a mount option either as a
> > > path or a file descriptor.
> > >
> > > Signed-off-by: Christian Brauner <brauner@xxxxxxxxxx>
> >
> > Looks sane
> >
> > Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> >
> > > ---
> > > fs/fs_parser.c | 19 +++++++++++++++++++
> > > include/linux/fs_parser.h | 5 ++++-
> > > 2 files changed, 23 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/fs_parser.c b/fs/fs_parser.c
> > > index 24727ec34e5aa434364e87879cccf9fe1ec19d37..a017415d8d6bc91608ece5d42fa4bea26e47456b 100644
> > > --- a/fs/fs_parser.c
> > > +++ b/fs/fs_parser.c
> > > @@ -308,6 +308,25 @@ int fs_param_is_fd(struct p_log *log, const struct fs_parameter_spec *p,
> > > }
> > > EXPORT_SYMBOL(fs_param_is_fd);
> > >
> > > +int fs_param_is_fd_or_path(struct p_log *log, const struct fs_parameter_spec *p,
> > > + struct fs_parameter *param,
> > > + struct fs_parse_result *result)
> > > +{
> > > + switch (param->type) {
> > > + case fs_value_is_string:
> > > + return fs_param_is_string(log, p, param, result);
> > > + case fs_value_is_file:
> > > + result->uint_32 = param->dirfd;
> > > + if (result->uint_32 <= INT_MAX)
> > > + return 0;
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > + return fs_param_bad_value(log, param);
> > > +}
> > > +EXPORT_SYMBOL(fs_param_is_fd_or_path);
> > > +
>
> I just noticed that it is a little weird that fsparam_is_fd() accepts a numeric
> string while fsparam_is_fd_or_path() does not.
> Not to mention that fsparam_is_fd_or_path does not accept type filename.
>
> Obviously a helper name fs_param_is_file_or_string() wouldn't have
Yes, I'll use that. Thanks!
