Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Date:   Wed, 18 May 2022 12:25:57 -0700
From:   Eric Biggers <ebiggers@kernel.org>
To:     Gabriel Krisman Bertazi <krisman@collabora.com>
Cc:     tytso@mit.edu, adilger.kernel@dilger.ca, jaegeuk@kernel.org,
        linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net,
        kernel@collabora.com
Subject: Re: [PATCH v5 7/8] ext4: Move CONFIG_UNICODE defguards into the code
 flow
Message-ID: <YoVIRewDI7EWLdQd@sol.localdomain>
References: <20220518172320.333617-1-krisman@collabora.com>
 <20220518172320.333617-8-krisman@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220518172320.333617-8-krisman@collabora.com>
Precedence: bulk

On Wed, May 18, 2022 at 01:23:19PM -0400, Gabriel Krisman Bertazi wrote:
> Instead of a bunch of ifdefs, make the unicode built checks part of the
> code flow where possible, as requested by Torvalds.
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com>
> 
> ---
> Changes since v4:
>   - Create stub for !CONFIG_UNICODE case (eric)
> ---
>  fs/ext4/ext4.h  | 37 ++++++++++++++++++++-----------------
>  fs/ext4/namei.c | 15 ++++++---------
>  fs/ext4/super.c |  4 +---
>  3 files changed, 27 insertions(+), 29 deletions(-)
> 
> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
> index 93a28fcb2e22..c38999ee3627 100644
> --- a/fs/ext4/ext4.h
> +++ b/fs/ext4/ext4.h
> @@ -2727,8 +2727,24 @@ ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);
>  
>  #if IS_ENABLED(CONFIG_UNICODE)
>  extern int ext4_fname_setup_ci_filename(struct inode *dir,
> -					 const struct qstr *iname,
> -					 struct ext4_filename *fname);
> +					const struct qstr *iname,
> +					struct ext4_filename *fname);
> +
> +static inline void ext4_fname_free_ci_filename(struct ext4_filename *fname)
> +{
> +	kfree(fname->cf_name.name);
> +	fname->cf_name.name = NULL;
> +}
> +#else
> +static inline int ext4_fname_setup_ci_filename(struct inode *dir,
> +					       const struct qstr *iname,
> +					       struct ext4_filename *fname)
> +{
> +	return 0;
> +}
> +static inline void ext4_fname_free_ci_filename(struct ext4_filename *fname)
> +{
> +}
>  #endif
>  
>  #ifdef CONFIG_FS_ENCRYPTION
> @@ -2758,9 +2774,7 @@ static inline int ext4_fname_setup_filename(struct inode *dir,
>  
>  	ext4_fname_from_fscrypt_name(fname, &name);
>  
> -#if IS_ENABLED(CONFIG_UNICODE)
>  	err = ext4_fname_setup_ci_filename(dir, iname, fname);
> -#endif
>  	return err;
>  }

This can just do 'return ext4_fname_setup_ci_filename(...)'.  No need for the
err variable.

>  
> @@ -2777,9 +2791,7 @@ static inline int ext4_fname_prepare_lookup(struct inode *dir,
>  
>  	ext4_fname_from_fscrypt_name(fname, &name);
>  
> -#if IS_ENABLED(CONFIG_UNICODE)
>  	err = ext4_fname_setup_ci_filename(dir, &dentry->d_name, fname);
> -#endif
>  	return err;
>  }

Similarly, this can just return ext4_fname_setup_ci_filename(...).

>  
> @@ -2794,10 +2806,7 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname)
>  	fname->usr_fname = NULL;
>  	fname->disk_name.name = NULL;
>  
> -#if IS_ENABLED(CONFIG_UNICODE)
> -	kfree(fname->cf_name.name);
> -	fname->cf_name.name = NULL;
> -#endif
> +	ext4_fname_free_ci_filename(fname);
>  }
>  #else /* !CONFIG_FS_ENCRYPTION */
>  static inline int ext4_fname_setup_filename(struct inode *dir,
> @@ -2810,10 +2819,7 @@ static inline int ext4_fname_setup_filename(struct inode *dir,
>  	fname->disk_name.name = (unsigned char *) iname->name;
>  	fname->disk_name.len = iname->len;
>  
> -#if IS_ENABLED(CONFIG_UNICODE)
>  	err = ext4_fname_setup_ci_filename(dir, iname, fname);
> -#endif
> -
>  	return err;

Likewise.

- Eric