linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steve French <smfrench@gmail.com>
To: "Pali Rohár" <pali@kernel.org>
Cc: "Matthew Wilcox" <willy@infradead.org>,
	"Viacheslav Dubeyko" <slava@dubeyko.com>,
	"Linux FS Devel" <linux-fsdevel@vger.kernel.org>,
	linux-ntfs-dev@lists.sourceforge.net,
	CIFS <linux-cifs@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Jan Kara" <jack@suse.cz>,
	"OGAWA Hirofumi" <hirofumi@mail.parknet.co.jp>,
	"Theodore Y . Ts'o" <tytso@mit.edu>,
	"Luis de Bethencourt" <luisbg@kernel.org>,
	"Salah Triki" <salah.triki@gmail.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Dave Kleikamp" <shaggy@kernel.org>,
	"Anton Altaparmakov" <anton@tuxera.com>,
	"Pavel Machek" <pavel@ucw.cz>, "Marek Behún" <marek.behun@nic.cz>,
	"Christoph Hellwig" <hch@infradead.org>
Subject: Re: [RFC PATCH 11/20] hfs: Explicitly set hsb->nls_disk when hsb->nls_io is set
Date: Mon, 9 Aug 2021 15:43:09 -0500	[thread overview]
Message-ID: <CAH2r5ms2wK4P9=J4q7OJ4fLhi=e981TY1+Ue7yawyQiCzS9ThQ@mail.gmail.com> (raw)
In-Reply-To: <20210809174741.4wont2drya3rvpsr@pali>

For cifs.ko, I don't mind running our automated regression tests on
this patch when the patch (or patches) is ready, but was thinking
about an earlier discussion a few months about parth conversion in
cifs.ko prompted by Al Viro, and whether additional changes should be
made to move the character conversion later as well (e.g. for
characters in the reserved range such as '\' to 0xF026, and'':' to
0xF022  and '>' to 0xF024 and '?' to 0xF025 etc) for the 10 special
characters which have to get remapped into the UCS-2 reserved
character range.

On Mon, Aug 9, 2021 at 12:49 PM Pali Rohár <pali@kernel.org> wrote:
>
> On Monday 09 August 2021 18:37:19 Matthew Wilcox wrote:
> > On Mon, Aug 09, 2021 at 10:31:55AM -0700, Viacheslav Dubeyko wrote:
> > > > On Aug 8, 2021, at 9:24 AM, Pali Rohár <pali@kernel.org> 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.
>
> This patch series is currently in RFC form, as stated in cover letter
> mostly untested. So they are not in form for merging or detailed
> reviewing. I just would like to know if this is the right direction with
> filesystems and if I should continue with this my effort or not.
> And I thought that sending RFC "incomplete" patches is better way than
> just describing what to do and how...



-- 
Thanks,

Steve

  reply	other threads:[~2021-08-09 20:43 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-08 16:24 [RFC PATCH 00/20] fs: Remove usage of broken nls_utf8 and drop it Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 01/20] fat: Fix iocharset=utf8 mount option Pali Rohár
2021-08-15  3:42   ` OGAWA Hirofumi
2021-08-15  9:42     ` Pali Rohár
2021-08-15 11:23       ` OGAWA Hirofumi
2021-08-23  3:51   ` Kari Argillander
2021-08-08 16:24 ` [RFC PATCH 02/20] hfsplus: Add iocharset= mount option as alias for nls= Pali Rohár
2021-08-09 17:51   ` Viacheslav Dubeyko
2021-08-09 20:49   ` Kari Argillander
2021-08-09 21:25     ` Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 03/20] udf: Fix iocharset=utf8 mount option Pali Rohár
2021-08-12 14:17   ` Jan Kara
2021-08-12 15:51     ` Pali Rohár
2021-08-13 13:48       ` Jan Kara
2021-08-19  8:34         ` Pali Rohár
2021-08-19 10:41           ` Jan Kara
2021-08-08 16:24 ` [RFC PATCH 04/20] isofs: joliet: " Pali Rohár
2021-08-12 14:18   ` Jan Kara
2021-08-08 16:24 ` [RFC PATCH 05/20] ntfs: Undeprecate iocharset= " Pali Rohár
2021-08-09 20:52   ` Kari Argillander
2021-08-19  1:21   ` Kari Argillander
2021-08-19  8:12     ` Pali Rohár
2021-08-19 10:23       ` Kari Argillander
2021-08-19 22:04         ` Pali Rohár
2021-08-19 23:18           ` Kari Argillander
2021-08-08 16:24 ` [RFC PATCH 06/20] ntfs: Fix error processing when load_nls() fails Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 07/20] befs: Fix printing iocharset= mount option Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 08/20] befs: Rename enum value Opt_charset to Opt_iocharset to match " Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 09/20] befs: Fix error processing when load_nls() fails Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 10/20] befs: Allow to use native UTF-8 mode Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 11/20] hfs: Explicitly set hsb->nls_disk when hsb->nls_io is set Pali Rohár
2021-08-09 17:31   ` Viacheslav Dubeyko
2021-08-09 17:37     ` Matthew Wilcox
2021-08-09 17:47       ` Pali Rohár
2021-08-09 20:43         ` Steve French [this message]
2021-08-09 18:00       ` Viacheslav Dubeyko
2021-08-08 16:24 ` [RFC PATCH 12/20] hfs: Do not use broken utf8 NLS table for iocharset=utf8 mount option Pali Rohár
2021-08-09 17:49   ` Viacheslav Dubeyko
2021-08-08 16:24 ` [RFC PATCH 13/20] hfsplus: " Pali Rohár
2021-08-09 17:42   ` Viacheslav Dubeyko
2021-08-08 16:24 ` [RFC PATCH 14/20] jfs: Remove custom iso8859-1 implementation Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 15/20] jfs: Fix buffer overflow in jfs_strfromUCS_le() function Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 16/20] jfs: Do not use broken utf8 NLS table for iocharset=utf8 mount option Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 17/20] ntfs: " Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 18/20] cifs: " Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 19/20] cifs: Remove usage of load_nls_default() calls Pali Rohár
2021-08-08 16:24 ` [RFC PATCH 20/20] nls: Drop broken nls_utf8 module Pali Rohár
2021-09-03 21:26 ` [RFC PATCH 00/20] fs: Remove usage of broken nls_utf8 and drop it Kari Argillander
2021-09-03 21:37   ` Pali Rohár
2021-09-03 22:06     ` Kari Argillander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAH2r5ms2wK4P9=J4q7OJ4fLhi=e981TY1+Ue7yawyQiCzS9ThQ@mail.gmail.com' \
    --to=smfrench@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=anton@tuxera.com \
    --cc=hch@infradead.org \
    --cc=hirofumi@mail.parknet.co.jp \
    --cc=jack@suse.cz \
    --cc=linux-cifs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-ntfs-dev@lists.sourceforge.net \
    --cc=luisbg@kernel.org \
    --cc=marek.behun@nic.cz \
    --cc=pali@kernel.org \
    --cc=pavel@ucw.cz \
    --cc=salah.triki@gmail.com \
    --cc=shaggy@kernel.org \
    --cc=slava@dubeyko.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --subject='Re: [RFC PATCH 11/20] hfs: Explicitly set hsb->nls_disk when hsb->nls_io is set' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).