Duplicate unicode tables in smb/cifs/jfs

Duplicate unicode tables in smb/cifs/jfs

[Dr. David Alan Gilbert]

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.

(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)

-- 
 -----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   |_______/

Re: Duplicate unicode tables in smb/cifs/jfs

[Dave Kleikamp]

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)
> 

Re: Duplicate unicode tables in smb/cifs/jfs

[Dr. David Alan Gilbert]

* 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?

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   |_______/

Re: Duplicate unicode tables in smb/cifs/jfs

[Dr. David Alan Gilbert]

* 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   |_______/