2020-06-24 05:43:57

by Gabriel Krisman Bertazi

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] ext4: Use generic casefolding support

Daniel Rosenberg <[email protected]> writes:

> -
> const struct dentry_operations ext4_dentry_ops = {
> - .d_hash = ext4_d_hash,
> - .d_compare = ext4_d_compare,
> + .d_hash = generic_ci_d_hash,
> + .d_compare = generic_ci_d_compare,
> };
> #endif

Can you make the structure generic since it is the same for f2fs and
ext4, which let you drop the code guards? Unless that becomes a problem for
d_revalidate with fscrypt, it is fine like this.

> #ifdef CONFIG_UNICODE
> - sbi = EXT4_SB(sb);
> - if (ext4_has_strict_mode(sbi) && IS_CASEFOLDED(dir) &&
> - sbi->s_encoding && utf8_validate(sbi->s_encoding, &dentry->d_name))
> + if (sb_has_enc_strict_mode(sb) && IS_CASEFOLDED(dir) &&

I keep reading the 'enc' in sb_has_enc_strict_mode() as 'encryption'. What do
you think about renaming it to sb_has_strict_encoding()?

These comments apply equally to patches 3 and 4. Other than that,

Reviewed-by: Gabriel Krisman Bertazi <[email protected]>

--
Gabriel Krisman Bertazi


2020-07-07 10:44:53

by Daniel Rosenberg

[permalink] [raw]
Subject: Re: [PATCH v9 4/4] ext4: Use generic casefolding support

On Tue, Jun 23, 2020 at 10:43 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> Daniel Rosenberg <[email protected]> writes:
>
> > -
> > const struct dentry_operations ext4_dentry_ops = {
> > - .d_hash = ext4_d_hash,
> > - .d_compare = ext4_d_compare,
> > + .d_hash = generic_ci_d_hash,
> > + .d_compare = generic_ci_d_compare,
> > };
> > #endif
>
> Can you make the structure generic since it is the same for f2fs and
> ext4, which let you drop the code guards? Unless that becomes a problem for
> d_revalidate with fscrypt, it is fine like this.
>
> --
> Gabriel Krisman Bertazi

I unify them in a later patch, since I end up having to deal with
fscrypt's d_revalidate. With that patch I'd end up undoing the export
I'd add for this, so I'll skip that for the moment.

-Daniel