Re: [RFC PATCH 1/4] fs/ntfs3: Use new api for mounting

["Pali Rohár" <pali@kernel.org>]

On Monday 16 August 2021 16:14:17 Kari Argillander wrote:
> > > + * Load nls table or if @nls is utf8 then return NULL because
> > > + * nls=utf8 is totally broken.
> > > + */
> > > +static struct nls_table *ntfs_load_nls(char *nls)
> > > +{
> > > +	struct nls_table *ret;
> > > +
> > > +	if (!nls)
> > > +		return ERR_PTR(-EINVAL);
> > > +	if (strcmp(nls, "utf8"))
> > > +		return NULL;
> > > +	if (strcmp(nls, CONFIG_NLS_DEFAULT))
> > > +		return load_nls_default();
> > > +
> > > +	ret = load_nls(nls);
> > > +	if (!ret)
> > > +		return ERR_PTR(-EINVAL);
> > > +
> > > +	return ret;
> > > +}
> > 
> > This looks like something quite generic and not file system specific.
> > But I haven't found time to look at the series from Pali how this all
> > fits together.
> 
> It is quite generic I agree. Pali's series not implemeted any new way
> doing this thing. In many cases Pali uses just load_nls and not
> load_nls_default. This function basically use that if possible. It seems
> that load_nls_default does not need error path so that's why it is nicer
> to use.

Yes, I'm using what is currently available. But providing some helper
function should be a nice cleanup.

> One though is to implement api function load_nls_or_utf8(). Then we do not
> need to test this utf8 stuff in all places.

Beware that there are more cases which can happen:

- iocharset is not specified
  --> then driver default behavior is used
      --> it is either some fixed encoding (e.g. iso8859-1, utf8) or
          CONFIG_NLS_DEFAULT (*); so it should behave like iocharset is
          set to that fixed encoding or CONFIG_NLS_DEFAULT
- iocharset is set to utf8
  --> then native utf8* functions should be used instead of nls
- iocharset is set to CONFIG_NLS_DEFAULT
  --> then load_nls_default() should be used which is IIRC guaranteed to
      not fail
- iocharset is not set to utf8, neither to CONFIG_NLS_DEFAULT
  --> then load_nls(iocharset) should be used; this may fail

(*) - it is pity that not all fs drivers are using CONFIG_NLS_DEFAULT
      and some are using some their own fixed encoding... it just
      increase mess and "user surprise"