On Wed, May 11, 2022 at 03:31:39PM -0400, Gabriel Krisman Bertazi wrote:
> Keeping it as qstr avoids the unnecessary conversion in f2fs_match
> 
> Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
> ---
>  fs/f2fs/dir.c      | 52 ++++++++++++++++++++++++++++------------------
>  fs/f2fs/f2fs.h     |  3 ++-
>  fs/f2fs/recovery.c |  5 +----
>  3 files changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 166f08623362..c2a02003c5b9 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -81,28 +81,47 @@ int f2fs_init_casefolded_name(const struct inode *dir,
>  {
>  #if IS_ENABLED(CONFIG_UNICODE)
>       struct super_block *sb = dir->i_sb;
> +     unsigned char *buf;
> +     int len;
>  
>       if (IS_CASEFOLDED(dir)) {
> -             fname->cf_name.name = f2fs_kmem_cache_alloc(f2fs_cf_name_slab,
> +             buf = f2fs_kmem_cache_alloc(f2fs_cf_name_slab,
>                                       GFP_NOFS, false, F2FS_SB(sb));
> -             if (!fname->cf_name.name)
> -                     return -ENOMEM;
> -             fname->cf_name.len = utf8_casefold(sb->s_encoding,
> -                                                fname->usr_fname,
> -                                                fname->cf_name.name,
> -                                                F2FS_NAME_LEN);
> -             if ((int)fname->cf_name.len <= 0) {
> -                     kmem_cache_free(f2fs_cf_name_slab, fname->cf_name.name);
> +             if (!buf) {
>                       fname->cf_name.name = NULL;
> +                     return -ENOMEM;
> +             }
> +
> +             len = utf8_casefold(sb->s_encoding, fname->usr_fname,
> +                                 buf, F2FS_NAME_LEN);
> +
> +             if (len <= 0) {
> +                     kmem_cache_free(f2fs_cf_name_slab, buf);
> +                     buf = NULL;
>                       if (sb_has_strict_encoding(sb))
>                               return -EINVAL;
>                       /* fall back to treating name as opaque byte sequence */
>               }
> +             fname->cf_name.name = buf;
> +             fname->cf_name.len = (unsigned int) len;
>       }
>  #endif

There's some inconsistent behavior above; now sometimes fname->cf_name.name is
set to NULL on failure and sometime it's not.  Also now fname->cf_name.len can
be set to a negative value.

Since struct f2fs_filename is always zero-initialized, how about only setting
the fname->cf_name fields if we actually have a valid value to assign?  I.e.

                buf = f2fs_kmem_cache_alloc(f2fs_cf_name_slab,
                                        GFP_NOFS, false, F2FS_SB(sb));
                if (!buf)
                        return -ENOMEM;

                len = utf8_casefold(sb->s_encoding, fname->usr_fname,
                                    buf, F2FS_NAME_LEN);
                if (len <= 0) {
                        kmem_cache_free(f2fs_cf_name_slab, buf);
                        if (sb_has_strict_encoding(sb))
                                return -EINVAL;
                        /* fall back to treating name as opaque byte sequence */
                        return 0;
                }
                fname->cf_name.name = buf;
                fname->cf_name.len = len;

> +void f2fs_free_casefolded_name(struct f2fs_filename *fname)
> +{
> +#if IS_ENABLED(CONFIG_UNICODE)
> +     unsigned char *buf = (unsigned char *)fname->cf_name.name;
> +
> +     if (buf) {
> +             kmem_cache_free(f2fs_cf_name_slab, buf);
> +             fname->cf_name.name = NULL;
> +     }
> +
> +#endif
> +}

Kernel code usually uses static inline stubs for the !CONFIG_$FOO case in cases
like this, as that causes the function calls to be compiled away to nothing when
they're unneeded.

- Eric


_______________________________________________
Linux-f2fs-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Reply via email to