diff mbox series

[U-Boot,v3,2/4] fs: add fs_get_type() for current filesystem type

Message ID 20191004030523.21579-3-takahiro.akashi@linaro.org
State Superseded
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk | expand

Commit Message

AKASHI Takahiro Oct. 4, 2019, 3:05 a.m. UTC
This function is a variant of fs_get_type_name() and returns a filesystem
type with which the current device is associated.
We don't want to export fs_type variable directly because we have to take
care of it consistently within fs.c.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fs.c      | 13 +++++++++++++
 include/fs.h | 10 ++++++++++
 2 files changed, 23 insertions(+)

Comments

Heinrich Schuchardt Oct. 4, 2019, 7:04 p.m. UTC | #1
On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> This function is a variant of fs_get_type_name() and returns a filesystem
> type with which the current device is associated.
> We don't want to export fs_type variable directly because we have to take
> care of it consistently within fs.c.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Looking at fs/fs.c there seems to be a lot of inconsistency in the usage
of FS_TYPE_ANY. Some of the file system functions set fs_type =
FS_TYPE_ANY before calling fs_close(). Others don't. Shouldn't we move
those assignments to fs_close() to get consistency?

FS_TYPE_ANY seems to be misnomer and could be replaced by FS_TYPE_NONE.

Please, use scripts/get_maintainer in future to determine the addressees
of patches. I have put the missing ones on CC now.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>


> ---
>  fs/fs.c      | 13 +++++++++++++
>  include/fs.h | 10 ++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/fs/fs.c b/fs/fs.c
> index 64ba25fea8bf..e5307dbeaa37 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -307,6 +307,19 @@ static struct fstype_info *fs_get_info(int fstype)
>  	return info;
>  }
>
> +/**
> + * fs_get_type() - Get type of current filesystem
> + *
> + * Return: filesystem type
> + *
> + * Returns filesystem type representing the current filesystem, or
> + * FS_TYPE_ANY for any unrecognised filesystem.
> + */
> +int fs_get_type(void)
> +{
> +	return fs_type;
> +}
> +
>  /**
>   * fs_get_type_name() - Get type of current filesystem
>   *
> diff --git a/include/fs.h b/include/fs.h
> index 5a1244d57fd2..6dfdb5c5307a 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -44,6 +44,16 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
>   */
>  void fs_close(void);
>
> +/**
> + * fs_get_type() - Get type of current filesystem
> + *
> + * Return: filesystem type
> + *
> + * Returns filesystem type representing the current filesystem, or
> + * FS_TYPE_ANY for any unrecognised filesystem.
> + */
> +int fs_get_type(void);
> +
>  /**
>   * fs_get_type_name() - Get type of current filesystem
>   *
>
AKASHI Takahiro Oct. 7, 2019, 2:13 a.m. UTC | #2
On Fri, Oct 04, 2019 at 09:04:59PM +0200, Heinrich Schuchardt wrote:
> On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> > This function is a variant of fs_get_type_name() and returns a filesystem
> > type with which the current device is associated.
> > We don't want to export fs_type variable directly because we have to take
> > care of it consistently within fs.c.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> Looking at fs/fs.c there seems to be a lot of inconsistency in the usage
> of FS_TYPE_ANY. Some of the file system functions set fs_type =
> FS_TYPE_ANY before calling fs_close(). Others don't. Shouldn't we move
> those assignments to fs_close() to get consistency?

Another patch.

> FS_TYPE_ANY seems to be misnomer and could be replaced by FS_TYPE_NONE.
> 
> Please, use scripts/get_maintainer in future to determine the addressees
> of patches. I have put the missing ones on CC now.

There is no dedicated maintainer for "fs" sub-system,
so Tom was included here after MAINTAINERS.

-Takahiro Akashi


> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> 
> > ---
> >  fs/fs.c      | 13 +++++++++++++
> >  include/fs.h | 10 ++++++++++
> >  2 files changed, 23 insertions(+)
> >
> > diff --git a/fs/fs.c b/fs/fs.c
> > index 64ba25fea8bf..e5307dbeaa37 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -307,6 +307,19 @@ static struct fstype_info *fs_get_info(int fstype)
> >  	return info;
> >  }
> >
> > +/**
> > + * fs_get_type() - Get type of current filesystem
> > + *
> > + * Return: filesystem type
> > + *
> > + * Returns filesystem type representing the current filesystem, or
> > + * FS_TYPE_ANY for any unrecognised filesystem.
> > + */
> > +int fs_get_type(void)
> > +{
> > +	return fs_type;
> > +}
> > +
> >  /**
> >   * fs_get_type_name() - Get type of current filesystem
> >   *
> > diff --git a/include/fs.h b/include/fs.h
> > index 5a1244d57fd2..6dfdb5c5307a 100644
> > --- a/include/fs.h
> > +++ b/include/fs.h
> > @@ -44,6 +44,16 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
> >   */
> >  void fs_close(void);
> >
> > +/**
> > + * fs_get_type() - Get type of current filesystem
> > + *
> > + * Return: filesystem type
> > + *
> > + * Returns filesystem type representing the current filesystem, or
> > + * FS_TYPE_ANY for any unrecognised filesystem.
> > + */
> > +int fs_get_type(void);
> > +
> >  /**
> >   * fs_get_type_name() - Get type of current filesystem
> >   *
> >
>
diff mbox series

Patch

diff --git a/fs/fs.c b/fs/fs.c
index 64ba25fea8bf..e5307dbeaa37 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -307,6 +307,19 @@  static struct fstype_info *fs_get_info(int fstype)
 	return info;
 }
 
+/**
+ * fs_get_type() - Get type of current filesystem
+ *
+ * Return: filesystem type
+ *
+ * Returns filesystem type representing the current filesystem, or
+ * FS_TYPE_ANY for any unrecognised filesystem.
+ */
+int fs_get_type(void)
+{
+	return fs_type;
+}
+
 /**
  * fs_get_type_name() - Get type of current filesystem
  *
diff --git a/include/fs.h b/include/fs.h
index 5a1244d57fd2..6dfdb5c5307a 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -44,6 +44,16 @@  int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
  */
 void fs_close(void);
 
+/**
+ * fs_get_type() - Get type of current filesystem
+ *
+ * Return: filesystem type
+ *
+ * Returns filesystem type representing the current filesystem, or
+ * FS_TYPE_ANY for any unrecognised filesystem.
+ */
+int fs_get_type(void);
+
 /**
  * fs_get_type_name() - Get type of current filesystem
  *