RE: [PATCH v2] hfsplus: fix s_fs_info leak on mount setup failure

[Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>]

On Wed, 2026-02-11 at 02:03 +0000, Al Viro wrote:
> On Fri, Feb 06, 2026 at 10:22:20PM +0000, Viacheslav Dubeyko wrote:
> 
> > I did run the xfstests for HFS+ with viro/vfs.git #untested.hfsplus. Everything
> > looks good, I don't see any new issues. Currently, around 29 test-cases fail for
> > HFS+. I see the same number of failures with applied patchset.
> > 
> > The code looks good. And I am ready to take the patchset into HFS/HFS+ tree.
> > Would you like to send the pathset for nls.h modification discussion?
> 
> FWIW, I wonder what the hell is that code doing:
>         err = -EINVAL;
>         if (!sbi->nls) {
>                 /* try utf8 first, as this is the old default behaviour */
>                 sbi->nls = load_nls("utf8");
>                 if (!sbi->nls)
>                         sbi->nls = load_nls_default();
>         }

As far as I can see, this code has been carefully moved from
hfsplus_parse_options() by this commit [1]. Also, we have logic of loading nls
in hfsplus_parse_param() [2].

It looks like redundant code to me. The whole logic looks like:
(1) try to load nls during parsing mount options;
(2) temporary load of "utf8" for hidden dir search;
(3) return previously loaded (or NULL) nls back.

Thanks,
Slava. 

>  
>         /* temporarily use utf8 to correctly find the hidden dir below */
>         nls = sbi->nls;   
>         sbi->nls = load_nls("utf8");
>         if (!sbi->nls) {
>                 pr_err("unable to load nls for utf8\n");
>                 goto out_unload_nls;
>         }
> 
> If load_nls("utf8") fails on the first call, I don't see how the second one
> might succeed.  What's the intended behaviour here?
> 
> What that code actually does is
> 	* if UTF8 isn't loadable, fail hard, no matter what
> 	* if it is loadable, use it for the duration of fill_super,
> then if we had something configured, switch back to that, otherwise
> stay with UTF8.

[1] https://lore.kernel.org/all/20240916172735.866916-6-sandeen@redhat.com/
[2] https://elixir.bootlin.com/linux/v6.19-rc5/source/fs/hfsplus/options.c#L118