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

[Jeff Layton]

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?

-- 
Jeff Layton <jlayton at redhat.com>