Re: [PATCH] cifs: add missing unicode handling routines needed for smb2

On Mon, 7 Mar 2011 13:28:14 -0500
Jeff Layton [off-list ref] wrote:

On Mon, 7 Mar 2011 11:09:06 -0600
Steve French [off-list ref] wrote:

quoted

On Mon, Mar 7, 2011 at 9:53 AM, Jeff Layton [off-list ref] wrote:

quoted

On Mon, 7 Mar 2011 09:07:52 -0600
Steve French [off-list ref] wrote:

quoted

On Mon, Mar 7, 2011 at 8:41 AM, Jeff Layton [off-list ref] wrote:

quoted

On Sun, 6 Mar 2011 22:17:32 -0600
Steve French [off-list ref] wrote:

This patch appears to be malformed...

Cut-paste or mailer error.

commit 11471483dbfd2f4235346acf9e181a549b3979a1
Author: Steve French [off-list ref]
Date:   Mon Mar 7 04:15:05 2011 +0000

    [CIFS] Add missing unicode handling routines needed by smb2

    Signed-off-by: Steve French [off-list ref]

Given that there are no callers of these functions, it's hard to
understand the need for this. These look like cut and paste of the cifs
functions. Why do we need separate routines for this for smb2?

The caller of these functions will include SMB2 Tree Connect,
but the patch for that is not ready since quite a bit of #defines
in smb2pdu.c had to be reworked to account for the merge.

The main two reasons these two routines are needed:
1) we needed a Unicode only string conversion routine
(the cifs one takes an ascii parameter which is superflous
for the case of SMB2, which only does Unicode
and would make the smb2pdu.c code messie).
The routine is trivial and uses the existing cifs_unicode worker.
2) similarly with the routine to allocate a unicodestring on
the fly  which is needed since we only use iovecs for cifs
and similar to the above is only Unicode (no ascii).

The routines themselves are small.  I doubt that cifs
would use a routine that is unicode only but they
may be useful in the future so I put them in cifs_unicode.c
rather than a new smb2_unicode.c

I'm sorry, I'm not convinced. These routines do *exactly* the same
thing as the code that's already there. If the existing code is a
superset of the functionality needed, then there's really no need to
add new routines for this. That's just increases the maintenance
overhead for no value at all. 

Also, you didn't answer my question about the endianness. These
routines look wrong in that they don't convert the characters to little
endian. I think the best thing would be to remove these routines and
adapt the new code to use the existing infrastructure.

Please disregard the above. I just looked closer and you're correct
that there is a difference.

This is doing a strndup *to* ucs2, and since the routine I was worried
about is just counting the bytes, endianness doesn't matter.

Reviewed-by: Jeff Layton <redacted>