[linux-cifs-client] [PATCH 0/2] [CIFS] cleanup code by moving often repeated code sequence into a separate function

Jeff Layton jlayton at redhat.com
Wed Jul 9 18:28:44 GMT 2008


On Wed, 9 Jul 2008 10:46:51 -0400
Jeff Layton <jlayton at redhat.com> wrote:

> On Thu, 29 May 2008 02:38:29 +0200
> Günter Kukkukk <linux at kukkukk.com> wrote:
> 
> > there are many places in cifssmb.c, where filenames have to
> > be setup for ucs (MS unicode) or ascii (nls codepage) representation.
> > 
> > In a first step move all repeating code sequences, which use 
> >   - cifsConvertToUCS() 
> > to a new function
> >   - setup_ucs_nls_name()
> > 
> > Then move all repeating code sequences, which use
> >   - cifs_strtoUCS()
> > to the new function
> >   - setup_ucs_nls_name()
> > passing the "remap" parameter as "0" (false).
> > 
> > There are still places left, which cannot easily moved
> > the same way.
> > In a future step cifs should be enhanced to also support codepages
> > for legacy servers (similar to the smbfs implementation). This could
> > best be done at "a single point" in the code path. Further cleanup
> > needs to be done.
> > 
> > Günter Kukkukk
> > 
> 
> Hi Günter,
> 
>  These patches seem like a good start, but I think we might need to
> modify this approach a bit. Here's a rough outline of what I think we
> need to do to get proper support for a codepage= option (similar to
> smbfs):
> 
> 1) we need to change all of the functions in cifssmb.c that take a
> nls_codepage and remap arg to take a cifs_sb pointer instead
> 
> 2) we can then do something like the patches you've done but just pass
> this cifs_sb pointer to the new functions you've added instead of the
> nls_codepage and remap args. We may even want to change these names to
> something more generic since they won't necessarily be converting to
> and from unicode (not as a final result anyway)
> 
> 3) add a remote_nls field to the cifs_sb struct and add a codepage=
> mount option that populates the remote_nls (similarly to how
> local_nls is done)
> 
> 5) change the new functions you're adding so that they can handle doing
> an extra step to convert to a remote_nls if one is needed
> 
> Again, this outline is very rough, but I think it's more or less how we
> should tackle this. Thoughts?
> 

Sorry to reply to my own post, but I'm giving this a hard look today. I
think we have to start by defining what we want the behavior to be as
far as mount options...

When iocharset= is defined then we'll use that charset for the
local_nls. Otherwise, it's determined by load_default_nls(). This
is the behavior there today...

When codepage= is defined then we'll disable unicode capability in the
negprot exchange (i.e. we won't set SMBFLG2_UNICODE) and will forcibly
use that setting for remote_nls. When it's not set, then we'll leave
the flag enabled to see if the server supports unicode.

If the server turns out to support unicode we'll use "utf8" for the
remote_nls. If it doesn't, then we'll assume "ascii".

Does this sound like reasonable logic?

-- 
Jeff Layton <jlayton at redhat.com>


More information about the linux-cifs-client mailing list