Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 8 Sep 2022 14:26:25 +0530
From:   "Ritesh Harjani (IBM)" <ritesh.list@gmail.com>
To:     Jason Yan <yanaijie@huawei.com>
Cc:     tytso@mit.edu, adilger.kernel@dilger.ca, jack@suse.cz,
        lczerner@redhat.com, linux-ext4@vger.kernel.org
Subject: Re: [PATCH v2 07/13] ext4: factor out ext4_encoding_init()
Message-ID: <20220908085625.r3xsfvdgn7ibykt2@riteshh-domain>
References: <20220903030156.770313-1-yanaijie@huawei.com>
 <20220903030156.770313-8-yanaijie@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220903030156.770313-8-yanaijie@huawei.com>
Precedence: bulk

On 22/09/03 11:01AM, Jason Yan wrote:
> Factor out ext4_encoding_init(). No functional change.
> 
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> Reviewed-by: Jan Kara <jack@suse.cz>
> ---
>  fs/ext4/super.c | 80 +++++++++++++++++++++++++++----------------------
>  1 file changed, 44 insertions(+), 36 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index f8806226b796..67972b0218c0 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4521,6 +4521,48 @@ static int ext4_inode_info_init(struct super_block *sb,
>  	return 0;
>  }
>  
> +static int ext4_encoding_init(struct super_block *sb, struct ext4_super_block *es)
> +{
> +#if IS_ENABLED(CONFIG_UNICODE)

How about simplying it like below.
		if (!IS_ENABLED(CONFIG_UNICODE))
			return 0;
	
		<...>	

Then we don't need #ifdef CONFIG_UNICODE

-ritesh

> +	const struct ext4_sb_encodings *encoding_info;
> +	struct unicode_map *encoding;
> +	__u16 encoding_flags = le16_to_cpu(es->s_encoding_flags);
> +
> +	if (!ext4_has_feature_casefold(sb) || sb->s_encoding)
> +		return 0;
> +
> +	encoding_info = ext4_sb_read_encoding(es);
> +	if (!encoding_info) {
> +		ext4_msg(sb, KERN_ERR,
> +			"Encoding requested by superblock is unknown");
> +		return -EINVAL;
> +	}
> +
> +	encoding = utf8_load(encoding_info->version);
> +	if (IS_ERR(encoding)) {
> +		ext4_msg(sb, KERN_ERR,
> +			"can't mount with superblock charset: %s-%u.%u.%u "
> +			"not supported by the kernel. flags: 0x%x.",
> +			encoding_info->name,
> +			unicode_major(encoding_info->version),
> +			unicode_minor(encoding_info->version),
> +			unicode_rev(encoding_info->version),
> +			encoding_flags);
> +		return -EINVAL;
> +	}
> +	ext4_msg(sb, KERN_INFO,"Using encoding defined by superblock: "
> +		"%s-%u.%u.%u with flags 0x%hx", encoding_info->name,
> +		unicode_major(encoding_info->version),
> +		unicode_minor(encoding_info->version),
> +		unicode_rev(encoding_info->version),
> +		encoding_flags);
> +
> +	sb->s_encoding = encoding;
> +	sb->s_encoding_flags = encoding_flags;
> +#endif
> +	return 0;
> +}
> +
>  static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  {
>  	struct buffer_head *bh, **group_desc;
> @@ -4678,42 +4720,8 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  
>  	ext4_apply_options(fc, sb);
>  
> -#if IS_ENABLED(CONFIG_UNICODE)
> -	if (ext4_has_feature_casefold(sb) && !sb->s_encoding) {
> -		const struct ext4_sb_encodings *encoding_info;
> -		struct unicode_map *encoding;
> -		__u16 encoding_flags = le16_to_cpu(es->s_encoding_flags);
> -
> -		encoding_info = ext4_sb_read_encoding(es);
> -		if (!encoding_info) {
> -			ext4_msg(sb, KERN_ERR,
> -				 "Encoding requested by superblock is unknown");
> -			goto failed_mount;
> -		}
> -
> -		encoding = utf8_load(encoding_info->version);
> -		if (IS_ERR(encoding)) {
> -			ext4_msg(sb, KERN_ERR,
> -				 "can't mount with superblock charset: %s-%u.%u.%u "
> -				 "not supported by the kernel. flags: 0x%x.",
> -				 encoding_info->name,
> -				 unicode_major(encoding_info->version),
> -				 unicode_minor(encoding_info->version),
> -				 unicode_rev(encoding_info->version),
> -				 encoding_flags);
> -			goto failed_mount;
> -		}
> -		ext4_msg(sb, KERN_INFO,"Using encoding defined by superblock: "
> -			 "%s-%u.%u.%u with flags 0x%hx", encoding_info->name,
> -			 unicode_major(encoding_info->version),
> -			 unicode_minor(encoding_info->version),
> -			 unicode_rev(encoding_info->version),
> -			 encoding_flags);
> -
> -		sb->s_encoding = encoding;
> -		sb->s_encoding_flags = encoding_flags;
> -	}
> -#endif
> +	if (ext4_encoding_init(sb, es))
> +		goto failed_mount;
>  
>  	if (test_opt(sb, DATA_FLAGS) == EXT4_MOUNT_JOURNAL_DATA) {
>  		printk_once(KERN_WARNING "EXT4-fs: Warning: mounting with data=journal disables delayed allocation, dioread_nolock, O_DIRECT and fast_commit support!\n");
> -- 
> 2.31.1
>