[linux-cifs-client] FILE_UNIX_INFO, and other stuff

[Richard Hughes]

Steve,

 Sorry to keep annoying you! I've a list of stuff I've noticed:

1. Question

I'm trying to simplify fs/cifs/file.c and it would make it a lot easier
to have the FileNameLength hang around for FILE_UNIX_INFO. I noticed
FILE_DIRECTORY_INFO has this variable already.
Can I add :
__u32 FileNameLength;
to FILE_UNIX_INFO? Or is this struct set up with a specific layout?

2. Quickfix

I also notice the use of the variable 'pUnixFindData' in
fs/cifs/file.c:cifs_filldir_unix (only used locally in this function) -
surely this should be 'pfindDataUnix' for consistency.

3. Quickfix

Again, only in function fs/cifs/file.c:cifs_filldir_unix there is a
variable 'pUnixFindData', for consistency it should be 'pfindDataUnix'

4. Observation

In fs/cifs/file.c:cifs_readdir, there are 4 places where:

 - 'pfindData->FileName' is converted from UNICODE to ASCII
 - A check is done for validness. ( '..' and '.' )
 - reset_resume_key is called with the ASCII filename

Then in fs/cifs/file.c:reset_resume_key the first thing it does is to 
* convert the ASCII back to UNICODE * (if Unicode is set)

If a routine was written, say 'cifs_check_normal_dir' it could do a
"UNICODE AWARE" check for '.' and '..' and not have to do the
UNICODE->ASCII->UNICODE conversion. This alone would :

 - Remove more than 80 lines of code (mostly from cifs_readdir)
 - Add a ~11 line function

And cifs_readdir sure needs some cleanup! Say the word...

5. Suggestion

Many more lines of code could be saved (more easily debugged) and
program flow simplified if all the functions in fs/cifs/file.c assumed
that the structure they get passed of type FILE_UNIX_INFO \could\ be
UNICODE. At the moment each function doesn't know what it's getting, and
it certainly not present in the comments.

For instance, cifs_readdir twice converts FileName from UNICODE to ANSI
and then calls unix_fill_in_inode, where FileName is not even used.

 The disadvantage to this approach is that the 'Unicode' variable has to
be passed to obscure functions - except if the FILE_UNIX_INFO structure
had a variable like, IsUnicode - making the functions that play with the
'FileName' strings much easier to write. 

 This really goes back to point (1), if the FILE_UNIX_INFO (and
FILE_DIRECTORY_INFO) structure in cifspdu.h can be modified.

What you think?

 Richard Hughes