Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 27 Jun 2023 10:42:29 +0000
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
Message-ID: <ZJq9FcdFLQIZ8cja@gallifrey>
References: <ZJhPIYSUzBpuqosh@gallifrey>
 <25821273-d391-1502-ff8a-07ea7a59c8f3@oracle.com>
 <CAH2r5mvwZnd+S8CmZGQSdtnAWD45YjFx-1j0EaFBR3Qn-SjHEA@mail.gmail.com>
 <ZJov5VNakxNXU0Mk@gallifrey>
 <CAH2r5mvEWeSOM=NYrxSG1EZ1py-DSkOrwEyh+_L-KuFLVWktQw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAH2r5mvEWeSOM=NYrxSG1EZ1py-DSkOrwEyh+_L-KuFLVWktQw@mail.gmail.com>
User-Agent: Mutt/2.0.5 (2021-01-21)
Precedence: bulk

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