Thread (49 messages) 49 messages, 6 authors, 2022-09-25

Re: [RFC PATCH 11/20] hfs: Explicitly set hsb->nls_disk when hsb->nls_io is set

From: Viacheslav Dubeyko <slava@dubeyko.com>
Date: 2021-08-09 18:00:41
Also in: linux-cifs, lkml

On Aug 9, 2021, at 10:37 AM, Matthew Wilcox [off-list ref] wrote:

On Mon, Aug 09, 2021 at 10:31:55AM -0700, Viacheslav Dubeyko wrote:
quoted
quoted
On Aug 8, 2021, at 9:24 AM, Pali Rohár [off-list ref] wrote:

It does not make any sense to set hsb->nls_io (NLS iocharset used between
VFS and hfs driver) when hsb->nls_disk (NLS codepage used between hfs
driver and disk) is not set.

Reverse engineering driver code shown what is doing in this special case:

  When codepage was not defined but iocharset was then
  hfs driver copied 8bit character from disk directly to
  16bit unicode wchar_t type. Which means it did conversion
  from Latin1 (ISO-8859-1) to Unicode because first 256
  Unicode code points matches 8bit ISO-8859-1 codepage table.
  So when iocharset was specified and codepage not, then
  codepage used implicit value "iso8859-1".

So when hsb->nls_disk is not set and hsb->nls_io is then explicitly set
hsb->nls_disk to "iso8859-1".

Such setup is obviously incompatible with Mac OS systems as they do not
support iso8859-1 encoding for hfs. So print warning into dmesg about this
fact.

After this change hsb->nls_disk is always set, so remove code paths for
case when hsb->nls_disk was not set as they are not needed anymore.

Sounds reasonable. But it will be great to know that the change has been tested reasonably well.
I don't think it's reasonable to ask Pali to test every single filesystem.
That's something the maintainer should do, as you're more likely to have
the infrastructure already set up to do testing of your filesystem and
be aware of fun corner cases and use cases than someone who's working
across all filesystems.
I see the point. But the whole approach needs to be tested as minimum for one particular file system. :) And it could be any favorite one.

Thanks,
Slava.

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help