Re: [PATCH v9 2/3] fs: fat: Add volume label entry method function

[Pali RohÃr]

On Thursday 08 February 2018 00:14:06 Chen Guanqiao wrote:
> Signed-off-by: Chen Guanqiao <chen.chenchacha@xxxxxxxxxxx>

Missing commit message and information what are the new proposed
functions suppose to do.

> ---
>  fs/fat/dir.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 8e100c3bf72c..d5286402c829 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -881,6 +881,53 @@ static int fat_get_short_entry(struct inode *dir, loff_t *pos,
> 	return -ENOENT;
>  }
> 
> +int fat_get_volume_label_entry(struct inode *dir, struct buffer_head **bh,
> +			       struct msdos_dir_entry **de)
> +{
> +	loff_t pos = 0;
> +
> +	*bh = NULL;
> +	*de = NULL;
> +	while (fat_get_entry(dir, &pos, bh, de) >= 0) {
> +		if (((*de)->attr & ATTR_VOLUME) && ((*de)->attr != ATTR_EXT) &&
> +		    !IS_FREE((*de)->name))
> +			return 0;
> +	}
> +	return -ENOENT;
> +}
> +EXPORT_SYMBOL_GPL(fat_get_volume_label_entry);

Why both functions are exported? In this patch series are used only in
file.c which is in the same object file.

> +
> +int fat_add_volume_label_entry(struct inode *dir, const unsigned char *name,
> +			       struct timespec *ts)
> +{
> +	struct msdos_sb_info *sbi = MSDOS_SB(dir->i_sb);
> +	struct msdos_dir_entry de;
> +	struct fat_slot_info sinfo;
> +	__le16 time, date;
> +	int err;
> +
> +	memcpy(de.name, name, MSDOS_NAME);
> +	de.attr = ATTR_VOLUME;
> +	de.lcase = 0;
> +	fat_time_unix2fat(sbi, ts, &time, &date, NULL);
> +	de.cdate = de.adate = 0;
> +	de.ctime = 0;
> +	de.ctime_cs = 0;
> +	de.time = time;
> +	de.date = date;
> +	fat_set_start(&de, 0);
> +	de.size = 0;
> +
> +	err = fat_add_entries(dir, &de, 1, &sinfo);
> +	if (err)
> +		return err;
> +
> +	brelse(sinfo.bh);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(fat_add_volume_label_entry);

This function looks like copy+paste of function msdos_add_entry().
Please de-duplicate code as this would lead to problems in future.

Also who is supposed to format dos name? Caller or callee? There is no
information neither in commit message nor in comments.

> +
>  /*
>   * The ".." entry can not provide the "struct fat_slot_info" information
>   * for inode, nor a usable i_pos. So, this function provides some information
> --
> 2.14.3

-- 
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: PGP signature