Re: Duplicate unicode tables in smb/cifs/jfs

From: Dr. David Alan Gilbert
Date: Tue Jun 27 2023 - 06:42:54 EST


* Steve French (smfrench@xxxxxxxxx) wrote:
> On Mon, Jun 26, 2023 at 7:40 PM Dr. David Alan Gilbert <linux@xxxxxxxxxxx>
> wrote:
>
> > * Steve French (smfrench@xxxxxxxxx) 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@xxxxxxxxxx
> > >
> > > 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 |_______/