From: "Dr. David Alan Gilbert" <linux@treblig.org>
To: Steve French <smfrench@gmail.com>
Cc: Dave Kleikamp <dave.kleikamp@oracle.com>,
sfrench@samba.org, linkinjeon@kernel.org,
jfs-discussion@lists.sourceforge.net, linux-cifs@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: Duplicate unicode tables in smb/cifs/jfs
Date: Tue, 27 Jun 2023 10:42:29 +0000 [thread overview]
Message-ID: <ZJq9FcdFLQIZ8cja@gallifrey> (raw)
In-Reply-To: <CAH2r5mvEWeSOM=NYrxSG1EZ1py-DSkOrwEyh+_L-KuFLVWktQw@mail.gmail.com>
* Steve French (smfrench@gmail.com) wrote:
> On Mon, Jun 26, 2023 at 7:40 PM Dr. David Alan Gilbert <linux@treblig.org>
> wrote:
>
> > * Steve French (smfrench@gmail.com) wrote:
> > > probably is low risk to make the ksmbd/cifs (server and client) code
> > common
> > > for this
> >
> > I'm up for trying to do that mod; do you have a feel of the best way?
> > I guess this is two copies to avoid symbol clashes if both the server
> > and clients are built/loaded on the same kernel?
> >
>
> First step would be to move common Unicode/UCS-2 header definitions from
> fs/smb/client
> and fs/smb/server to fs/smb/common
>
> Second stuff could be having a common helper module in fs/smb/common
> similar to
> fs/smb/common/cifs_md4.ko
OK, let me have a crack at that and see where I get to.
Dave
>
>
> > I guess the clean way is to make this a separate .c/module that the
> > others depend on and hten you have a nice single copy in the binary
> > as well as the source.
> >
> > The shorter hack that at least avoids the source dupe would be to
> > change the declarations in the uniupr.h files to a #define that then
> > instantiates it with different names in the place those are #included.
> > You'd want to move the uniupr.h up somewhere, to hmm fs/uniupr.h ?
> >
> > (Incidentally, I think the UNIUPR_NOLOWER is always defined
> > so that whole chunk can go in both of them).
> >
> > I guess the next level would be trying to merge parts of server/client
> > unicode.[ch] - but I was just eyeing up this particularly simple dupe in
> > that odd uniupr.h.
> >
> > > (and leave the JFS code alone as Dave Kleikamp suggests)
> >
> > Well hmm; my reading is the tables that JFS uses are *identical*
> > to these; so if this header was somewhere outside of fs/smb it could
> > probably #include it and just use the same table, with a
> > no-binary-change.
> >
> > Dave
> >
> > > On Mon, Jun 26, 2023 at 12:03 PM Dave Kleikamp <dave.kleikamp@oracle.com
> > >
> > > wrote:
> > >
> > > > On 6/25/23 9:28AM, Dr. David Alan Gilbert wrote:
> > > > > Hi All,
> > > > > I just tripped over three sets of duplicated unicode tables and
> > > > > wondered if anyone had tried to rationalise them:
> > > > >
> > > > > The pair of:
> > > > > ./fs/smb/server/uniupr.h
> > > > > ./fs/smb/client/cifs_uniupr.h
> > > > >
> > > > > are identical except for formatting.
> > > > >
> > > > > ./fs/jfs/jfs_uniupr.c,
> > > > > and I think this is the same with some change in variable name.
> > > >
> > > > From JFS's point of view, I wonder how relevant any of this code is.
> > > > The Linux port of JFS originally was interested in compatibility with
> > > > OS/2, which had case-insensitive file names. (Case-preserving, if I
> > > > remember correctly, but names would match in a case-insensitive
> > manner.)
> > > >
> > > > I would be surprised if anyone cares about this feature anymore.
> > Without
> > > > a JFS_OS2 flag set in the superblock, none of the case-conversion code
> > > > comes into play.
> > > >
> > > > I assume SMB cares more about this and if Steve has an opinion on how
> > to
> > > > address this, I'd be happy to follow his lead. Probably better than
> > > > ripping function out of JFS that could break some user that I'm not
> > > > aware of.
> > > >
> > > > Shaggy
> > > >
> > > > >
> > > > > (I'm guessing the same thing is implemented in a bunch of
> > > > > other places as well in a different way)
> > > > >
> > > > > Would it make sense to swing fs/smb/server/uniupr.h up to
> > > > > hmm, maybe fs/uniupr.h, remove any of the cifs_ prefixes
> > > > > and then use the same include in all 3 places?
> > > > >
> > > > > Maybe then later look at using some of the nls code?
> > > > >
> > > > > Dave (who just tripped over this stuff)
> > > > >
> > > >
> > >
> > >
> > > --
> > > Thanks,
> > >
> > > Steve
> > --
> > -----Open up your eyes, open up your mind, open up your code -------
> > / Dr. David Alan Gilbert | Running GNU/Linux | Happy \
> > \ dave @ treblig.org | | In Hex /
> > \ _________________________|_____ http://www.treblig.org |_______/
> >
>
>
> --
> Thanks,
>
> Steve
--
-----Open up your eyes, open up your mind, open up your code -------
/ Dr. David Alan Gilbert | Running GNU/Linux | Happy \
\ dave @ treblig.org | | In Hex /
\ _________________________|_____ http://www.treblig.org |_______/
prev parent reply other threads:[~2023-06-27 10:42 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-25 14:28 Duplicate unicode tables in smb/cifs/jfs Dr. David Alan Gilbert
2023-06-26 17:00 ` Dave Kleikamp
[not found] ` <CAH2r5mvwZnd+S8CmZGQSdtnAWD45YjFx-1j0EaFBR3Qn-SjHEA@mail.gmail.com>
2023-06-27 0:40 ` Dr. David Alan Gilbert
[not found] ` <CAH2r5mvEWeSOM=NYrxSG1EZ1py-DSkOrwEyh+_L-KuFLVWktQw@mail.gmail.com>
2023-06-27 10:42 ` Dr. David Alan Gilbert [this message]
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=ZJq9FcdFLQIZ8cja@gallifrey \
--to=linux@treblig.org \
--cc=dave.kleikamp@oracle.com \
--cc=jfs-discussion@lists.sourceforge.net \
--cc=linkinjeon@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=sfrench@samba.org \
--cc=smfrench@gmail.com \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox