[U-Boot] [PATCH 11/17] fs: add mkdir interface

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Fri, Jul 20, 2018 at 07:35:18PM +0200, Heinrich Schuchardt wrote:
> On 07/20/2018 04:57 AM, AKASHI Takahiro wrote:
> > "mkdir" interface is added to file operations.
> > This is a preparatory change as mkdir support for FAT file system
> > will be added in next patch.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  fs/fs.c      | 45 +++++++++++++++++++++++++++++++++++++++++++++
> >  include/fs.h | 10 ++++++++++
> >  2 files changed, 55 insertions(+)
> > 
> > diff --git a/fs/fs.c b/fs/fs.c
> > index 33808d549e..3cb6b21fe9 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -105,6 +105,11 @@ static inline int fs_opendir_unsupported(const char *filename,
> >  	return -EACCES;
> >  }
> >  
> > +static inline int fs_mkdir_unsupported(const char *dirname)
> > +{
> > +	return -1;
> > +}
> > +
> >  struct fstype_info {
> >  	int fstype;
> >  	char *name;
> > @@ -142,6 +147,7 @@ struct fstype_info {
> >  	int (*readdir)(struct fs_dir_stream *dirs, struct fs_dirent **dentp);
> >  	/* see fs_closedir() */
> >  	void (*closedir)(struct fs_dir_stream *dirs);
> > +	int (*mkdir)(const char *dirname);
> >  };
> >  
> >  static struct fstype_info fstypes[] = {
> > @@ -165,6 +171,7 @@ static struct fstype_info fstypes[] = {
> >  		.opendir = fat_opendir,
> >  		.readdir = fat_readdir,
> >  		.closedir = fat_closedir,
> > +		.mkdir = fs_mkdir_unsupported,
> 
> Why should we create a dummy function? Just use NULL as an indicator

because I followed coding style of the existing code here.

> that the interface method is not implemented. See the use of structures
> for the driver model.

Can any file system ever be based on driver model?

> @Simon
> The array fstypes[] should be replaced by a linker list to match the
> driver model.
> 
> >  	},
> >  #endif
> >  #ifdef CONFIG_FS_EXT4
> > @@ -185,6 +192,7 @@ static struct fstype_info fstypes[] = {
> >  #endif
> >  		.uuid = ext4fs_uuid,
> >  		.opendir = fs_opendir_unsupported,
> > +		.mkdir = fs_mkdir_unsupported,
> >  	},
> >  #endif
> >  #ifdef CONFIG_SANDBOX
> > @@ -201,6 +209,7 @@ static struct fstype_info fstypes[] = {
> >  		.write = fs_write_sandbox,
> >  		.uuid = fs_uuid_unsupported,
> >  		.opendir = fs_opendir_unsupported,
> > +		.mkdir = fs_mkdir_unsupported,
> >  	},
> >  #endif
> >  #ifdef CONFIG_CMD_UBIFS
> > @@ -217,6 +226,7 @@ static struct fstype_info fstypes[] = {
> >  		.write = fs_write_unsupported,
> >  		.uuid = fs_uuid_unsupported,
> >  		.opendir = fs_opendir_unsupported,
> > +		.mkdir = fs_mkdir_unsupported,
> >  	},
> >  #endif
> >  #ifdef CONFIG_FS_BTRFS
> > @@ -233,6 +243,7 @@ static struct fstype_info fstypes[] = {
> >  		.write = fs_write_unsupported,
> >  		.uuid = btrfs_uuid,
> >  		.opendir = fs_opendir_unsupported,
> > +		.mkdir = fs_mkdir_unsupported,
> >  	},
> >  #endif
> >  	{
> > @@ -248,6 +259,7 @@ static struct fstype_info fstypes[] = {
> >  		.write = fs_write_unsupported,
> >  		.uuid = fs_uuid_unsupported,
> >  		.opendir = fs_opendir_unsupported,
> > +		.mkdir = fs_mkdir_unsupported,
> >  	},
> >  };
> >  
> > @@ -498,6 +510,20 @@ void fs_closedir(struct fs_dir_stream *dirs)
> >  }
> >  
> >  
> > +int fs_mkdir(const char *dirname)
> > +{
> > +	int ret;
> > +
> > +	struct fstype_info *info = fs_get_info(fs_type);
> > +
> > +	ret = info->mkdir(dirname);
> 
> Please, check if mkdir is NULL before calling.

No other place checks for NULL. Is this really required?

> > +
> > +	fs_type = FS_TYPE_ANY;
> > +	fs_close();
> 
> What do you want to close here if mkdir() is not implemented by the file
> system?

I'm not quite confident here but ->close() be paired with ->probe(),
which is set to be called in fs_set_blk_dev().

Thanks,
-Takahiro AKASHI


> Best regards
> 
> Heinrich
> 
> > +
> > +	return ret;
> > +}
> > +
> >  int do_size(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> >  		int fstype)
> >  {
> > @@ -700,3 +726,22 @@ int do_fs_type(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> >  	return CMD_RET_SUCCESS;
> >  }
> >  
> > +int do_mkdir(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> > +								int fstype)
> > +{
> > +	int ret;
> > +
> > +	if (argc != 4)
> > +		return CMD_RET_USAGE;
> > +
> > +	if (fs_set_blk_dev(argv[1], argv[2], fstype))
> > +		return 1;
> > +
> > +	ret = fs_mkdir(argv[3]);
> > +	if (ret) {
> > +		printf("** Unable to create a directory \"%s\" **\n", argv[3]);
> > +		return 1;
> > +	}
> > +
> > +	return 0;
> > +}
> > diff --git a/include/fs.h b/include/fs.h
> > index 163da103b4..fbaee154dd 100644
> > --- a/include/fs.h
> > +++ b/include/fs.h
> > @@ -155,6 +155,14 @@ struct fs_dirent *fs_readdir(struct fs_dir_stream *dirs);
> >   */
> >  void fs_closedir(struct fs_dir_stream *dirs);
> >  
> > +/*
> > + * fs_mkdir - Create a directory
> > + *
> > + * @filename: Name of directory to create
> > + * @return 0 on success, -1 on error conditions
> > + */
> > +int fs_mkdir(const char *filename);
> > +
> >  /*
> >   * Common implementation for various filesystem commands, optionally limited
> >   * to a specific filesystem type via the fstype parameter.
> > @@ -169,6 +177,8 @@ int file_exists(const char *dev_type, const char *dev_part, const char *file,
> >  		int fstype);
> >  int do_save(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> >  		int fstype);
> > +int do_mkdir(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
> > +		int fstype);
> >  
> >  /*
> >   * Determine the UUID of the specified filesystem and print it. Optionally it is
> > 
>